diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index e1371e566..07fc8e035 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -75,7 +75,7 @@ type connection struct { // initialized after receiving FUSE_INIT reply. // Until it's set, suspend sending FUSE requests. // Use SetInitialized() and IsInitialized() for atomic access. - initialized int32 + initialized atomicbitops.Int32 // initializedChan is used to block requests before initialization. initializedChan chan struct{} `state:".(bool)"` diff --git a/pkg/sentry/fsimpl/fuse/connection_control.go b/pkg/sentry/fsimpl/fuse/connection_control.go index 498fd1b54..5204f893d 100644 --- a/pkg/sentry/fsimpl/fuse/connection_control.go +++ b/pkg/sentry/fsimpl/fuse/connection_control.go @@ -15,8 +15,6 @@ package fuse import ( - "sync/atomic" - "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -65,13 +63,13 @@ func (conn *connection) SetInitialized() { // And it prevents the newer tasks from gaining // unnecessary higher chance to be issued before the blocked one. - atomic.StoreInt32(&(conn.initialized), int32(1)) + conn.initialized.Store(1) } -// IsInitialized atomically check if the connection is initialized. -// pairs with SetInitialized(). +// Initialized atomically check if the connection is initialized. pairs with +// SetInitialized(). func (conn *connection) Initialized() bool { - return atomic.LoadInt32(&(conn.initialized)) != 0 + return conn.initialized.Load() != 0 } // InitSend sends a FUSE_INIT request. diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index 6dc2a7f5e..77f18dcfd 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/pkg/sentry/fsimpl/gofer/directory.go @@ -16,7 +16,6 @@ package gofer import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -116,18 +115,18 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) { refs: atomicbitops.FromInt64(1), // held by d fs: d.fs, ino: d.fs.nextIno(), - mode: uint32(opts.mode), - uid: uint32(opts.kuid), - gid: uint32(opts.kgid), - blockSize: hostarch.PageSize, // arbitrary + mode: atomicbitops.FromUint32(uint32(opts.mode)), + uid: atomicbitops.FromUint32(uint32(opts.kuid)), + gid: atomicbitops.FromUint32(uint32(opts.kgid)), + blockSize: atomicbitops.FromUint32(hostarch.PageSize), // arbitrary atime: atomicbitops.FromInt64(now), mtime: atomicbitops.FromInt64(now), ctime: atomicbitops.FromInt64(now), btime: atomicbitops.FromInt64(now), - readFD: -1, - writeFD: -1, - mmapFD: -1, - nlink: uint32(2), + readFD: atomicbitops.FromInt32(-1), + writeFD: atomicbitops.FromInt32(-1), + mmapFD: atomicbitops.FromInt32(-1), + nlink: atomicbitops.FromUint32(2), } refsvfs2.Register(child) switch opts.mode.FileType() { @@ -228,7 +227,7 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) { }, { Name: "..", - Type: uint8(atomic.LoadUint32(&parent.mode) >> 12), + Type: uint8(parent.mode.Load() >> 12), Ino: uint64(parent.ino), NextOff: 2, }, @@ -338,7 +337,7 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) { } dirents = append(dirents, vfs.Dirent{ Name: child.name, - Type: uint8(atomic.LoadUint32(&child.mode) >> 12), + Type: uint8(child.mode.Load() >> 12), Ino: uint64(child.ino), NextOff: int64(len(dirents) + 1), }) diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 984cdf008..70ac6e7f8 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -19,10 +19,10 @@ import ( "math" "strings" "sync" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -625,7 +625,7 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b // Load child if sticky bit is set because we need to determine whether // deletion is allowed. var child *dentry - if atomic.LoadUint32(&parent.mode)&linux.ModeSticky == 0 { + if parent.mode.Load()&linux.ModeSticky == 0 { var ok bool child, ok = parent.children[name] if ok && child == nil { @@ -821,16 +821,16 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. if d.isDir() { return linuxerr.EPERM } - gid := auth.KGID(atomic.LoadUint32(&d.gid)) - uid := auth.KUID(atomic.LoadUint32(&d.uid)) - mode := linux.FileMode(atomic.LoadUint32(&d.mode)) + gid := auth.KGID(d.gid.Load()) + uid := auth.KUID(d.uid.Load()) + mode := linux.FileMode(d.mode.Load()) if err := vfs.MayLink(rp.Credentials(), mode, uid, gid); err != nil { return err } - if d.nlink == 0 { + if d.nlink.Load() == 0 { return linuxerr.ENOENT } - if d.nlink == math.MaxUint32 { + if d.nlink.Load() == math.MaxUint32 { return linuxerr.EMLINK } if fs.opts.lisaEnabled { @@ -858,8 +858,8 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v // rather than the caller's and enable setgid. kgid := creds.EffectiveKGID mode := opts.Mode - if atomic.LoadUint32(&parent.mode)&linux.S_ISGID != 0 { - kgid = auth.KGID(atomic.LoadUint32(&parent.gid)) + if parent.mode.Load()&linux.S_ISGID != 0 { + kgid = auth.KGID(parent.gid.Load()) mode |= linux.S_ISGID } var err error @@ -1130,7 +1130,7 @@ func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, opts *vfs.Open if err := fd.vfsfd.Init(fd, opts.Flags, mnt, &d.vfsd, &vfs.FileDescriptionOptions{}); err != nil { return nil, err } - if atomic.LoadInt32(&d.readFD) >= 0 { + if d.readFD.Load() >= 0 { fsmetric.GoferOpensHost.Increment() } else { fsmetric.GoferOpens9P.Increment() @@ -1280,8 +1280,8 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving // If the parent is a setgid directory, use the parent's GID rather // than the caller's. kgid := creds.EffectiveKGID - if atomic.LoadUint32(&d.mode)&linux.S_ISGID != 0 { - kgid = auth.KGID(atomic.LoadUint32(&d.gid)) + if d.mode.Load()&linux.S_ISGID != 0 { + kgid = auth.KGID(d.gid.Load()) } var child *dentry @@ -1357,14 +1357,14 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving child.readFile = openP9File child.readFDLisa = d.fs.clientLisa.NewFD(openLisaFD) if openHostFD != -1 { - child.readFD = openHostFD - child.mmapFD = openHostFD + child.readFD = atomicbitops.FromInt32(openHostFD) + child.mmapFD = atomicbitops.FromInt32(openHostFD) } } if vfs.MayWriteFileWithOpenFlags(opts.Flags) { child.writeFile = openP9File child.writeFDLisa = d.fs.clientLisa.NewFD(openLisaFD) - child.writeFD = openHostFD + child.writeFD = atomicbitops.FromInt32(openHostFD) } child.handleMu.Unlock() } diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 4dd712ef0..cb7fbddd3 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -42,7 +42,6 @@ import ( "path" "strconv" "strings" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" @@ -182,9 +181,8 @@ type filesystem struct { // savedDentryRW records open read/write handles during save/restore. savedDentryRW map[*dentry]savedDentryRW - // released is nonzero once filesystem.Release has been called. It is accessed - // with atomic memory operations. - released int32 + // released is nonzero once filesystem.Release has been called. + released atomicbitops.Int32 } // +stateify savable @@ -660,7 +658,7 @@ func (fs *filesystem) dial(ctx context.Context) error { // Release implements vfs.FilesystemImpl.Release. func (fs *filesystem) Release(ctx context.Context) { - atomic.StoreInt32(&fs.released, 1) + fs.released.Store(1) mf := fs.mfp.MemoryFile() fs.syncMu.Lock() @@ -678,16 +676,17 @@ func (fs *filesystem) Release(ctx context.Context) { d.cache.DropAll(mf) d.dirty.RemoveAll() d.dataMu.Unlock() - // Close host FDs if they exist. - if d.readFD >= 0 { - _ = unix.Close(int(d.readFD)) + // Close host FDs if they exist. We can use RacyLoad() because d.handleMu + // is locked. + if d.readFD.RacyLoad() >= 0 { + _ = unix.Close(int(d.readFD.RacyLoad())) } - if d.writeFD >= 0 && d.readFD != d.writeFD { - _ = unix.Close(int(d.writeFD)) + if d.writeFD.RacyLoad() >= 0 && d.readFD.RacyLoad() != d.writeFD.RacyLoad() { + _ = unix.Close(int(d.writeFD.RacyLoad())) } - d.readFD = -1 - d.writeFD = -1 - d.mmapFD = -1 + d.readFD = atomicbitops.FromInt32(-1) + d.writeFD = atomicbitops.FromInt32(-1) + d.mmapFD = atomicbitops.FromInt32(-1) d.handleMu.Unlock() } // There can't be any specialFileFDs still using fs, since each such @@ -814,8 +813,8 @@ type dentry struct { controlFDLisa lisafs.ClientFD `state:"nosave"` // If deleted is non-zero, the file represented by this dentry has been - // deleted. deleted is accessed using atomic memory operations. - deleted uint32 + // deleted is accessed using atomic memory operations. + deleted atomicbitops.Uint32 // cachingMu is used to synchronize concurrent dentry caching attempts on // this dentry. @@ -859,12 +858,12 @@ type dentry struct { // To mutate: // - Lock metadataMu and use atomic operations to update because we might // have atomic readers that don't hold the lock. - metadataMu sync.Mutex `state:"nosave"` - ino uint64 // immutable - mode uint32 // type is immutable, perms are mutable - uid uint32 // auth.KUID, but stored as raw uint32 for sync/atomic - gid uint32 // auth.KGID, but ... - blockSize uint32 // 0 if unknown + metadataMu sync.Mutex `state:"nosave"` + ino uint64 // immutable + mode atomicbitops.Uint32 // type is immutable, perms are mutable + uid atomicbitops.Uint32 // auth.KUID, but stored as raw uint32 for sync/atomic + gid atomicbitops.Uint32 // auth.KGID, but ... + blockSize atomicbitops.Uint32 // 0 if unknown // Timestamps, all nsecs from the Unix epoch. atime atomicbitops.Int64 mtime atomicbitops.Int64 @@ -882,13 +881,13 @@ type dentry struct { // atimeDirty/mtimeDirty are non-zero, atime/mtime may have diverged from the // remote file's timestamps, which should be updated when this dentry is // evicted. - atimeDirty uint32 - mtimeDirty uint32 + atimeDirty atomicbitops.Uint32 + mtimeDirty atomicbitops.Uint32 // nlink counts the number of hard links to this dentry. It's updated and // accessed using atomic operations. It's not protected by metadataMu like the // other metadata fields. - nlink uint32 + nlink atomicbitops.Uint32 mapsMu sync.Mutex `state:"nosave"` @@ -923,14 +922,14 @@ type dentry struct { // always either -1 or equal to readFD; if !writeFile.isNil() (the file has // been opened for writing), it is additionally either -1 or equal to // writeFD. - handleMu sync.RWMutex `state:"nosave"` - readFile p9file `state:"nosave"` - writeFile p9file `state:"nosave"` - readFDLisa lisafs.ClientFD `state:"nosave"` - writeFDLisa lisafs.ClientFD `state:"nosave"` - readFD int32 `state:"nosave"` - writeFD int32 `state:"nosave"` - mmapFD int32 `state:"nosave"` + handleMu sync.RWMutex `state:"nosave"` + readFile p9file `state:"nosave"` + writeFile p9file `state:"nosave"` + readFDLisa lisafs.ClientFD `state:"nosave"` + writeFDLisa lisafs.ClientFD `state:"nosave"` + readFD atomicbitops.Int32 `state:"nosave"` + writeFD atomicbitops.Int32 `state:"nosave"` + mmapFD atomicbitops.Int32 `state:"nosave"` dataMu sync.RWMutex `state:"nosave"` @@ -1010,26 +1009,26 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma qidPath: qid.Path, file: file, ino: fs.inoFromQIDPath(qid.Path), - mode: uint32(attr.Mode), - uid: uint32(fs.opts.dfltuid), - gid: uint32(fs.opts.dfltgid), - blockSize: hostarch.PageSize, - readFD: -1, - writeFD: -1, - mmapFD: -1, + mode: atomicbitops.FromUint32(uint32(attr.Mode)), + uid: atomicbitops.FromUint32(uint32(fs.opts.dfltuid)), + gid: atomicbitops.FromUint32(uint32(fs.opts.dfltgid)), + blockSize: atomicbitops.FromUint32(hostarch.PageSize), + readFD: atomicbitops.FromInt32(-1), + writeFD: atomicbitops.FromInt32(-1), + mmapFD: atomicbitops.FromInt32(-1), } d.pf.dentry = d if mask.UID { - d.uid = dentryUIDFromP9UID(attr.UID) + d.uid = atomicbitops.FromUint32(dentryUIDFromP9UID(attr.UID)) } if mask.GID { - d.gid = dentryGIDFromP9GID(attr.GID) + d.gid = atomicbitops.FromUint32(dentryGIDFromP9GID(attr.GID)) } if mask.Size { d.size = atomicbitops.FromUint64(attr.Size) } if attr.BlockSize != 0 { - d.blockSize = uint32(attr.BlockSize) + d.blockSize = atomicbitops.FromUint32(uint32(attr.BlockSize)) } if mask.ATime { d.atime = atomicbitops.FromInt64(dentryTimestampFromP9(attr.ATimeSeconds, attr.ATimeNanoSeconds)) @@ -1044,7 +1043,7 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma d.btime = atomicbitops.FromInt64(dentryTimestampFromP9(attr.BTimeSeconds, attr.BTimeNanoSeconds)) } if mask.NLink { - d.nlink = uint32(attr.NLink) + d.nlink = atomicbitops.FromUint32(uint32(attr.NLink)) } d.vfsd.Init(d) refsvfs2.Register(d) @@ -1069,28 +1068,28 @@ func (fs *filesystem) newDentryLisa(ctx context.Context, ino *lisafs.Inode) (*de fs: fs, inoKey: inoKey, ino: fs.inoFromKey(inoKey), - mode: uint32(ino.Stat.Mode), - uid: uint32(fs.opts.dfltuid), - gid: uint32(fs.opts.dfltgid), - blockSize: hostarch.PageSize, - readFD: -1, - writeFD: -1, - mmapFD: -1, + mode: atomicbitops.FromUint32(uint32(ino.Stat.Mode)), + uid: atomicbitops.FromUint32(uint32(fs.opts.dfltuid)), + gid: atomicbitops.FromUint32(uint32(fs.opts.dfltgid)), + blockSize: atomicbitops.FromUint32(hostarch.PageSize), + readFD: atomicbitops.FromInt32(-1), + writeFD: atomicbitops.FromInt32(-1), + mmapFD: atomicbitops.FromInt32(-1), controlFDLisa: fs.clientLisa.NewFD(ino.ControlFD), } d.pf.dentry = d if ino.Stat.Mask&linux.STATX_UID != 0 { - d.uid = dentryUIDFromLisaUID(lisafs.UID(ino.Stat.UID)) + d.uid = atomicbitops.FromUint32(dentryUIDFromLisaUID(lisafs.UID(ino.Stat.UID))) } if ino.Stat.Mask&linux.STATX_GID != 0 { - d.gid = dentryGIDFromLisaGID(lisafs.GID(ino.Stat.GID)) + d.gid = atomicbitops.FromUint32(dentryGIDFromLisaGID(lisafs.GID(ino.Stat.GID))) } if ino.Stat.Mask&linux.STATX_SIZE != 0 { d.size = atomicbitops.FromUint64(ino.Stat.Size) } if ino.Stat.Blksize != 0 { - d.blockSize = ino.Stat.Blksize + d.blockSize = atomicbitops.FromUint32(ino.Stat.Blksize) } if ino.Stat.Mask&linux.STATX_ATIME != 0 { d.atime = atomicbitops.FromInt64(dentryTimestampFromLisa(ino.Stat.Atime)) @@ -1105,7 +1104,7 @@ func (fs *filesystem) newDentryLisa(ctx context.Context, ino *lisafs.Inode) (*de d.btime = atomicbitops.FromInt64(dentryTimestampFromLisa(ino.Stat.Btime)) } if ino.Stat.Mask&linux.STATX_NLINK != 0 { - d.nlink = ino.Stat.Nlink + d.nlink = atomicbitops.FromUint32(ino.Stat.Nlink) } d.vfsd.Init(d) refsvfs2.Register(d) @@ -1159,24 +1158,24 @@ func (d *dentry) updateFromP9AttrsLocked(mask p9.AttrMask, attr *p9.Attr) { if got, want := uint32(attr.Mode.FileType()), d.fileType(); got != want { panic(fmt.Sprintf("gofer.dentry file type changed from %#o to %#o", want, got)) } - atomic.StoreUint32(&d.mode, uint32(attr.Mode)) + d.mode.Store(uint32(attr.Mode)) } if mask.UID { - atomic.StoreUint32(&d.uid, dentryUIDFromP9UID(attr.UID)) + d.uid.Store(dentryUIDFromP9UID(attr.UID)) } if mask.GID { - atomic.StoreUint32(&d.gid, dentryGIDFromP9GID(attr.GID)) + d.gid.Store(dentryGIDFromP9GID(attr.GID)) } // There is no P9_GETATTR_* bit for I/O block size. if attr.BlockSize != 0 { - atomic.StoreUint32(&d.blockSize, uint32(attr.BlockSize)) + d.blockSize.Store(uint32(attr.BlockSize)) } // Don't override newer client-defined timestamps with old server-defined // ones. - if mask.ATime && atomic.LoadUint32(&d.atimeDirty) == 0 { + if mask.ATime && d.atimeDirty.Load() == 0 { d.atime.Store(dentryTimestampFromP9(attr.ATimeSeconds, attr.ATimeNanoSeconds)) } - if mask.MTime && atomic.LoadUint32(&d.mtimeDirty) == 0 { + if mask.MTime && d.mtimeDirty.Load() == 0 { d.mtime.Store(dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds)) } if mask.CTime { @@ -1186,7 +1185,7 @@ func (d *dentry) updateFromP9AttrsLocked(mask p9.AttrMask, attr *p9.Attr) { d.btime.Store(dentryTimestampFromP9(attr.BTimeSeconds, attr.BTimeNanoSeconds)) } if mask.NLink { - atomic.StoreUint32(&d.nlink, uint32(attr.NLink)) + d.nlink.Store(uint32(attr.NLink)) } if mask.Size { d.updateSizeLocked(attr.Size) @@ -1204,23 +1203,23 @@ func (d *dentry) updateFromLisaStatLocked(stat *linux.Statx) { } } if stat.Mask&linux.STATX_MODE != 0 { - atomic.StoreUint32(&d.mode, uint32(stat.Mode)) + d.mode.Store(uint32(stat.Mode)) } if stat.Mask&linux.STATX_UID != 0 { - atomic.StoreUint32(&d.uid, dentryUIDFromLisaUID(lisafs.UID(stat.UID))) + d.uid.Store(dentryUIDFromLisaUID(lisafs.UID(stat.UID))) } if stat.Mask&linux.STATX_GID != 0 { - atomic.StoreUint32(&d.gid, dentryGIDFromLisaGID(lisafs.GID(stat.GID))) + d.gid.Store(dentryGIDFromLisaGID(lisafs.GID(stat.GID))) } if stat.Blksize != 0 { - atomic.StoreUint32(&d.blockSize, stat.Blksize) + d.blockSize.Store(stat.Blksize) } // Don't override newer client-defined timestamps with old server-defined // ones. - if stat.Mask&linux.STATX_ATIME != 0 && atomic.LoadUint32(&d.atimeDirty) == 0 { + if stat.Mask&linux.STATX_ATIME != 0 && d.atimeDirty.Load() == 0 { d.atime.Store(dentryTimestampFromLisa(stat.Atime)) } - if stat.Mask&linux.STATX_MTIME != 0 && atomic.LoadUint32(&d.mtimeDirty) == 0 { + if stat.Mask&linux.STATX_MTIME != 0 && d.mtimeDirty.Load() == 0 { d.mtime.Store(dentryTimestampFromLisa(stat.Mtime)) } if stat.Mask&linux.STATX_CTIME != 0 { @@ -1230,7 +1229,7 @@ func (d *dentry) updateFromLisaStatLocked(stat *linux.Statx) { d.btime.Store(dentryTimestampFromLisa(stat.Btime)) } if stat.Mask&linux.STATX_NLINK != 0 { - atomic.StoreUint32(&d.nlink, stat.Nlink) + d.nlink.Store(stat.Nlink) } if stat.Mask&linux.STATX_SIZE != 0 { d.updateSizeLocked(stat.Size) @@ -1243,7 +1242,8 @@ func (d *dentry) updateFromLisaStatLocked(stat *linux.Statx) { func (d *dentry) refreshSizeLocked(ctx context.Context) error { d.handleMu.RLock() - if d.writeFD < 0 { + // Can use RacyLoad() because handleMu is locked. + if d.writeFD.RacyLoad() < 0 { d.handleMu.RUnlock() // Ask the gofer if we don't have a host FD. if d.fs.opts.lisaEnabled { @@ -1253,7 +1253,8 @@ func (d *dentry) refreshSizeLocked(ctx context.Context) error { } var stat unix.Statx_t - err := unix.Statx(int(d.writeFD), "", unix.AT_EMPTY_PATH, unix.STATX_SIZE, &stat) + // Can use RacyLoad() because handleMu is locked. + err := unix.Statx(int(d.writeFD.RacyLoad()), "", unix.AT_EMPTY_PATH, unix.STATX_SIZE, &stat) d.handleMu.RUnlock() // must be released before updateSizeLocked() if err != nil { return err @@ -1354,13 +1355,13 @@ func (d *dentry) updateFromGetattrLocked(ctx context.Context, file p9file) error } func (d *dentry) fileType() uint32 { - return atomic.LoadUint32(&d.mode) & linux.S_IFMT + return d.mode.Load() & linux.S_IFMT } func (d *dentry) statTo(stat *linux.Statx) { stat.Mask = linux.STATX_TYPE | linux.STATX_MODE | linux.STATX_NLINK | linux.STATX_UID | linux.STATX_GID | linux.STATX_ATIME | linux.STATX_MTIME | linux.STATX_CTIME | linux.STATX_INO | linux.STATX_SIZE | linux.STATX_BLOCKS | linux.STATX_BTIME - stat.Blksize = atomic.LoadUint32(&d.blockSize) - stat.Nlink = atomic.LoadUint32(&d.nlink) + stat.Blksize = d.blockSize.Load() + stat.Nlink = d.nlink.Load() if stat.Nlink == 0 { // The remote filesystem doesn't support link count; just make // something up. This is consistent with Linux, where @@ -1369,9 +1370,9 @@ func (d *dentry) statTo(stat *linux.Statx) { // it's not provided by the remote filesystem. stat.Nlink = 1 } - stat.UID = atomic.LoadUint32(&d.uid) - stat.GID = atomic.LoadUint32(&d.gid) - stat.Mode = uint16(atomic.LoadUint32(&d.mode)) + stat.UID = d.uid.Load() + stat.GID = d.gid.Load() + stat.Mode = uint16(d.mode.Load()) stat.Ino = uint64(d.ino) stat.Size = d.size.Load() // This is consistent with regularFileFD.Seek(), which treats regular files @@ -1393,8 +1394,8 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs if stat.Mask&^(linux.STATX_MODE|linux.STATX_UID|linux.STATX_GID|linux.STATX_ATIME|linux.STATX_MTIME|linux.STATX_SIZE) != 0 { return linuxerr.EPERM } - mode := linux.FileMode(atomic.LoadUint32(&d.mode)) - if err := vfs.CheckSetStat(ctx, creds, opts, mode, auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid))); err != nil { + mode := linux.FileMode(d.mode.Load()) + if err := vfs.CheckSetStat(ctx, creds, opts, mode, auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { return err } if err := mnt.CheckBeginWrite(); err != nil { @@ -1441,14 +1442,14 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs // As with Linux, if the UID, GID, or file size is changing, we have to // clear permission bits. Note that when set, clearSGID may cause // permissions to be updated. - clearSGID := (stat.Mask&linux.STATX_UID != 0 && stat.UID != atomic.LoadUint32(&d.uid)) || - (stat.Mask&linux.STATX_GID != 0 && stat.GID != atomic.LoadUint32(&d.gid)) || + clearSGID := (stat.Mask&linux.STATX_UID != 0 && stat.UID != d.uid.Load()) || + (stat.Mask&linux.STATX_GID != 0 && stat.GID != d.gid.Load()) || stat.Mask&linux.STATX_SIZE != 0 if clearSGID { if stat.Mask&linux.STATX_MODE != 0 { stat.Mode = uint16(vfs.ClearSUIDAndSGID(uint32(stat.Mode))) } else { - oldMode := atomic.LoadUint32(&d.mode) + oldMode := d.mode.Load() if updatedMode := vfs.ClearSUIDAndSGID(oldMode); updatedMode != oldMode { stat.Mode = uint16(updatedMode) stat.Mask |= linux.STATX_MODE @@ -1527,13 +1528,13 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs } } if stat.Mask&linux.STATX_MODE != 0 && failureMask&linux.STATX_MODE == 0 { - atomic.StoreUint32(&d.mode, d.fileType()|uint32(stat.Mode)) + d.mode.Store(d.fileType() | uint32(stat.Mode)) } if stat.Mask&linux.STATX_UID != 0 && failureMask&linux.STATX_UID == 0 { - atomic.StoreUint32(&d.uid, stat.UID) + d.uid.Store(stat.UID) } if stat.Mask&linux.STATX_GID != 0 && failureMask&linux.STATX_GID == 0 { - atomic.StoreUint32(&d.gid, stat.GID) + d.gid.Store(stat.GID) } // Note that stat.Atime.Nsec and stat.Mtime.Nsec can't be UTIME_NOW because // if d.cachedMetadataAuthoritative() then we converted stat.Atime and @@ -1542,11 +1543,11 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs // d.file.setAttr(). For the same reason, now must have been initialized. if stat.Mask&linux.STATX_ATIME != 0 && failureMask&linux.STATX_ATIME == 0 { d.atime.Store(stat.Atime.ToNsec()) - atomic.StoreUint32(&d.atimeDirty, 0) + d.atimeDirty.Store(0) } if stat.Mask&linux.STATX_MTIME != 0 && failureMask&linux.STATX_MTIME == 0 { d.mtime.Store(stat.Mtime.ToNsec()) - atomic.StoreUint32(&d.mtimeDirty, 0) + d.mtimeDirty.Store(0) } d.ctime.Store(now) if failureMask != 0 { @@ -1623,7 +1624,7 @@ func (d *dentry) updateSizeAndUnlockDataMuLocked(newSize uint64) { } func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) error { - return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(atomic.LoadUint32(&d.mode)), auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid))) + return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())) } func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats vfs.AccessTypes) error { @@ -1638,9 +1639,9 @@ func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats if strings.HasPrefix(name, linux.XATTR_SECURITY_PREFIX) || strings.HasPrefix(name, linux.XATTR_SYSTEM_PREFIX) || strings.HasPrefix(name, linux.XATTR_TRUSTED_PREFIX) { return linuxerr.EOPNOTSUPP } - mode := linux.FileMode(atomic.LoadUint32(&d.mode)) - kuid := auth.KUID(atomic.LoadUint32(&d.uid)) - kgid := auth.KGID(atomic.LoadUint32(&d.gid)) + mode := linux.FileMode(d.mode.Load()) + kuid := auth.KUID(d.uid.Load()) + kgid := auth.KGID(d.gid.Load()) if err := vfs.GenericCheckPermissions(creds, ats, mode, kuid, kgid); err != nil { return err } @@ -1650,10 +1651,10 @@ func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error { return vfs.CheckDeleteSticky( creds, - linux.FileMode(atomic.LoadUint32(&d.mode)), - auth.KUID(atomic.LoadUint32(&d.uid)), - auth.KUID(atomic.LoadUint32(&child.uid)), - auth.KGID(atomic.LoadUint32(&child.gid)), + linux.FileMode(d.mode.Load()), + auth.KUID(d.uid.Load()), + auth.KUID(child.uid.Load()), + auth.KGID(child.gid.Load()), ) } @@ -1846,7 +1847,7 @@ func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked boo return } - if atomic.LoadInt32(&d.fs.released) != 0 { + if d.fs.released.Load() != 0 { d.cachingMu.Unlock() if !renameMuWriteLocked { // Need to lock d.fs.renameMu to access d.parent. Lock it for writing as @@ -2011,15 +2012,16 @@ func (d *dentry) destroyLocked(ctx context.Context) { d.readFile = p9file{} d.writeFile = p9file{} } - if d.readFD >= 0 { - _ = unix.Close(int(d.readFD)) + // Can use RacyLoad() because handleMu is locked. + if d.readFD.RacyLoad() >= 0 { + _ = unix.Close(int(d.readFD.RacyLoad())) } - if d.writeFD >= 0 && d.readFD != d.writeFD { - _ = unix.Close(int(d.writeFD)) + if d.writeFD.RacyLoad() >= 0 && d.readFD.RacyLoad() != d.writeFD.RacyLoad() { + _ = unix.Close(int(d.writeFD.RacyLoad())) } - d.readFD = -1 - d.writeFD = -1 - d.mmapFD = -1 + d.readFD = atomicbitops.FromInt32(-1) + d.writeFD = atomicbitops.FromInt32(-1) + d.mmapFD = atomicbitops.FromInt32(-1) d.handleMu.Unlock() if d.isControlFileOk() { @@ -2059,11 +2061,11 @@ func (d *dentry) destroyLocked(ctx context.Context) { } func (d *dentry) isDeleted() bool { - return atomic.LoadUint32(&d.deleted) != 0 + return d.deleted.Load() != 0 } func (d *dentry) setDeleted() { - atomic.StoreUint32(&d.deleted, 1) + d.deleted.Store(1) } func (d *dentry) isControlFileOk() bool { @@ -2214,11 +2216,11 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool return err } - // Update d.readFD and d.writeFD. + // Update d.readFD and d.writeFD if h.fd >= 0 { - if openReadable && openWritable && (d.readFD < 0 || d.writeFD < 0 || d.readFD != d.writeFD) { + if openReadable && openWritable && (d.readFD.RacyLoad() < 0 || d.writeFD.RacyLoad() < 0 || d.readFD.RacyLoad() != d.writeFD.RacyLoad()) { // Replace existing FDs with this one. - if d.readFD >= 0 { + if d.readFD.RacyLoad() >= 0 { // We already have a readable FD that may be in use by // concurrent callers of d.pf.FD(). if d.fs.opts.overlayfsStaleRead { @@ -2232,9 +2234,9 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool h.close(ctx) return err } - fdsToClose = append(fdsToClose, d.readFD) + fdsToClose = append(fdsToClose, d.readFD.RacyLoad()) invalidateTranslations = true - atomic.StoreInt32(&d.readFD, h.fd) + d.readFD.Store(h.fd) } else { // Otherwise, we want to avoid invalidating existing // memmap.Translations (which is expensive); instead, use @@ -2244,26 +2246,26 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool // may use the old or new file description, but this // doesn't matter since they refer to the same file, and // any racing mappings must be read-only. - if err := unix.Dup3(int(h.fd), int(d.readFD), unix.O_CLOEXEC); err != nil { - oldFD := d.readFD + if err := unix.Dup3(int(h.fd), int(d.readFD.RacyLoad()), unix.O_CLOEXEC); err != nil { + oldFD := d.readFD.RacyLoad() d.handleMu.Unlock() ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to dup fd %d to fd %d: %v", h.fd, oldFD, err) h.close(ctx) return err } fdsToClose = append(fdsToClose, h.fd) - h.fd = d.readFD + h.fd = d.readFD.RacyLoad() } } else { - atomic.StoreInt32(&d.readFD, h.fd) + d.readFD.Store(h.fd) } - if d.writeFD != h.fd && d.writeFD >= 0 { - fdsToClose = append(fdsToClose, d.writeFD) + if d.writeFD.RacyLoad() != h.fd && d.writeFD.RacyLoad() >= 0 { + fdsToClose = append(fdsToClose, d.writeFD.RacyLoad()) } - atomic.StoreInt32(&d.writeFD, h.fd) - atomic.StoreInt32(&d.mmapFD, h.fd) - } else if openReadable && d.readFD < 0 { - atomic.StoreInt32(&d.readFD, h.fd) + d.writeFD.Store(h.fd) + d.mmapFD.Store(h.fd) + } else if openReadable && d.readFD.RacyLoad() < 0 { + d.readFD.Store(h.fd) // If the file has not been opened for writing, the new FD may // be used for read-only memory mappings. If the file was // previously opened for reading (without an FD), then existing @@ -2272,17 +2274,17 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool if d.fs.opts.lisaEnabled { if !d.writeFDLisa.Ok() { invalidateTranslations = d.readFDLisa.Ok() - atomic.StoreInt32(&d.mmapFD, h.fd) + d.mmapFD.Store(h.fd) } } else { if d.writeFile.isNil() { invalidateTranslations = !d.readFile.isNil() - atomic.StoreInt32(&d.mmapFD, h.fd) + d.mmapFD.Store(h.fd) } } - } else if openWritable && d.writeFD < 0 { - atomic.StoreInt32(&d.writeFD, h.fd) - if d.readFD >= 0 { + } else if openWritable && d.writeFD.RacyLoad() < 0 { + d.writeFD.Store(h.fd) + if d.readFD.RacyLoad() >= 0 { // We have an existing read-only FD, but the file has just // been opened for writing, so we need to start supporting // writable memory mappings. However, the new FD is not @@ -2290,19 +2292,19 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool // writable memory mappings. Switch to using the internal // page cache. invalidateTranslations = true - atomic.StoreInt32(&d.mmapFD, -1) + d.mmapFD.Store(-1) } } else { // The new FD is not useful. fdsToClose = append(fdsToClose, h.fd) } - } else if openWritable && d.writeFD < 0 && d.mmapFD >= 0 { + } else if openWritable && d.writeFD.RacyLoad() < 0 && d.mmapFD.RacyLoad() >= 0 { // We have an existing read-only FD, but the file has just been // opened for writing, so we need to start supporting writable // memory mappings. However, we have no writable host FD. Switch to // using the internal page cache. invalidateTranslations = true - atomic.StoreInt32(&d.mmapFD, -1) + d.mmapFD.Store(-1) } // Switch to new fids/FDs. @@ -2369,7 +2371,7 @@ func (d *dentry) readHandleLocked() handle { return handle{ fdLisa: d.readFDLisa, file: d.readFile, - fd: d.readFD, + fd: d.readFD.RacyLoad(), } } @@ -2378,7 +2380,7 @@ func (d *dentry) writeHandleLocked() handle { return handle{ fdLisa: d.writeFDLisa, file: d.writeFile, - fd: d.writeFD, + fd: d.writeFD.RacyLoad(), } } @@ -2394,9 +2396,9 @@ func (d *dentry) syncRemoteFileLocked(ctx context.Context) error { // RPC. Prefer syncing write handles over read handles, since some remote // filesystem implementations may not sync changes made through write // handles otherwise. - if d.writeFD >= 0 { + if d.writeFD.RacyLoad() >= 0 { ctx.UninterruptibleSleepStart(false) - err := unix.Fsync(int(d.writeFD)) + err := unix.Fsync(int(d.writeFD.RacyLoad())) ctx.UninterruptibleSleepFinish(false) return err } @@ -2405,9 +2407,9 @@ func (d *dentry) syncRemoteFileLocked(ctx context.Context) error { } else if !d.fs.opts.lisaEnabled && !d.writeFile.isNil() { return d.writeFile.fsync(ctx) } - if d.readFD >= 0 { + if d.readFD.RacyLoad() >= 0 { ctx.UninterruptibleSleepStart(false) - err := unix.Fsync(int(d.readFD)) + err := unix.Fsync(int(d.readFD.RacyLoad())) ctx.UninterruptibleSleepFinish(false) return err } @@ -2448,20 +2450,20 @@ func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool) err // incLinks increments link count. func (d *dentry) incLinks() { - if atomic.LoadUint32(&d.nlink) == 0 { + if d.nlink.Load() == 0 { // The remote filesystem doesn't support link count. return } - atomic.AddUint32(&d.nlink, 1) + d.nlink.Add(1) } // decLinks decrements link count. func (d *dentry) decLinks() { - if atomic.LoadUint32(&d.nlink) == 0 { + if d.nlink.Load() == 0 { // The remote filesystem doesn't support link count. return } - atomic.AddUint32(&d.nlink, ^uint32(0)) + d.nlink.Add(^uint32(0)) } // fileDescription is embedded by gofer implementations of diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go index 8ba5f1131..759d29c57 100644 --- a/pkg/sentry/fsimpl/gofer/regular_file.go +++ b/pkg/sentry/fsimpl/gofer/regular_file.go @@ -18,7 +18,6 @@ import ( "fmt" "io" "math" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -59,10 +58,10 @@ func newRegularFileFD(mnt *vfs.Mount, d *dentry, flags uint32) (*regularFileFD, }); err != nil { return nil, err } - if fd.vfsfd.IsWritable() && (atomic.LoadUint32(&d.mode)&0111 != 0) { + if fd.vfsfd.IsWritable() && (d.mode.Load()&0111 != 0) { metric.SuspiciousOperationsMetric.Increment("opened_write_execute_file") } - if atomic.LoadInt32(&d.mmapFD) >= 0 { + if d.mmapFD.Load() >= 0 { fsmetric.GoferOpensHost.Increment() } else { fsmetric.GoferOpens9P.Increment() @@ -128,7 +127,7 @@ func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs start := fsmetric.StartReadWait() d := fd.dentry() defer func() { - if atomic.LoadInt32(&d.readFD) >= 0 { + if d.readFD.Load() >= 0 { fsmetric.GoferReadsHost.Increment() fsmetric.FinishReadWait(fsmetric.GoferReadWaitHost, start) } else { @@ -286,11 +285,11 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // As with Linux, writing clears the setuid and setgid bits. if n > 0 { - oldMode := atomic.LoadUint32(&d.mode) + oldMode := d.mode.Load() // If setuid or setgid were set, update d.mode and propagate // changes to the host. if newMode := vfs.ClearSUIDAndSGID(oldMode); newMode != oldMode { - atomic.StoreUint32(&d.mode, newMode) + d.mode.Store(newMode) if d.fs.opts.lisaEnabled { stat := linux.Statx{Mask: linux.STATX_MODE, Mode: uint16(newMode)} failureMask, failureErr, err := d.controlFDLisa.SetStat(ctx, &stat) @@ -399,7 +398,7 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) // dentry.readHandleLocked() without locking dentry.dataMu. rw.d.handleMu.RLock() h := rw.d.readHandleLocked() - if (rw.d.mmapFD >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct { + if (rw.d.mmapFD.RacyLoad() >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct { n, err := h.readToBlocksAt(rw.ctx, dsts, rw.off) rw.d.handleMu.RUnlock() rw.off += n @@ -519,7 +518,7 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro // without locking dentry.dataMu. rw.d.handleMu.RLock() h := rw.d.writeHandleLocked() - if (rw.d.mmapFD >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct { + if (rw.d.mmapFD.RacyLoad() >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct { n, err := h.writeFromBlocksAt(rw.ctx, srcs, rw.off) rw.off += n rw.d.dataMu.Lock() @@ -720,7 +719,7 @@ func (fd *regularFileFD) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpt case InteropModeShared: // All mappings require a host FD to be coherent with other // filesystem users. - if atomic.LoadInt32(&d.mmapFD) < 0 { + if d.mmapFD.Load() < 0 { return linuxerr.ENODEV } default: @@ -790,7 +789,7 @@ func (d *dentry) CopyMapping(ctx context.Context, ms memmap.MappingSpace, srcAR, // Translate implements memmap.Mappable.Translate. func (d *dentry) Translate(ctx context.Context, required, optional memmap.MappableRange, at hostarch.AccessType) ([]memmap.Translation, error) { d.handleMu.RLock() - if d.mmapFD >= 0 && !d.fs.opts.forcePageCache { + if d.mmapFD.RacyLoad() >= 0 && !d.fs.opts.forcePageCache { d.handleMu.RUnlock() mr := optional if d.fs.opts.limitHostFDTranslation { @@ -981,12 +980,12 @@ func (d *dentryPlatformFile) DecRef(fr memmap.FileRange) { func (d *dentryPlatformFile) MapInternal(fr memmap.FileRange, at hostarch.AccessType) (safemem.BlockSeq, error) { d.handleMu.RLock() defer d.handleMu.RUnlock() - return d.hostFileMapper.MapInternal(fr, int(d.mmapFD), at.Write) + return d.hostFileMapper.MapInternal(fr, int(d.mmapFD.RacyLoad()), at.Write) } // FD implements memmap.File.FD. func (d *dentryPlatformFile) FD() int { d.handleMu.RLock() defer d.handleMu.RUnlock() - return int(d.mmapFD) + return int(d.mmapFD.RacyLoad()) } diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index 482c48b9e..449aec324 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -17,9 +17,9 @@ package gofer import ( "fmt" "io" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" @@ -100,7 +100,7 @@ func (fd *specialFileFD) savePipeData(ctx context.Context) error { } } if len(fd.buf) != 0 { - atomic.StoreUint32(&fd.haveBuf, 1) + fd.haveBuf.Store(1) } return nil } @@ -149,9 +149,9 @@ func (d *dentry) beforeSave() { // afterLoad is invoked by stateify. func (d *dentry) afterLoad() { - d.readFD = -1 - d.writeFD = -1 - d.mmapFD = -1 + d.readFD = atomicbitops.FromInt32(-1) + d.writeFD = atomicbitops.FromInt32(-1) + d.mmapFD = atomicbitops.FromInt32(-1) if d.refs.Load() != -1 { refsvfs2.Register(d) } diff --git a/pkg/sentry/fsimpl/gofer/special_file.go b/pkg/sentry/fsimpl/gofer/special_file.go index a9ad0a239..0dcd74ab4 100644 --- a/pkg/sentry/fsimpl/gofer/special_file.go +++ b/pkg/sentry/fsimpl/gofer/special_file.go @@ -16,10 +16,10 @@ package gofer import ( "fmt" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" @@ -74,10 +74,9 @@ type specialFileFD struct { // If haveBuf is non-zero, this FD represents a pipe, and buf contains data // read from the pipe from previous calls to specialFileFD.savePipeData(). - // haveBuf and buf are protected by bufMu. haveBuf is accessed using atomic - // memory operations. + // haveBuf and buf are protected by bufMu. bufMu sync.Mutex `state:"nosave"` - haveBuf uint32 + haveBuf atomicbitops.Uint32 buf []byte // If handle.fd >= 0, hostFileMapper caches mappings of handle.fd, and @@ -119,7 +118,7 @@ func newSpecialFileFD(h handle, mnt *vfs.Mount, d *dentry, flags uint32) (*speci d.fs.syncMu.Lock() d.fs.specialFileFDs[fd] = struct{}{} d.fs.syncMu.Unlock() - if fd.vfsfd.IsWritable() && (atomic.LoadUint32(&d.mode)&0111 != 0) { + if fd.vfsfd.IsWritable() && (d.mode.Load()&0111 != 0) { metric.SuspiciousOperationsMetric.Increment("opened_write_execute_file") } if h.fd >= 0 { @@ -239,7 +238,7 @@ func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs } bufN := int64(0) - if atomic.LoadUint32(&fd.haveBuf) != 0 { + if fd.haveBuf.Load() != 0 { var err error fd.bufMu.Lock() if len(fd.buf) != 0 { @@ -248,7 +247,7 @@ func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs dst = dst.DropFirst(n) fd.buf = fd.buf[n:] if len(fd.buf) == 0 { - atomic.StoreUint32(&fd.haveBuf, 0) + fd.haveBuf.Store(0) fd.buf = nil } bufN = int64(n) diff --git a/pkg/sentry/fsimpl/gofer/time.go b/pkg/sentry/fsimpl/gofer/time.go index 2381ea30c..17a35612b 100644 --- a/pkg/sentry/fsimpl/gofer/time.go +++ b/pkg/sentry/fsimpl/gofer/time.go @@ -15,8 +15,6 @@ package gofer import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/sentry/vfs" ) @@ -40,7 +38,7 @@ func (d *dentry) touchAtime(mnt *vfs.Mount) { now := d.fs.clock.Now().Nanoseconds() d.metadataMu.Lock() d.atime.Store(now) - atomic.StoreUint32(&d.atimeDirty, 1) + d.atimeDirty.Store(1) d.metadataMu.Unlock() mnt.EndWrite() } @@ -55,7 +53,7 @@ func (d *dentry) touchAtimeLocked(mnt *vfs.Mount) { } now := d.fs.clock.Now().Nanoseconds() d.atime.Store(now) - atomic.StoreUint32(&d.atimeDirty, 1) + d.atimeDirty.Store(1) mnt.EndWrite() } @@ -77,7 +75,7 @@ func (d *dentry) touchCMtime() { d.metadataMu.Lock() d.mtime.Store(now) d.ctime.Store(now) - atomic.StoreUint32(&d.mtimeDirty, 1) + d.mtimeDirty.Store(1) d.metadataMu.Unlock() } @@ -88,5 +86,5 @@ func (d *dentry) touchCMtimeLocked() { now := d.fs.clock.Now().Nanoseconds() d.mtime.Store(now) d.ctime.Store(now) - atomic.StoreUint32(&d.mtimeDirty, 1) + d.mtimeDirty.Store(1) } diff --git a/pkg/sentry/fsimpl/host/host.go b/pkg/sentry/fsimpl/host/host.go index 8fa22b74d..fd9a751a9 100644 --- a/pkg/sentry/fsimpl/host/host.go +++ b/pkg/sentry/fsimpl/host/host.go @@ -19,10 +19,10 @@ package host import ( "fmt" "math" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" @@ -55,23 +55,23 @@ type virtualOwner struct { // mu protects the fields below and they can be accessed using atomic memory // operations. mu sync.Mutex `state:"nosave"` - uid uint32 - gid uint32 + uid atomicbitops.Uint32 + gid atomicbitops.Uint32 // mode is also stored, otherwise setting the host file to `0000` could remove // access to the file. - mode uint32 + mode atomicbitops.Uint32 } func (v *virtualOwner) atomicUID() uint32 { - return atomic.LoadUint32(&v.uid) + return v.uid.Load() } func (v *virtualOwner) atomicGID() uint32 { - return atomic.LoadUint32(&v.gid) + return v.gid.Load() } func (v *virtualOwner) atomicMode() uint32 { - return atomic.LoadUint32(&v.mode) + return v.mode.Load() } func isEpollable(fd int) bool { @@ -153,10 +153,9 @@ type inode struct { // If haveBuf is non-zero, hostFD represents a pipe, and buf contains data // read from the pipe from previous calls to inode.beforeSave(). haveBuf - // and buf are protected by bufMu. haveBuf is accessed using atomic memory - // operations. + // and buf are protected by bufMu. bufMu sync.Mutex `state:"nosave"` - haveBuf uint32 + haveBuf atomicbitops.Uint32 buf []byte } @@ -249,9 +248,9 @@ func NewFD(ctx context.Context, mnt *vfs.Mount, hostFD int, opts *NewFDOptions) } if opts.VirtualOwner { i.virtualOwner.enabled = true - i.virtualOwner.uid = uint32(opts.UID) - i.virtualOwner.gid = uint32(opts.GID) - i.virtualOwner.mode = stat.Mode + i.virtualOwner.uid = atomicbitops.FromUint32(uint32(opts.UID)) + i.virtualOwner.gid = atomicbitops.FromUint32(uint32(opts.GID)) + i.virtualOwner.mode = atomicbitops.FromUint32(stat.Mode) } d := &kernfs.Dentry{} @@ -521,7 +520,8 @@ func (i *inode) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *auth.Cre if m&linux.STATX_MODE != 0 { if i.virtualOwner.enabled { - i.virtualOwner.mode = uint32(opts.Stat.Mode) + // We hold i.virtualOwner.mu. + i.virtualOwner.mode = atomicbitops.FromUint32(uint32(opts.Stat.Mode)) } else { if err := unix.Fchmod(i.hostFD, uint32(s.Mode)); err != nil { return err @@ -555,10 +555,12 @@ func (i *inode) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *auth.Cre } if i.virtualOwner.enabled { if m&linux.STATX_UID != 0 { - i.virtualOwner.uid = opts.Stat.UID + // We hold i.virtualOwner.mu. + i.virtualOwner.uid = atomicbitops.FromUint32(opts.Stat.UID) } if m&linux.STATX_GID != 0 { - i.virtualOwner.gid = opts.Stat.GID + // We hold i.virtualOwner.mu. + i.virtualOwner.gid = atomicbitops.FromUint32(opts.Stat.GID) } } return nil @@ -757,7 +759,7 @@ func (f *fileDescription) Read(ctx context.Context, dst usermem.IOSequence, opts } func (i *inode) readFromBuf(ctx context.Context, dst *usermem.IOSequence) (int64, error) { - if atomic.LoadUint32(&i.haveBuf) == 0 { + if i.haveBuf.Load() == 0 { return 0, nil } i.bufMu.Lock() @@ -769,7 +771,7 @@ func (i *inode) readFromBuf(ctx context.Context, dst *usermem.IOSequence) (int64 *dst = dst.DropFirst(n) i.buf = i.buf[n:] if len(i.buf) == 0 { - atomic.StoreUint32(&i.haveBuf, 0) + i.haveBuf.Store(0) i.buf = nil } return int64(n), err diff --git a/pkg/sentry/fsimpl/host/save_restore.go b/pkg/sentry/fsimpl/host/save_restore.go index 5fb44d03f..7f46cbb55 100644 --- a/pkg/sentry/fsimpl/host/save_restore.go +++ b/pkg/sentry/fsimpl/host/save_restore.go @@ -17,7 +17,6 @@ package host import ( "fmt" "io" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/fdnotifier" @@ -52,7 +51,7 @@ func (i *inode) beforeSave() { } } if len(i.buf) != 0 { - atomic.StoreUint32(&i.haveBuf, 1) + i.haveBuf.Store(1) } } } diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index 424d3f33d..8530464eb 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -16,7 +16,6 @@ package kernfs import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -178,11 +177,11 @@ type InodeAttrs struct { devMajor uint32 devMinor uint32 ino atomicbitops.Uint64 - mode uint32 - uid uint32 - gid uint32 - nlink uint32 - blockSize uint32 + mode atomicbitops.Uint32 + uid atomicbitops.Uint32 + gid atomicbitops.Uint32 + nlink atomicbitops.Uint32 + blockSize atomicbitops.Uint32 // Timestamps, all nsecs from the Unix epoch. atime atomicbitops.Int64 @@ -203,11 +202,11 @@ func (a *InodeAttrs) Init(ctx context.Context, creds *auth.Credentials, devMajor a.devMajor = devMajor a.devMinor = devMinor a.ino.Store(ino) - atomic.StoreUint32(&a.mode, uint32(mode)) - atomic.StoreUint32(&a.uid, uint32(creds.EffectiveKUID)) - atomic.StoreUint32(&a.gid, uint32(creds.EffectiveKGID)) - atomic.StoreUint32(&a.nlink, nlink) - atomic.StoreUint32(&a.blockSize, hostarch.PageSize) + a.mode.Store(uint32(mode)) + a.uid.Store(uint32(creds.EffectiveKUID)) + a.gid.Store(uint32(creds.EffectiveKGID)) + a.nlink.Store(nlink) + a.blockSize.Store(hostarch.PageSize) now := ktime.NowFromContext(ctx).Nanoseconds() a.atime.Store(now) a.mtime.Store(now) @@ -231,12 +230,12 @@ func (a *InodeAttrs) Ino() uint64 { // Mode implements Inode.Mode. func (a *InodeAttrs) Mode() linux.FileMode { - return linux.FileMode(atomic.LoadUint32(&a.mode)) + return linux.FileMode(a.mode.Load()) } // Links returns the link count. func (a *InodeAttrs) Links() uint32 { - return atomic.LoadUint32(&a.nlink) + return a.nlink.Load() } // TouchAtime updates a.atime to the current time. @@ -270,10 +269,10 @@ func (a *InodeAttrs) Stat(context.Context, *vfs.Filesystem, vfs.StatOptions) (li stat.DevMinor = a.devMinor stat.Ino = a.ino.Load() stat.Mode = uint16(a.Mode()) - stat.UID = atomic.LoadUint32(&a.uid) - stat.GID = atomic.LoadUint32(&a.gid) - stat.Nlink = atomic.LoadUint32(&a.nlink) - stat.Blksize = atomic.LoadUint32(&a.blockSize) + stat.UID = a.uid.Load() + stat.GID = a.gid.Load() + stat.Nlink = a.nlink.Load() + stat.Blksize = a.blockSize.Load() stat.Atime = linux.NsecToStatxTimestamp(a.atime.Load()) stat.Mtime = linux.NsecToStatxTimestamp(a.mtime.Load()) stat.Ctime = linux.NsecToStatxTimestamp(a.ctime.Load()) @@ -296,29 +295,29 @@ func (a *InodeAttrs) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *aut if opts.Stat.Mask&linux.STATX_SIZE != 0 && a.Mode().IsDir() { return linuxerr.EISDIR } - if err := vfs.CheckSetStat(ctx, creds, &opts, a.Mode(), auth.KUID(atomic.LoadUint32(&a.uid)), auth.KGID(atomic.LoadUint32(&a.gid))); err != nil { + if err := vfs.CheckSetStat(ctx, creds, &opts, a.Mode(), auth.KUID(a.uid.Load()), auth.KGID(a.gid.Load())); err != nil { return err } clearSID := false stat := opts.Stat if stat.Mask&linux.STATX_UID != 0 { - atomic.StoreUint32(&a.uid, stat.UID) + a.uid.Store(stat.UID) clearSID = true } if stat.Mask&linux.STATX_GID != 0 { - atomic.StoreUint32(&a.gid, stat.GID) + a.gid.Store(stat.GID) clearSID = true } if stat.Mask&linux.STATX_MODE != 0 { for { - old := atomic.LoadUint32(&a.mode) + old := a.mode.Load() ft := old & linux.S_IFMT newMode := ft | uint32(stat.Mode & ^uint16(linux.S_IFMT)) if clearSID { newMode = vfs.ClearSUIDAndSGID(newMode) } - if swapped := atomic.CompareAndSwapUint32(&a.mode, old, newMode); swapped { + if swapped := a.mode.CompareAndSwap(old, newMode); swapped { clearSID = false break } @@ -329,9 +328,9 @@ func (a *InodeAttrs) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *aut // STATX_MODE. if clearSID { for { - old := atomic.LoadUint32(&a.mode) + old := a.mode.Load() newMode := vfs.ClearSUIDAndSGID(old) - if swapped := atomic.CompareAndSwapUint32(&a.mode, old, newMode); swapped { + if swapped := a.mode.CompareAndSwap(old, newMode); swapped { break } } @@ -360,21 +359,21 @@ func (a *InodeAttrs) CheckPermissions(_ context.Context, creds *auth.Credentials creds, ats, a.Mode(), - auth.KUID(atomic.LoadUint32(&a.uid)), - auth.KGID(atomic.LoadUint32(&a.gid)), + auth.KUID(a.uid.Load()), + auth.KGID(a.gid.Load()), ) } // IncLinks implements Inode.IncLinks. func (a *InodeAttrs) IncLinks(n uint32) { - if atomic.AddUint32(&a.nlink, n) <= n { + if a.nlink.Add(n) <= n { panic("InodeLink.IncLinks called with no existing links") } } // DecLinks implements Inode.DecLinks. func (a *InodeAttrs) DecLinks() { - if nlink := atomic.AddUint32(&a.nlink, ^uint32(0)); nlink == ^uint32(0) { + if nlink := a.nlink.Add(^uint32(0)); nlink == ^uint32(0) { // Negative overflow panic("Inode.DecLinks called at 0 links") } diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index d4afec291..163fa1459 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -57,7 +57,6 @@ package kernfs import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -221,8 +220,8 @@ type Dentry struct { fs *Filesystem // flags caches useful information about the dentry from the inode. See the - // dflags* consts above. Must be accessed by atomic ops. - flags uint32 + // dflags* consts above. + flags atomicbitops.Uint32 parent *Dentry name string @@ -459,10 +458,10 @@ func (d *Dentry) Init(fs *Filesystem, inode Inode) { d.refs.Store(1) ftype := inode.Mode().FileType() if ftype == linux.ModeDirectory { - d.flags |= dflagsIsDir + d.flags = atomicbitops.FromUint32(d.flags.RacyLoad() | dflagsIsDir) } if ftype == linux.ModeSymlink { - d.flags |= dflagsIsSymlink + d.flags = atomicbitops.FromUint32(d.flags.RacyLoad() | dflagsIsSymlink) } refsvfs2.Register(d) } @@ -474,12 +473,12 @@ func (d *Dentry) VFSDentry() *vfs.Dentry { // isDir checks whether the dentry points to a directory inode. func (d *Dentry) isDir() bool { - return atomic.LoadUint32(&d.flags)&dflagsIsDir != 0 + return d.flags.Load()&dflagsIsDir != 0 } // isSymlink checks whether the dentry points to a symlink inode. func (d *Dentry) isSymlink() bool { - return atomic.LoadUint32(&d.flags)&dflagsIsSymlink != 0 + return d.flags.Load()&dflagsIsSymlink != 0 } // InotifyWithParent implements vfs.DentryImpl.InotifyWithParent. diff --git a/pkg/sentry/fsimpl/overlay/copy_up.go b/pkg/sentry/fsimpl/overlay/copy_up.go index ec50f52f5..31dd82381 100644 --- a/pkg/sentry/fsimpl/overlay/copy_up.go +++ b/pkg/sentry/fsimpl/overlay/copy_up.go @@ -16,7 +16,6 @@ package overlay import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -29,7 +28,7 @@ import ( ) func (d *dentry) isCopiedUp() bool { - return atomic.LoadUint32(&d.copiedUp) != 0 + return d.copiedUp.Load() != 0 } // copyUpLocked ensures that d exists on the upper layer, i.e. d.upperVD.Ok(). @@ -49,7 +48,7 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy // credentials from context rather an take an explicit creds parameter. ctx = auth.ContextWithCredentials(ctx, d.fs.creds) - ftype := atomic.LoadUint32(&d.mode) & linux.S_IFMT + ftype := d.mode.Load() & linux.S_IFMT switch ftype { case linux.S_IFREG, linux.S_IFDIR, linux.S_IFLNK, linux.S_IFBLK, linux.S_IFCHR: // Can be copied-up. @@ -126,7 +125,8 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy defer oldFD.DecRef(ctx) newFD, err := vfsObj.OpenAt(ctx, d.fs.creds, &newpop, &vfs.OpenOptions{ Flags: linux.O_WRONLY | linux.O_CREAT | linux.O_EXCL, - Mode: linux.FileMode(d.mode &^ linux.S_IFMT), + // d.mode can be read because d.copyMu is locked. + Mode: linux.FileMode(d.mode.RacyLoad() &^ linux.S_IFMT), }) if err != nil { return err @@ -157,9 +157,10 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy } if err := newFD.SetStat(ctx, vfs.SetStatOptions{ Stat: linux.Statx{ - Mask: linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, - UID: d.uid, - GID: d.gid, + Mask: linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, + // d.uid and d.gid can be read because d.copyMu is locked. + UID: d.uid.RacyLoad(), + GID: d.gid.RacyLoad(), Atime: oldStat.Atime, Mtime: oldStat.Mtime, }, @@ -172,16 +173,18 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy case linux.S_IFDIR: if err := vfsObj.MkdirAt(ctx, d.fs.creds, &newpop, &vfs.MkdirOptions{ - Mode: linux.FileMode(d.mode &^ linux.S_IFMT), + // d.mode can be read because d.copyMu is locked. + Mode: linux.FileMode(d.mode.RacyLoad() &^ linux.S_IFMT), ForSyntheticMountpoint: forSyntheticMountpoint, }); err != nil { return err } if err := vfsObj.SetStatAt(ctx, d.fs.creds, &newpop, &vfs.SetStatOptions{ Stat: linux.Statx{ - Mask: linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, - UID: d.uid, - GID: d.gid, + Mask: linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, + // d.uid and d.gid can be read because d.copyMu is locked. + UID: d.uid.RacyLoad(), + GID: d.gid.RacyLoad(), Atime: oldStat.Atime, Mtime: oldStat.Mtime, }, @@ -206,10 +209,11 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy } if err := vfsObj.SetStatAt(ctx, d.fs.creds, &newpop, &vfs.SetStatOptions{ Stat: linux.Statx{ - Mask: linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, - Mode: uint16(d.mode), - UID: d.uid, - GID: d.gid, + Mask: linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, + // d.{uid,gid,mode} can be read because d.copyMu is locked. + Mode: uint16(d.mode.RacyLoad()), + UID: d.uid.RacyLoad(), + GID: d.gid.RacyLoad(), Atime: oldStat.Atime, Mtime: oldStat.Mtime, }, @@ -226,7 +230,8 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy case linux.S_IFBLK, linux.S_IFCHR: if err := vfsObj.MknodAt(ctx, d.fs.creds, &newpop, &vfs.MknodOptions{ - Mode: linux.FileMode(d.mode), + // d.mode can be read because d.copyMu is locked. + Mode: linux.FileMode(d.mode.RacyLoad()), DevMajor: oldStat.RdevMajor, DevMinor: oldStat.RdevMinor, }); err != nil { @@ -234,9 +239,10 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy } if err := vfsObj.SetStatAt(ctx, d.fs.creds, &newpop, &vfs.SetStatOptions{ Stat: linux.Statx{ - Mask: linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, - UID: d.uid, - GID: d.gid, + Mask: linux.STATX_UID | linux.STATX_GID | oldStat.Mask×tampsMask, + // d.uid and d.gid can be read because d.copyMu is locked. + UID: d.uid.RacyLoad(), + GID: d.gid.RacyLoad(), Atime: oldStat.Atime, Mtime: oldStat.Mtime, }, @@ -278,8 +284,8 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy cleanupUndoCopyUp() return linuxerr.EREMOTE } - atomic.StoreUint32(&d.devMajor, upperStat.DevMajor) - atomic.StoreUint32(&d.devMinor, upperStat.DevMinor) + d.devMajor.Store(upperStat.DevMajor) + d.devMinor.Store(upperStat.DevMinor) d.ino.Store(upperStat.Ino) } @@ -339,7 +345,7 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy d.lowerMappings.RemoveAll() } - atomic.StoreUint32(&d.copiedUp, 1) + d.copiedUp.Store(1) return nil } diff --git a/pkg/sentry/fsimpl/overlay/directory.go b/pkg/sentry/fsimpl/overlay/directory.go index d47eab1ab..1a2a42681 100644 --- a/pkg/sentry/fsimpl/overlay/directory.go +++ b/pkg/sentry/fsimpl/overlay/directory.go @@ -15,8 +15,6 @@ package overlay import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -26,7 +24,7 @@ import ( ) func (d *dentry) isDir() bool { - return atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFDIR + return d.mode.Load()&linux.S_IFMT == linux.S_IFDIR } // Preconditions: @@ -168,7 +166,7 @@ func (d *dentry) getDirentsLocked(ctx context.Context) ([]vfs.Dirent, error) { }, { Name: "..", - Type: uint8(atomic.LoadUint32(&parent.mode) >> 12), + Type: uint8(parent.mode.Load() >> 12), Ino: parent.ino.Load(), NextOff: 2, }, diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index e27bc3020..d8bc1c003 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -17,7 +17,6 @@ package overlay import ( "fmt" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -272,7 +271,7 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str childVD.IncRef() if isUpper { child.upperVD = childVD - child.copiedUp = 1 + child.copiedUp = atomicbitops.FromUint32(1) } else { child.lowerVDs = append(child.lowerVDs, childVD) } @@ -282,11 +281,11 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str } else { topLookupLayer = lookupLayerLower } - child.mode = uint32(stat.Mode) - child.uid = stat.UID - child.gid = stat.GID - child.devMajor = stat.DevMajor - child.devMinor = stat.DevMinor + child.mode = atomicbitops.FromUint32(uint32(stat.Mode)) + child.uid = atomicbitops.FromUint32(stat.UID) + child.gid = atomicbitops.FromUint32(stat.GID) + child.devMajor = atomicbitops.FromUint32(stat.DevMajor) + child.devMinor = atomicbitops.FromUint32(stat.DevMinor) child.ino = atomicbitops.FromUint64(stat.Ino) } @@ -319,14 +318,15 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str // Device and inode numbers were copied from the topmost layer above. Remap // the device number to an appropriate overlay-private one. - childDevMinor, err := fs.getPrivateDevMinor(child.devMajor, child.devMinor) + // We can use RacyLoad() because child is still being initialized. + childDevMinor, err := fs.getPrivateDevMinor(child.devMajor.RacyLoad(), child.devMinor.RacyLoad()) if err != nil { - ctx.Infof("overlay.filesystem.lookupLocked: failed to map layer device number (%d, %d) to an overlay-specific device number: %v", child.devMajor, child.devMinor, err) + ctx.Infof("overlay.filesystem.lookupLocked: failed to map layer device number (%d, %d) to an overlay-specific device number: %v", child.devMajor.RacyLoad(), child.devMinor.RacyLoad(), err) child.destroyLocked(ctx) return nil, topLookupLayer, err } - child.devMajor = linux.UNNAMED_MAJOR - child.devMinor = childDevMinor + child.devMajor = atomicbitops.FromUint32(linux.UNNAMED_MAJOR) + child.devMinor = atomicbitops.FromUint32(childDevMinor) parent.IncRef() child.parent = parent @@ -887,7 +887,7 @@ func (d *dentry) openCopiedUp(ctx context.Context, rp *vfs.ResolvingPath, opts * // Directory FDs open FDs from each layer when directory entries are read, // so they don't require opening an FD from d.topLayer() up front. - ftype := atomic.LoadUint32(&d.mode) & linux.S_IFMT + ftype := d.mode.Load() & linux.S_IFMT if ftype == linux.S_IFDIR { // Can't open directories with O_CREAT. if opts.Flags&linux.O_CREAT != 0 { @@ -1440,8 +1440,8 @@ func (fs *filesystem) SetStatAt(ctx context.Context, rp *vfs.ResolvingPath, opts // Precondition: d.fs.renameMu must be held for reading. func (d *dentry) setStatLocked(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.SetStatOptions) error { - mode := linux.FileMode(atomic.LoadUint32(&d.mode)) - if err := vfs.CheckSetStat(ctx, rp.Credentials(), &opts, mode, auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid))); err != nil { + mode := linux.FileMode(d.mode.Load()) + if err := vfs.CheckSetStat(ctx, rp.Credentials(), &opts, mode, auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { return err } mnt := rp.Mount() diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index 74079a18e..da5ea1d48 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -36,7 +36,6 @@ package overlay import ( "fmt" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -254,7 +253,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt root.refs = atomicbitops.FromInt64(1) if fs.opts.UpperRoot.Ok() { fs.opts.UpperRoot.IncRef() - root.copiedUp = 1 + root.copiedUp = atomicbitops.FromUint32(1) root.upperVD = fs.opts.UpperRoot } for _, lowerRoot := range fs.opts.LowerRoots { @@ -286,10 +285,10 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt fs.vfsfs.DecRef(ctx) return nil, nil, linuxerr.EINVAL } - root.mode = uint32(rootStat.Mode) - root.uid = rootStat.UID - root.gid = rootStat.GID - root.devMajor = linux.UNNAMED_MAJOR + root.mode = atomicbitops.FromUint32(uint32(rootStat.Mode)) + root.uid = atomicbitops.FromUint32(rootStat.UID) + root.gid = atomicbitops.FromUint32(rootStat.GID) + root.devMajor = atomicbitops.FromUint32(linux.UNNAMED_MAJOR) rootDevMinor, err := fs.getPrivateDevMinor(rootStat.DevMajor, rootStat.DevMinor) if err != nil { ctx.Infof("overlay.FilesystemType.GetFilesystem: failed to get device number for root: %v", err) @@ -297,7 +296,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt fs.vfsfs.DecRef(ctx) return nil, nil, err } - root.devMinor = rootDevMinor + root.devMinor = atomicbitops.FromUint32(rootDevMinor) root.ino.Store(rootStat.Ino) return &fs.vfsfs, &root.vfsd, nil @@ -386,14 +385,14 @@ type dentry struct { // mode, uid, and gid are the file mode, owner, and group of the file in // the topmost layer (and therefore the overlay file as well), and are used // for permission checks on this dentry. These fields are protected by - // copyMu and accessed using atomic memory operations. - mode uint32 - uid uint32 - gid uint32 + // copyMu. + mode atomicbitops.Uint32 + uid atomicbitops.Uint32 + gid atomicbitops.Uint32 // copiedUp is 1 if this dentry has been copied-up (i.e. upperVD.Ok()) and - // 0 otherwise. copiedUp is accessed using atomic memory operations. - copiedUp uint32 + // 0 otherwise. + copiedUp atomicbitops.Uint32 // parent is the dentry corresponding to this dentry's parent directory. // name is this dentry's name in parent. If this dentry is a filesystem @@ -426,10 +425,9 @@ type dentry struct { inlineLowerVDs [1]vfs.VirtualDentry // devMajor, devMinor, and ino are the device major/minor and inode numbers - // used by this dentry. These fields are protected by copyMu and accessed - // using atomic memory operations. - devMajor uint32 - devMinor uint32 + // used by this dentry. These fields are protected by copyMu. + devMajor atomicbitops.Uint32 + devMinor atomicbitops.Uint32 ino atomicbitops.Uint64 // If this dentry represents a regular file, then: @@ -459,7 +457,7 @@ type dentry struct { lowerMappings memmap.MappingSet dataMu sync.RWMutex `state:"nosave"` wrappedMappable memmap.Mappable - isMappable uint32 + isMappable atomicbitops.Uint32 locks vfs.FileLocks @@ -688,13 +686,13 @@ func (d *dentry) topLookupLayer() lookupLayer { } func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) error { - return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(atomic.LoadUint32(&d.mode)), auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid))) + return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())) } func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats vfs.AccessTypes) error { - mode := linux.FileMode(atomic.LoadUint32(&d.mode)) - kuid := auth.KUID(atomic.LoadUint32(&d.uid)) - kgid := auth.KGID(atomic.LoadUint32(&d.gid)) + mode := linux.FileMode(d.mode.Load()) + kuid := auth.KUID(d.uid.Load()) + kgid := auth.KGID(d.gid.Load()) if err := vfs.GenericCheckPermissions(creds, ats, mode, kuid, kgid); err != nil { return err } @@ -716,34 +714,34 @@ func (d *dentry) statInternalTo(ctx context.Context, opts *vfs.StatOptions, stat // and some of our tests expect this. stat.Nlink = 2 } - stat.UID = atomic.LoadUint32(&d.uid) - stat.GID = atomic.LoadUint32(&d.gid) - stat.Mode = uint16(atomic.LoadUint32(&d.mode)) + stat.UID = d.uid.Load() + stat.GID = d.gid.Load() + stat.Mode = uint16(d.mode.Load()) stat.Ino = d.ino.Load() - stat.DevMajor = atomic.LoadUint32(&d.devMajor) - stat.DevMinor = atomic.LoadUint32(&d.devMinor) + stat.DevMajor = d.devMajor.Load() + stat.DevMinor = d.devMinor.Load() } // Preconditions: d.copyMu must be locked for writing. func (d *dentry) updateAfterSetStatLocked(opts *vfs.SetStatOptions) { if opts.Stat.Mask&linux.STATX_MODE != 0 { - atomic.StoreUint32(&d.mode, (d.mode&linux.S_IFMT)|uint32(opts.Stat.Mode&^linux.S_IFMT)) + d.mode.Store((d.mode.RacyLoad() & linux.S_IFMT) | uint32(opts.Stat.Mode&^linux.S_IFMT)) } if opts.Stat.Mask&linux.STATX_UID != 0 { - atomic.StoreUint32(&d.uid, opts.Stat.UID) + d.uid.Store(opts.Stat.UID) } if opts.Stat.Mask&linux.STATX_GID != 0 { - atomic.StoreUint32(&d.gid, opts.Stat.GID) + d.gid.Store(opts.Stat.GID) } } func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error { return vfs.CheckDeleteSticky( creds, - linux.FileMode(atomic.LoadUint32(&d.mode)), - auth.KUID(atomic.LoadUint32(&d.uid)), - auth.KUID(atomic.LoadUint32(&child.uid)), - auth.KGID(atomic.LoadUint32(&child.gid)), + linux.FileMode(d.mode.Load()), + auth.KUID(d.uid.Load()), + auth.KUID(child.uid.Load()), + auth.KGID(child.gid.Load()), ) } @@ -758,8 +756,8 @@ func (d *dentry) newChildOwnerStat(mode linux.FileMode, creds *auth.Credentials) // Set GID and possibly the SGID bit if the parent is an SGID directory. d.copyMu.RLock() defer d.copyMu.RUnlock() - if atomic.LoadUint32(&d.mode)&linux.ModeSetGID == linux.ModeSetGID { - stat.GID = atomic.LoadUint32(&d.gid) + if d.mode.Load()&linux.ModeSetGID == linux.ModeSetGID { + stat.GID = d.gid.Load() if stat.Mode&linux.ModeDirectory == linux.ModeDirectory { stat.Mode = uint16(mode) | linux.ModeSetGID stat.Mask |= linux.STATX_MODE diff --git a/pkg/sentry/fsimpl/overlay/regular_file.go b/pkg/sentry/fsimpl/overlay/regular_file.go index 3d3aab6dc..54204f0b8 100644 --- a/pkg/sentry/fsimpl/overlay/regular_file.go +++ b/pkg/sentry/fsimpl/overlay/regular_file.go @@ -15,8 +15,6 @@ package overlay import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -32,11 +30,11 @@ import ( ) func (d *dentry) isRegularFile() bool { - return atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFREG + return d.mode.Load()&linux.S_IFMT == linux.S_IFREG } func (d *dentry) isSymlink() bool { - return atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFLNK + return d.mode.Load()&linux.S_IFMT == linux.S_IFLNK } func (d *dentry) readlink(ctx context.Context) (string, error) { @@ -169,8 +167,8 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint // SetStat implements vfs.FileDescriptionImpl.SetStat. func (fd *regularFileFD) SetStat(ctx context.Context, opts vfs.SetStatOptions) error { d := fd.dentry() - mode := linux.FileMode(atomic.LoadUint32(&d.mode)) - if err := vfs.CheckSetStat(ctx, auth.CredentialsFromContext(ctx), &opts, mode, auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid))); err != nil { + mode := linux.FileMode(d.mode.Load()) + if err := vfs.CheckSetStat(ctx, auth.CredentialsFromContext(ctx), &opts, mode, auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { return err } mnt := fd.vfsfd.Mount() @@ -332,7 +330,7 @@ func (fd *regularFileFD) updateSetUserGroupIDs(ctx context.Context, wrappedFD *v // Writing can clear the setuid and/or setgid bits. We only have to // check this if something was written and one of those bits was set. dentry := fd.dentry() - if written == 0 || atomic.LoadUint32(&dentry.mode)&(linux.S_ISUID|linux.S_ISGID) == 0 { + if written == 0 || dentry.mode.Load()&(linux.S_ISUID|linux.S_ISGID) == 0 { return written, nil } stat, err := wrappedFD.Stat(ctx, vfs.StatOptions{Mask: linux.STATX_MODE}) @@ -341,7 +339,7 @@ func (fd *regularFileFD) updateSetUserGroupIDs(ctx context.Context, wrappedFD *v } dentry.copyMu.Lock() defer dentry.copyMu.Unlock() - atomic.StoreUint32(&dentry.mode, uint32(stat.Mode)) + dentry.mode.Store(uint32(stat.Mode)) return written, nil } @@ -398,13 +396,13 @@ func (fd *regularFileFD) ensureMappable(ctx context.Context, opts *memmap.MMapOp d := fd.dentry() // Fast path if we already have a Mappable for the current top layer. - if atomic.LoadUint32(&d.isMappable) != 0 { + if d.isMappable.Load() != 0 { return nil } // Only permit mmap of regular files, since other file types may have // unpredictable behavior when mmapped (e.g. /dev/zero). - if atomic.LoadUint32(&d.mode)&linux.S_IFMT != linux.S_IFREG { + if d.mode.Load()&linux.S_IFMT != linux.S_IFREG { return linuxerr.ENODEV } @@ -413,7 +411,7 @@ func (fd *regularFileFD) ensureMappable(ctx context.Context, opts *memmap.MMapOp defer fd.mu.Unlock() d.copyMu.RLock() defer d.copyMu.RUnlock() - if atomic.LoadUint32(&d.isMappable) != 0 { + if d.isMappable.Load() != 0 { return nil } wrappedFD, err := fd.currentFDLocked(ctx) @@ -435,7 +433,7 @@ func (fd *regularFileFD) ensureMappable(ctx context.Context, opts *memmap.MMapOp defer d.dataMu.Unlock() if d.wrappedMappable == nil { d.wrappedMappable = opts.Mappable - atomic.StoreUint32(&d.isMappable, 1) + d.isMappable.Store(1) } return nil } diff --git a/pkg/sentry/fsimpl/proc/yama.go b/pkg/sentry/fsimpl/proc/yama.go index 072ed1cbf..cc05061a1 100644 --- a/pkg/sentry/fsimpl/proc/yama.go +++ b/pkg/sentry/fsimpl/proc/yama.go @@ -17,9 +17,9 @@ package proc import ( "bytes" "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -44,14 +44,14 @@ type yamaPtraceScope struct { kernfs.DynamicBytesFile // level is the ptrace_scope level. - level *int32 + level *atomicbitops.Int32 } var _ vfs.WritableDynamicBytesSource = (*yamaPtraceScope)(nil) // Generate implements vfs.DynamicBytesSource.Generate. func (s *yamaPtraceScope) Generate(ctx context.Context, buf *bytes.Buffer) error { - _, err := fmt.Fprintf(buf, "%d\n", atomic.LoadInt32(s.level)) + _, err := fmt.Fprintf(buf, "%d\n", s.level.Load()) return err } @@ -79,6 +79,6 @@ func (s *yamaPtraceScope) Write(ctx context.Context, _ *vfs.FileDescription, src return 0, linuxerr.EINVAL } - atomic.StoreInt32(s.level, v) + s.level.Store(v) return n, nil } diff --git a/pkg/sentry/fsimpl/tmpfs/device_file.go b/pkg/sentry/fsimpl/tmpfs/device_file.go index 616ae6730..617bcc0ff 100644 --- a/pkg/sentry/fsimpl/tmpfs/device_file.go +++ b/pkg/sentry/fsimpl/tmpfs/device_file.go @@ -18,6 +18,7 @@ import ( "fmt" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/vfs" ) @@ -45,6 +46,6 @@ func (fs *filesystem) newDeviceFile(kuid auth.KUID, kgid auth.KGID, mode linux.F panic(fmt.Sprintf("invalid DeviceKind: %v", kind)) } file.inode.init(file, fs, kuid, kgid, mode, parentDir) - file.inode.nlink = 1 // from parent directory + file.inode.nlink = atomicbitops.FromUint32(1) // from parent directory return &file.inode } diff --git a/pkg/sentry/fsimpl/tmpfs/directory.go b/pkg/sentry/fsimpl/tmpfs/directory.go index 2848d1019..6d20d69cb 100644 --- a/pkg/sentry/fsimpl/tmpfs/directory.go +++ b/pkg/sentry/fsimpl/tmpfs/directory.go @@ -15,8 +15,6 @@ package tmpfs import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" @@ -53,7 +51,7 @@ type directory struct { func (fs *filesystem) newDirectory(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, parentDir *directory) *directory { dir := &directory{} dir.inode.init(dir, fs, kuid, kgid, linux.S_IFDIR|mode, parentDir) - dir.inode.nlink = 2 // from "." and parent directory or ".." for root + dir.inode.nlink = atomicbitops.FromUint32(2) // from "." and parent directory or ".." for root dir.dentry.inode = &dir.inode dir.dentry.vfsd.Init(&dir.dentry) return dir @@ -87,10 +85,10 @@ func (dir *directory) removeChildLocked(child *dentry) { func (dir *directory) mayDelete(creds *auth.Credentials, child *dentry) error { return vfs.CheckDeleteSticky( creds, - linux.FileMode(atomic.LoadUint32(&dir.inode.mode)), - auth.KUID(atomic.LoadUint32(&dir.inode.uid)), - auth.KUID(atomic.LoadUint32(&child.inode.uid)), - auth.KGID(atomic.LoadUint32(&child.inode.gid)), + linux.FileMode(dir.inode.mode.Load()), + auth.KUID(dir.inode.uid.Load()), + auth.KUID(child.inode.uid.Load()), + auth.KGID(child.inode.gid.Load()), ) } diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index 5fff5709e..03a086e57 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -16,7 +16,6 @@ package tmpfs import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -270,13 +269,13 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. if i.isDir() { return linuxerr.EPERM } - if err := vfs.MayLink(auth.CredentialsFromContext(ctx), linux.FileMode(atomic.LoadUint32(&i.mode)), auth.KUID(atomic.LoadUint32(&i.uid)), auth.KGID(atomic.LoadUint32(&i.gid))); err != nil { + if err := vfs.MayLink(auth.CredentialsFromContext(ctx), linux.FileMode(i.mode.Load()), auth.KUID(i.uid.Load()), auth.KGID(i.gid.Load())); err != nil { return err } - if i.nlink == 0 { + if i.nlink.Load() == 0 { return linuxerr.ENOENT } - if i.nlink == maxLinks { + if i.nlink.Load() == maxLinks { return linuxerr.EMLINK } i.incLinksLocked() @@ -290,7 +289,7 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MkdirOptions) error { return fs.doCreateAt(ctx, rp, true /* dir */, func(parentDir *directory, name string) error { creds := rp.Credentials() - if parentDir.inode.nlink == maxLinks { + if parentDir.inode.nlink.Load() == maxLinks { return linuxerr.EMLINK } parentDir.inode.incLinksLocked() // from child's ".." @@ -597,7 +596,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } else { - if renamed.inode.isDir() && newParentDir.inode.nlink == maxLinks { + if renamed.inode.isDir() && newParentDir.inode.nlink.Load() == maxLinks { return linuxerr.EMLINK } } diff --git a/pkg/sentry/fsimpl/tmpfs/named_pipe.go b/pkg/sentry/fsimpl/tmpfs/named_pipe.go index 65c2cbf86..9cd55496d 100644 --- a/pkg/sentry/fsimpl/tmpfs/named_pipe.go +++ b/pkg/sentry/fsimpl/tmpfs/named_pipe.go @@ -16,6 +16,7 @@ package tmpfs import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/kernel/pipe" ) @@ -33,6 +34,6 @@ type namedPipe struct { func (fs *filesystem) newNamedPipe(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, parentDir *directory) *inode { file := &namedPipe{pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize)} file.inode.init(file, fs, kuid, kgid, linux.S_IFIFO|mode, parentDir) - file.inode.nlink = 1 // Only the parent has a link. + file.inode.nlink = atomicbitops.FromUint32(1) // Only the parent has a link. return &file.inode } diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index ef2fc073d..e6c7d8762 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -18,7 +18,6 @@ import ( "fmt" "io" "math" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -100,7 +99,7 @@ func (fs *filesystem) newRegularFile(kuid auth.KUID, kgid auth.KGID, mode linux. seals: linux.F_SEAL_SEAL, } file.inode.init(file, fs, kuid, kgid, linux.S_IFREG|mode, parentDir) - file.inode.nlink = 1 // from parent directory + file.inode.nlink = atomicbitops.FromUint32(1) // from parent directory return &file.inode } @@ -468,9 +467,9 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off f.inode.touchCMtimeLocked() for { - old := atomic.LoadUint32(&f.inode.mode) + old := f.inode.mode.Load() new := vfs.ClearSUIDAndSGID(old) - if swapped := atomic.CompareAndSwapUint32(&f.inode.mode, old, new); swapped { + if swapped := f.inode.mode.CompareAndSwap(old, new); swapped { break } } diff --git a/pkg/sentry/fsimpl/tmpfs/socket_file.go b/pkg/sentry/fsimpl/tmpfs/socket_file.go index 6112279b0..5970da8b3 100644 --- a/pkg/sentry/fsimpl/tmpfs/socket_file.go +++ b/pkg/sentry/fsimpl/tmpfs/socket_file.go @@ -16,6 +16,7 @@ package tmpfs import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/socket/unix/transport" ) @@ -31,6 +32,6 @@ type socketFile struct { func (fs *filesystem) newSocketFile(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, ep transport.BoundEndpoint, parentDir *directory) *inode { file := &socketFile{ep: ep} file.inode.init(file, fs, kuid, kgid, mode, parentDir) - file.inode.nlink = 1 // from parent directory + file.inode.nlink = atomicbitops.FromUint32(1) // from parent directory return &file.inode } diff --git a/pkg/sentry/fsimpl/tmpfs/symlink.go b/pkg/sentry/fsimpl/tmpfs/symlink.go index 6a83296bc..da6360dbe 100644 --- a/pkg/sentry/fsimpl/tmpfs/symlink.go +++ b/pkg/sentry/fsimpl/tmpfs/symlink.go @@ -16,6 +16,7 @@ package tmpfs import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" ) @@ -30,7 +31,7 @@ func (fs *filesystem) newSymlink(kuid auth.KUID, kgid auth.KGID, mode linux.File target: target, } link.inode.init(link, fs, kuid, kgid, linux.S_IFLNK|mode, parentDir) - link.inode.nlink = 1 // from parent directory + link.inode.nlink = atomicbitops.FromUint32(1) // from parent directory return &link.inode } diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index deeac78e0..bcc0cc924 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -33,7 +33,6 @@ import ( "math" "strconv" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -435,12 +434,12 @@ type inode struct { // Inode metadata. Writing multiple fields atomically requires holding // mu, othewise atomic operations can be used. - mu sync.Mutex `state:"nosave"` - mode uint32 // file type and mode - nlink uint32 // protected by filesystem.mu instead of inode.mu - uid uint32 // auth.KUID, but stored as raw uint32 for sync/atomic - gid uint32 // auth.KGID, but ... - ino uint64 // immutable + mu sync.Mutex `state:"nosave"` + mode atomicbitops.Uint32 // file type and mode + nlink atomicbitops.Uint32 // protected by filesystem.mu instead of inode.mu + uid atomicbitops.Uint32 // auth.KUID, but stored as raw uint32 for sync/atomic + gid atomicbitops.Uint32 // auth.KGID, but ... + ino uint64 // immutable // Linux's tmpfs has no concept of btime. atime atomicbitops.Int64 // nanoseconds @@ -463,17 +462,17 @@ func (i *inode) init(impl interface{}, fs *filesystem, kuid auth.KUID, kgid auth } // Inherit the group and setgid bit as in fs/inode.c:inode_init_owner(). - if parentDir != nil && atomic.LoadUint32(&parentDir.inode.mode)&linux.S_ISGID == linux.S_ISGID { - kgid = auth.KGID(atomic.LoadUint32(&parentDir.inode.gid)) + if parentDir != nil && parentDir.inode.mode.Load()&linux.S_ISGID == linux.S_ISGID { + kgid = auth.KGID(parentDir.inode.gid.Load()) if mode&linux.S_IFDIR == linux.S_IFDIR { mode |= linux.S_ISGID } } i.fs = fs - i.mode = uint32(mode) - i.uid = uint32(kuid) - i.gid = uint32(kgid) + i.mode = atomicbitops.FromUint32(uint32(mode)) + i.uid = atomicbitops.FromUint32(uint32(kuid)) + i.gid = atomicbitops.FromUint32(uint32(kgid)) i.ino = fs.nextInoMinusOne.Add(1) // Tmpfs creation sets atime, ctime, and mtime to current time. now := fs.clock.Now().Nanoseconds() @@ -489,16 +488,17 @@ func (i *inode) init(impl interface{}, fs *filesystem, kuid auth.KUID, kgid auth // // Preconditions: // * filesystem.mu must be locked for writing. +// * i.mu must be lcoked. // * i.nlink != 0. // * i.nlink < maxLinks. func (i *inode) incLinksLocked() { - if i.nlink == 0 { + if i.nlink.RacyLoad() == 0 { panic("tmpfs.inode.incLinksLocked() called with no existing links") } - if i.nlink == maxLinks { + if i.nlink.RacyLoad() == maxLinks { panic("tmpfs.inode.incLinksLocked() called with maximum link count") } - atomic.AddUint32(&i.nlink, 1) + i.nlink.Add(1) } // decLinksLocked decrements i's link count. If the link count reaches 0, we @@ -506,12 +506,13 @@ func (i *inode) incLinksLocked() { // // Preconditions: // * filesystem.mu must be locked for writing. +// * i.mu must be lcoked. // * i.nlink != 0. func (i *inode) decLinksLocked(ctx context.Context) { - if i.nlink == 0 { + if i.nlink.RacyLoad() == 0 { panic("tmpfs.inode.decLinksLocked() called with no existing links") } - if atomic.AddUint32(&i.nlink, ^uint32(0)) == 0 { + if i.nlink.Add(^uint32(0)) == 0 { i.decRef(ctx) } } @@ -537,8 +538,8 @@ func (i *inode) decRef(ctx context.Context) { } func (i *inode) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) error { - mode := linux.FileMode(atomic.LoadUint32(&i.mode)) - return vfs.GenericCheckPermissions(creds, ats, mode, auth.KUID(atomic.LoadUint32(&i.uid)), auth.KGID(atomic.LoadUint32(&i.gid))) + mode := linux.FileMode(i.mode.Load()) + return vfs.GenericCheckPermissions(creds, ats, mode, auth.KUID(i.uid.Load()), auth.KGID(i.gid.Load())) } // Go won't inline this function, and returning linux.Statx (which is quite @@ -553,10 +554,10 @@ func (i *inode) statTo(stat *linux.Statx) { linux.STATX_BLOCKS | linux.STATX_ATIME | linux.STATX_CTIME | linux.STATX_MTIME stat.Blksize = hostarch.PageSize - stat.Nlink = atomic.LoadUint32(&i.nlink) - stat.UID = atomic.LoadUint32(&i.uid) - stat.GID = atomic.LoadUint32(&i.gid) - stat.Mode = uint16(atomic.LoadUint32(&i.mode)) + stat.Nlink = i.nlink.Load() + stat.UID = i.uid.Load() + stat.GID = i.gid.Load() + stat.Mode = uint16(i.mode.Load()) stat.Ino = i.ino stat.Atime = linux.NsecToStatxTimestamp(i.atime.Load()) stat.Ctime = linux.NsecToStatxTimestamp(i.ctime.Load()) @@ -595,8 +596,8 @@ func (i *inode) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs. if stat.Mask&^(linux.STATX_MODE|linux.STATX_UID|linux.STATX_GID|linux.STATX_ATIME|linux.STATX_MTIME|linux.STATX_CTIME|linux.STATX_SIZE) != 0 { return linuxerr.EPERM } - mode := linux.FileMode(atomic.LoadUint32(&i.mode)) - if err := vfs.CheckSetStat(ctx, creds, opts, mode, auth.KUID(atomic.LoadUint32(&i.uid)), auth.KGID(atomic.LoadUint32(&i.gid))); err != nil { + mode := linux.FileMode(i.mode.Load()) + if err := vfs.CheckSetStat(ctx, creds, opts, mode, auth.KUID(i.uid.Load()), auth.KGID(i.gid.Load())); err != nil { return err } @@ -627,24 +628,24 @@ func (i *inode) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs. } } if mask&linux.STATX_UID != 0 { - atomic.StoreUint32(&i.uid, stat.UID) + i.uid.Store(stat.UID) needsCtimeBump = true clearSID = true } if mask&linux.STATX_GID != 0 { - atomic.StoreUint32(&i.gid, stat.GID) + i.gid.Store(stat.GID) needsCtimeBump = true clearSID = true } if mask&linux.STATX_MODE != 0 { for { - old := atomic.LoadUint32(&i.mode) + old := i.mode.Load() ft := old & linux.S_IFMT newMode := ft | uint32(stat.Mode & ^uint16(linux.S_IFMT)) if clearSID { newMode = vfs.ClearSUIDAndSGID(newMode) } - if swapped := atomic.CompareAndSwapUint32(&i.mode, old, newMode); swapped { + if swapped := i.mode.CompareAndSwap(old, newMode); swapped { clearSID = false break } @@ -684,9 +685,9 @@ func (i *inode) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs. // STATX_MODE. if clearSID { for { - old := atomic.LoadUint32(&i.mode) + old := i.mode.Load() newMode := vfs.ClearSUIDAndSGID(old) - if swapped := atomic.CompareAndSwapUint32(&i.mode, old, newMode); swapped { + if swapped := i.mode.CompareAndSwap(old, newMode); swapped { break } } @@ -739,7 +740,7 @@ func (i *inode) direntType() uint8 { } func (i *inode) isDir() bool { - mode := linux.FileMode(atomic.LoadUint32(&i.mode)) + mode := linux.FileMode(i.mode.Load()) return mode.FileType() == linux.S_IFDIR } @@ -807,9 +808,9 @@ func (i *inode) getXattr(creds *auth.Credentials, opts *vfs.GetXattrOptions) (st if err := checkXattrName(opts.Name); err != nil { return "", err } - mode := linux.FileMode(atomic.LoadUint32(&i.mode)) - kuid := auth.KUID(atomic.LoadUint32(&i.uid)) - kgid := auth.KGID(atomic.LoadUint32(&i.gid)) + mode := linux.FileMode(i.mode.Load()) + kuid := auth.KUID(i.uid.Load()) + kgid := auth.KGID(i.gid.Load()) if err := vfs.GenericCheckPermissions(creds, vfs.MayRead, mode, kuid, kgid); err != nil { return "", err } @@ -820,9 +821,9 @@ func (i *inode) setXattr(creds *auth.Credentials, opts *vfs.SetXattrOptions) err if err := checkXattrName(opts.Name); err != nil { return err } - mode := linux.FileMode(atomic.LoadUint32(&i.mode)) - kuid := auth.KUID(atomic.LoadUint32(&i.uid)) - kgid := auth.KGID(atomic.LoadUint32(&i.gid)) + mode := linux.FileMode(i.mode.Load()) + kuid := auth.KUID(i.uid.Load()) + kgid := auth.KGID(i.gid.Load()) if err := vfs.GenericCheckPermissions(creds, vfs.MayWrite, mode, kuid, kgid); err != nil { return err } @@ -833,9 +834,9 @@ func (i *inode) removeXattr(creds *auth.Credentials, name string) error { if err := checkXattrName(name); err != nil { return err } - mode := linux.FileMode(atomic.LoadUint32(&i.mode)) - kuid := auth.KUID(atomic.LoadUint32(&i.uid)) - kgid := auth.KGID(atomic.LoadUint32(&i.gid)) + mode := linux.FileMode(i.mode.Load()) + kuid := auth.KUID(i.uid.Load()) + kgid := auth.KGID(i.gid.Load()) if err := vfs.GenericCheckPermissions(creds, vfs.MayWrite, mode, kuid, kgid); err != nil { return err } diff --git a/pkg/sentry/fsimpl/verity/filesystem.go b/pkg/sentry/fsimpl/verity/filesystem.go index 8b059aa7d..d1089d83d 100644 --- a/pkg/sentry/fsimpl/verity/filesystem.go +++ b/pkg/sentry/fsimpl/verity/filesystem.go @@ -21,9 +21,9 @@ import ( "io" "strconv" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -408,7 +408,7 @@ func (fs *filesystem) verifyStatAndChildrenLocked(ctx context.Context, d *dentry DataAndTreeInSameFile: false, } d.hashMu.RUnlock() - if atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFDIR { + if d.mode.Load()&linux.S_IFMT == linux.S_IFDIR { params.DataAndTreeInSameFile = true } @@ -426,9 +426,9 @@ func (fs *filesystem) verifyStatAndChildrenLocked(ctx context.Context, d *dentry if _, err := merkletree.Verify(params); err != nil && err != io.EOF { return fs.alertIntegrityViolation(fmt.Sprintf("Verification stat for %s failed: %v", childPath, err)) } - d.mode = uint32(stat.Mode) - d.uid = stat.UID - d.gid = stat.GID + d.mode.Store(uint32(stat.Mode)) + d.uid.Store(stat.UID) + d.gid.Store(stat.GID) d.size = uint32(size) d.symlinkTarget = params.SymlinkTarget return nil @@ -604,9 +604,9 @@ func (fs *filesystem) lookupAndVerifyLocked(ctx context.Context, parent *dentry, child.name = name - child.mode = uint32(stat.Mode) - child.uid = stat.UID - child.gid = stat.GID + child.mode = atomicbitops.FromUint32(uint32(stat.Mode)) + child.uid = atomicbitops.FromUint32(stat.UID) + child.gid = atomicbitops.FromUint32(stat.GID) child.childrenNames = make(map[string]struct{}) // Verify child hash. This should always be performed unless in diff --git a/pkg/sentry/fsimpl/verity/verity.go b/pkg/sentry/fsimpl/verity/verity.go index 297caf566..c77316386 100644 --- a/pkg/sentry/fsimpl/verity/verity.go +++ b/pkg/sentry/fsimpl/verity/verity.go @@ -44,7 +44,6 @@ import ( "sort" "strconv" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -217,9 +216,8 @@ type filesystem struct { // enabled the same time. verityMu sync.RWMutex `state:"nosave"` - // released is nonzero once filesystem.Release has been called. It is accessed - // with atomic memory operations. - released int32 + // released is nonzero once filesystem.Release has been called. + released atomicbitops.Int32 } // InternalFilesystemOptions may be passed as @@ -462,9 +460,9 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return nil, nil, err } - d.mode = uint32(stat.Mode) - d.uid = stat.UID - d.gid = stat.GID + d.mode = atomicbitops.FromUint32(uint32(stat.Mode)) + d.uid = atomicbitops.FromUint32(stat.UID) + d.gid = atomicbitops.FromUint32(stat.GID) d.childrenNames = make(map[string]struct{}) d.hashMu.Lock() @@ -555,7 +553,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // Release implements vfs.FilesystemImpl.Release. func (fs *filesystem) Release(ctx context.Context) { - atomic.StoreInt32(&fs.released, 1) + fs.released.Store(1) fs.lowerMount.DecRef(ctx) fs.renameMu.Lock() @@ -591,9 +589,9 @@ type dentry struct { // mode, uid, gid and size are the file mode, owner, group, and size of // the file in the underlying file system. They are set when a dentry // is initialized, and never modified. - mode uint32 - uid uint32 - gid uint32 + mode atomicbitops.Uint32 + uid atomicbitops.Uint32 + gid atomicbitops.Uint32 size uint32 // parent is the dentry corresponding to this dentry's parent directory. @@ -815,7 +813,7 @@ func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked boo return } - if atomic.LoadInt32(&d.fs.released) != 0 { + if d.fs.released.Load() != 0 { d.cachingMu.Unlock() if !renameMuWriteLocked { // Need to lock d.fs.renameMu to access d.parent. Lock it for writing as @@ -915,15 +913,15 @@ func (fs *filesystem) evictCachedDentryLocked(ctx context.Context) { } func (d *dentry) isSymlink() bool { - return atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFLNK + return d.mode.Load()&linux.S_IFMT == linux.S_IFLNK } func (d *dentry) isDir() bool { - return atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFDIR + return d.mode.Load()&linux.S_IFMT == linux.S_IFDIR } func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) error { - return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(atomic.LoadUint32(&d.mode)), auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid))) + return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())) } // verityEnabled checks whether the file is enabled with verity features. It @@ -1173,7 +1171,7 @@ func (fd *fileDescription) generateMerkleLocked(ctx context.Context) ([]byte, ui GID: stat.GID, } - switch atomic.LoadUint32(&fd.d.mode) & linux.S_IFMT { + switch fd.d.mode.Load() & linux.S_IFMT { case linux.S_IFREG: // For a regular file, generate a Merkle tree based on its // content. @@ -1470,9 +1468,9 @@ func (fd *fileDescription) PRead(ctx context.Context, dst usermem.IOSequence, of Tree: &merkleReader, Size: int64(size), Name: fd.d.name, - Mode: fd.d.mode, - UID: fd.d.uid, - GID: fd.d.gid, + Mode: fd.d.mode.Load(), + UID: fd.d.uid.Load(), + GID: fd.d.gid.Load(), Children: fd.d.childrenList, HashAlgorithms: fd.d.fs.alg.toLinuxHashAlg(), ReadOffset: offset, @@ -1595,9 +1593,9 @@ func (fd *fileDescription) Translate(ctx context.Context, required, optional mem Tree: &merkleReader, Size: int64(size), Name: fd.d.name, - Mode: fd.d.mode, - UID: fd.d.uid, - GID: fd.d.gid, + Mode: fd.d.mode.Load(), + UID: fd.d.uid.Load(), + GID: fd.d.gid.Load(), HashAlgorithms: fd.d.fs.alg.toLinuxHashAlg(), ReadOffset: int64(t.Source.Start), ReadSize: int64(t.Source.Length()), diff --git a/pkg/sentry/kernel/cgroup.go b/pkg/sentry/kernel/cgroup.go index 3635734ea..36918520f 100644 --- a/pkg/sentry/kernel/cgroup.go +++ b/pkg/sentry/kernel/cgroup.go @@ -18,8 +18,8 @@ import ( "bytes" "fmt" "sort" - "sync/atomic" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/vfs" @@ -180,9 +180,9 @@ type cgroupFS interface { // +stateify savable type CgroupRegistry struct { // lastHierarchyID is the id of the last allocated cgroup hierarchy. Valid - // ids are from 1 to math.MaxUint32. Must be accessed through atomic ops. + // ids are from 1 to math.MaxUint32. // - lastHierarchyID uint32 + lastHierarchyID atomicbitops.Uint32 mu sync.Mutex `state:"nosave"` @@ -207,7 +207,7 @@ func newCgroupRegistry() *CgroupRegistry { // nextHierarchyID returns a newly allocated, unique hierarchy ID. func (r *CgroupRegistry) nextHierarchyID() (uint32, error) { - if hid := atomic.AddUint32(&r.lastHierarchyID, 1); hid != 0 { + if hid := r.lastHierarchyID.Add(1); hid != 0 { return hid, nil } return InvalidCgroupHierarchyID, fmt.Errorf("cgroup hierarchy ID overflow") diff --git a/pkg/sentry/kernel/futex/BUILD b/pkg/sentry/kernel/futex/BUILD index c897e3a5f..c5078fd26 100644 --- a/pkg/sentry/kernel/futex/BUILD +++ b/pkg/sentry/kernel/futex/BUILD @@ -52,6 +52,7 @@ go_test( srcs = ["futex_test.go"], library = ":futex", deps = [ + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/kernel/futex/futex_test.go b/pkg/sentry/kernel/futex/futex_test.go index 04c136f87..9fe1c6cec 100644 --- a/pkg/sentry/kernel/futex/futex_test.go +++ b/pkg/sentry/kernel/futex/futex_test.go @@ -17,10 +17,10 @@ package futex import ( "math" "runtime" - "sync/atomic" "testing" "unsafe" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -44,19 +44,19 @@ func newTestData(size uint) testData { } func (t testData) SwapUint32(addr hostarch.Addr, new uint32) (uint32, error) { - val := atomic.SwapUint32((*uint32)(unsafe.Pointer(&t.data[addr])), new) + val := (*atomicbitops.Uint32)(unsafe.Pointer(&t.data[addr])).Swap(new) return val, nil } func (t testData) CompareAndSwapUint32(addr hostarch.Addr, old, new uint32) (uint32, error) { - if atomic.CompareAndSwapUint32((*uint32)(unsafe.Pointer(&t.data[addr])), old, new) { + if (*atomicbitops.Uint32)(unsafe.Pointer(&t.data[addr])).CompareAndSwap(old, new) { return old, nil } - return atomic.LoadUint32((*uint32)(unsafe.Pointer(&t.data[addr]))), nil + return (*atomicbitops.Uint32)(unsafe.Pointer(&t.data[addr])).Load(), nil } func (t testData) LoadUint32(addr hostarch.Addr) (uint32, error) { - return atomic.LoadUint32((*uint32)(unsafe.Pointer(&t.data[addr]))), nil + return (*atomicbitops.Uint32)(unsafe.Pointer(&t.data[addr])).Load(), nil } func (t testData) GetSharedKey(addr hostarch.Addr) (Key, error) { @@ -477,8 +477,7 @@ func newTestMutex(addr hostarch.Addr, d testData, m *Manager) *testMutex { func (t *testMutex) Lock() { for { // Attempt to grab the lock. - if atomic.CompareAndSwapUint32( - (*uint32)(unsafe.Pointer(&t.d.data[t.a])), + if (*atomicbitops.Uint32)(unsafe.Pointer(&t.d.data[t.a])).CompareAndSwap( testMutexUnlocked, testMutexLocked) { // Lock held. @@ -504,7 +503,7 @@ func (t *testMutex) Lock() { // This will notify any waiters via the futex manager. func (t *testMutex) Unlock() { // Unlock. - atomic.StoreUint32((*uint32)(unsafe.Pointer(&t.d.data[t.a])), testMutexUnlocked) + (*atomicbitops.Uint32)(unsafe.Pointer(&t.d.data[t.a])).Store(testMutexUnlocked) // Notify all waiters. t.m.Wake(t.d, t.a, true, ^uint32(0), math.MaxInt32) diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 82926c5d2..dda7f46ca 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -35,7 +35,6 @@ import ( "errors" "fmt" "path/filepath" - "sync/atomic" "time" "gvisor.dev/gvisor/pkg/abi/linux" @@ -239,9 +238,8 @@ type Kernel struct { // nextInotifyCookie is a monotonically increasing counter used for // generating unique inotify event cookies. // - // nextInotifyCookie is mutable, and is accessed using atomic memory - // operations. - nextInotifyCookie uint32 + // nextInotifyCookie is mutable. + nextInotifyCookie atomicbitops.Uint32 // netlinkPorts manages allocation of netlink socket port IDs. netlinkPorts *port.Manager @@ -325,7 +323,7 @@ type Kernel struct { ptraceExceptions map[*Task]*Task // YAMAPtraceScope is the current level of YAMA ptrace restrictions. - YAMAPtraceScope int32 + YAMAPtraceScope atomicbitops.Int32 // cgroupRegistry contains the set of active cgroup controllers on the // system. It is controller by cgroupfs. Nil if cgroupfs is unavailable on @@ -431,7 +429,7 @@ func (k *Kernel) Init(args InitKernelArgs) error { k.futexes = futex.NewManager() k.netlinkPorts = port.New() k.ptraceExceptions = make(map[*Task]*Task) - k.YAMAPtraceScope = linux.YAMA_SCOPE_RELATIONAL + k.YAMAPtraceScope = atomicbitops.FromInt32(linux.YAMA_SCOPE_RELATIONAL) k.userCountersMap = make(map[auth.KUID]*userCounters) if VFS2Enabled { @@ -1501,10 +1499,10 @@ func (k *Kernel) Syslog() *syslog { // space is exhausted. 0 is not a valid cookie value, all other values // representable in a uint32 are allowed. func (k *Kernel) GenerateInotifyCookie() uint32 { - id := atomic.AddUint32(&k.nextInotifyCookie, 1) + id := k.nextInotifyCookie.Add(1) // Wrap-around is explicitly allowed for inotify event cookies. if id == 0 { - id = atomic.AddUint32(&k.nextInotifyCookie, 1) + id = k.nextInotifyCookie.Add(1) } return id } diff --git a/pkg/sentry/kernel/pipe/BUILD b/pkg/sentry/kernel/pipe/BUILD index e302ab276..8c845633b 100644 --- a/pkg/sentry/kernel/pipe/BUILD +++ b/pkg/sentry/kernel/pipe/BUILD @@ -19,6 +19,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/kernel/pipe/node.go b/pkg/sentry/kernel/pipe/node.go index 5581e1eb9..682d2258e 100644 --- a/pkg/sentry/kernel/pipe/node.go +++ b/pkg/sentry/kernel/pipe/node.go @@ -15,8 +15,6 @@ package pipe import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -75,10 +73,10 @@ func NewInodeOperations(ctx context.Context, perms fs.FilePermissions, p *Pipe) func (i *inodeOperations) GetFile(ctx context.Context, d *fs.Dirent, flags fs.FileFlags) (*fs.File, error) { switch { case flags.Read && !flags.Write: // O_RDONLY. - tWriters := atomic.LoadInt32(&i.p.totalWriters) + tWriters := i.p.totalWriters.Load() r := i.p.Open(ctx, d, flags) for i.p.isNamed && !flags.NonBlocking && !i.p.HasWriters() && - tWriters == atomic.LoadInt32(&i.p.totalWriters) { + tWriters == i.p.totalWriters.Load() { if !ctx.BlockOn((*waitWriters)(i.p), waiter.EventInternal) { r.DecRef(ctx) return nil, linuxerr.ErrInterrupted @@ -91,10 +89,10 @@ func (i *inodeOperations) GetFile(ctx context.Context, d *fs.Dirent, flags fs.Fi return r, nil case flags.Write && !flags.Read: // O_WRONLY. - tReaders := atomic.LoadInt32(&i.p.totalReaders) + tReaders := i.p.totalReaders.Load() w := i.p.Open(ctx, d, flags) for i.p.isNamed && !i.p.HasReaders() && - tReaders == atomic.LoadInt32(&i.p.totalReaders) { + tReaders == i.p.totalReaders.Load() { // On a nonblocking, write-only open, the open fails with ENXIO if the // read side isn't open yet. if flags.NonBlocking { diff --git a/pkg/sentry/kernel/pipe/pipe.go b/pkg/sentry/kernel/pipe/pipe.go index d236a4142..955f75097 100644 --- a/pkg/sentry/kernel/pipe/pipe.go +++ b/pkg/sentry/kernel/pipe/pipe.go @@ -18,9 +18,9 @@ package pipe import ( "fmt" "io" - "sync/atomic" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -120,24 +120,16 @@ type Pipe struct { isNamed bool // The number of active readers for this pipe. - // - // Access atomically. - readers int32 + readers atomicbitops.Int32 // The total number of readers for this pipe. - // - // Access atomically. - totalReaders int32 + totalReaders atomicbitops.Int32 // The number of active writers for this pipe. - // - // Access atomically. - writers int32 + writers atomicbitops.Int32 // The total number of writers for this pipe. - // - // Access atomically. - totalWriters int32 + totalWriters atomicbitops.Int32 // mu protects all pipe internal state below. mu sync.Mutex `state:"nosave"` @@ -382,8 +374,8 @@ func (p *Pipe) writeLocked(count int64, f func(safemem.BlockSeq) (uint64, error) // rOpen signals a new reader of the pipe. func (p *Pipe) rOpen() { - atomic.AddInt32(&p.readers, 1) - atomic.AddInt32(&p.totalReaders, 1) + p.readers.Add(1) + p.totalReaders.Add(1) // Notify for blocking openers. p.queue.Notify(waiter.EventInternal) @@ -393,8 +385,8 @@ func (p *Pipe) rOpen() { func (p *Pipe) wOpen() { p.mu.Lock() p.hadWriter = true - atomic.AddInt32(&p.writers, 1) - atomic.AddInt32(&p.totalWriters, 1) + p.writers.Add(1) + p.totalWriters.Add(1) p.mu.Unlock() // Notify for blocking openers. @@ -403,26 +395,26 @@ func (p *Pipe) wOpen() { // rClose signals that a reader has closed their end of the pipe. func (p *Pipe) rClose() { - if newReaders := atomic.AddInt32(&p.readers, -1); newReaders < 0 { + if newReaders := p.readers.Add(-1); newReaders < 0 { panic(fmt.Sprintf("Refcounting bug, pipe has negative readers: %v", newReaders)) } } // wClose signals that a writer has closed their end of the pipe. func (p *Pipe) wClose() { - if newWriters := atomic.AddInt32(&p.writers, -1); newWriters < 0 { + if newWriters := p.writers.Add(-1); newWriters < 0 { panic(fmt.Sprintf("Refcounting bug, pipe has negative writers: %v.", newWriters)) } } // HasReaders returns whether the pipe has any active readers. func (p *Pipe) HasReaders() bool { - return atomic.LoadInt32(&p.readers) > 0 + return p.readers.Load() > 0 } // HasWriters returns whether the pipe has any active writers. func (p *Pipe) HasWriters() bool { - return atomic.LoadInt32(&p.writers) > 0 + return p.writers.Load() > 0 } // rReadinessLocked calculates the read readiness. diff --git a/pkg/sentry/kernel/pipe/vfs.go b/pkg/sentry/kernel/pipe/vfs.go index 2389157db..e79514770 100644 --- a/pkg/sentry/kernel/pipe/vfs.go +++ b/pkg/sentry/kernel/pipe/vfs.go @@ -15,8 +15,6 @@ package pipe import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -102,12 +100,12 @@ func (vp *VFSPipe) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, s // Pipes opened for read-write always succeed without blocking. case readable: - tWriters := atomic.LoadInt32(&vp.pipe.totalWriters) + tWriters := vp.pipe.totalWriters.Load() vp.pipe.rOpen() // If this pipe is being opened as blocking and there's no // writer, we have to wait for a writer to open the other end. for vp.pipe.isNamed && statusFlags&linux.O_NONBLOCK == 0 && !vp.pipe.HasWriters() && - tWriters == atomic.LoadInt32(&vp.pipe.totalWriters) { + tWriters == vp.pipe.totalWriters.Load() { if !ctx.BlockOn((*waitWriters)(&vp.pipe), waiter.EventInternal) { fd.DecRef(ctx) return nil, linuxerr.EINTR @@ -115,10 +113,10 @@ func (vp *VFSPipe) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, s } case writable: - tReaders := atomic.LoadInt32(&vp.pipe.totalReaders) + tReaders := vp.pipe.totalReaders.Load() vp.pipe.wOpen() for vp.pipe.isNamed && !vp.pipe.HasReaders() && - tReaders == atomic.LoadInt32(&vp.pipe.totalReaders) { + tReaders == vp.pipe.totalReaders.Load() { // Non-blocking, write-only opens fail with ENXIO when the read // side isn't open yet. if statusFlags&linux.O_NONBLOCK != 0 { diff --git a/pkg/sentry/kernel/ptrace.go b/pkg/sentry/kernel/ptrace.go index c1463c3b3..c9c434c4b 100644 --- a/pkg/sentry/kernel/ptrace.go +++ b/pkg/sentry/kernel/ptrace.go @@ -16,7 +16,6 @@ package kernel import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -129,7 +128,7 @@ func (t *Task) CanTrace(target *Task, attach bool) bool { return true } - if atomic.LoadInt32(&t.k.YAMAPtraceScope) == linux.YAMA_SCOPE_RELATIONAL { + if t.k.YAMAPtraceScope.Load() == linux.YAMA_SCOPE_RELATIONAL { t.tg.pidns.owner.mu.RLock() defer t.tg.pidns.owner.mu.RUnlock() if !t.canTraceYAMALocked(target) { @@ -155,7 +154,7 @@ func (t *Task) canTraceLocked(target *Task, attach bool) bool { return true } - if atomic.LoadInt32(&t.k.YAMAPtraceScope) == linux.YAMA_SCOPE_RELATIONAL { + if t.k.YAMAPtraceScope.Load() == linux.YAMA_SCOPE_RELATIONAL { if !t.canTraceYAMALocked(target) { return false } diff --git a/pkg/sentry/kernel/syscalls.go b/pkg/sentry/kernel/syscalls.go index 8548ccbf0..b3a78bed4 100644 --- a/pkg/sentry/kernel/syscalls.go +++ b/pkg/sentry/kernel/syscalls.go @@ -16,9 +16,9 @@ package kernel import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/bits" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/sentry/arch" @@ -121,10 +121,10 @@ type SyscallFlagsTable struct { // // missing syscalls have the same value in enable as missingEnable to // avoid an extra branch in Word. - enable [maxSyscallNum + 1]uint32 + enable [maxSyscallNum + 1]atomicbitops.Uint32 // missingEnable contains the enable bits for missing syscalls. - missingEnable uint32 + missingEnable atomicbitops.Uint32 } // Init initializes the struct, with all syscalls in table set to enable. @@ -132,17 +132,17 @@ type SyscallFlagsTable struct { // max is the largest syscall number in table. func (e *SyscallFlagsTable) init(table map[uintptr]Syscall) { for num := range table { - e.enable[num] = syscallPresent + e.enable[num] = atomicbitops.FromUint32(syscallPresent) } } // Word returns the enable bitfield for sysno. func (e *SyscallFlagsTable) Word(sysno uintptr) uint32 { if sysno <= maxSyscallNum { - return atomic.LoadUint32(&e.enable[sysno]) + return e.enable[sysno].Load() } - return atomic.LoadUint32(&e.missingEnable) + return e.missingEnable.Load() } // Enable sets enable bit bit for all syscalls based on s. @@ -158,19 +158,19 @@ func (e *SyscallFlagsTable) Enable(bit uint32, s map[uintptr]bool, missingEnable e.mu.Lock() defer e.mu.Unlock() - missingVal := atomic.LoadUint32(&e.missingEnable) + missingVal := e.missingEnable.Load() if missingEnable { missingVal |= bit } else { missingVal &^= bit } - atomic.StoreUint32(&e.missingEnable, missingVal) + e.missingEnable.Store(missingVal) for num := range e.enable { - val := atomic.LoadUint32(&e.enable[num]) + val := e.enable[num].Load() if !bits.IsOn32(val, syscallPresent) { // Missing. - atomic.StoreUint32(&e.enable[num], missingVal) + e.enable[num].Store(missingVal) continue } @@ -179,7 +179,7 @@ func (e *SyscallFlagsTable) Enable(bit uint32, s map[uintptr]bool, missingEnable } else { val &^= bit } - atomic.StoreUint32(&e.enable[num], val) + e.enable[num].Store(val) } } @@ -188,20 +188,20 @@ func (e *SyscallFlagsTable) EnableAll(bit uint32) { e.mu.Lock() defer e.mu.Unlock() - missingVal := atomic.LoadUint32(&e.missingEnable) + missingVal := e.missingEnable.Load() missingVal |= bit - atomic.StoreUint32(&e.missingEnable, missingVal) + e.missingEnable.Store(missingVal) for num := range e.enable { - val := atomic.LoadUint32(&e.enable[num]) + val := e.enable[num].Load() if !bits.IsOn32(val, syscallPresent) { // Missing. - atomic.StoreUint32(&e.enable[num], missingVal) + e.enable[num].Store(missingVal) continue } val |= bit - atomic.StoreUint32(&e.enable[num], val) + e.enable[num].Store(val) } } diff --git a/pkg/sentry/kernel/task.go b/pkg/sentry/kernel/task.go index fcaf802b2..5d817ed3e 100644 --- a/pkg/sentry/kernel/task.go +++ b/pkg/sentry/kernel/task.go @@ -71,9 +71,7 @@ type Task struct { // taskWorkCount represents the current size of the task work queue. It is // used to avoid acquiring taskWorkMu when the queue is empty. - // - // Must accessed with atomic memory operations. - taskWorkCount int32 + taskWorkCount atomicbitops.Int32 // taskWorkMu protects taskWork. taskWorkMu sync.Mutex `state:"nosave"` @@ -218,7 +216,7 @@ type Task struct { // stop; after a save/restore cycle, the restored sentry has no knowledge // of the pre-save sentryctl command, and the stopped task would remain // stopped forever.) - stopCount int32 `state:"nosave"` + stopCount atomicbitops.Int32 `state:"nosave"` // endStopCond is signaled when stopCount transitions to 0. The combination // of stopCount and endStopCond effectively form a sync.WaitGroup, but @@ -483,9 +481,7 @@ type Task struct { // cpu is the fake cpu number returned by getcpu(2). cpu is ignored // entirely if Kernel.useHostCores is true. - // - // cpu is accessed using atomic memory operations. - cpu int32 + cpu atomicbitops.Int32 // This is used to keep track of changes made to a process' priority/niceness. // It is mostly used to provide some reasonable return value from @@ -624,7 +620,7 @@ func (t *Task) afterLoad() { t.interruptChan = make(chan struct{}, 1) t.gosched.State = TaskGoroutineNonexistent if t.stop != nil { - t.stopCount = 1 + t.stopCount = atomicbitops.FromInt32(1) } t.endStopCond.L = &t.tg.signalHandlers.mu t.rseqPreempted = true @@ -844,7 +840,7 @@ func (t *Task) ContainerID() string { // OOMScoreAdj gets the task's thread group's OOM score adjustment. func (t *Task) OOMScoreAdj() int32 { - return atomic.LoadInt32(&t.tg.oomScoreAdj) + return t.tg.oomScoreAdj.Load() } // SetOOMScoreAdj sets the task's thread group's OOM score adjustment. The @@ -853,7 +849,7 @@ func (t *Task) SetOOMScoreAdj(adj int32) error { if adj > 1000 || adj < -1000 { return linuxerr.EINVAL } - atomic.StoreInt32(&t.tg.oomScoreAdj, adj) + t.tg.oomScoreAdj.Store(adj) return nil } diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go index c824d6269..9d8188744 100644 --- a/pkg/sentry/kernel/task_clone.go +++ b/pkg/sentry/kernel/task_clone.go @@ -15,9 +15,8 @@ package kernel import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/bpf" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -182,7 +181,7 @@ func (t *Task) Clone(args *linux.CloneArgs) (ThreadID, *SyscallControl, error) { sh = sh.Fork() } tg = t.k.NewThreadGroup(tg.mounts, pidns, sh, linux.Signal(args.ExitSignal), tg.limits.GetCopy()) - tg.oomScoreAdj = atomic.LoadInt32(&t.tg.oomScoreAdj) + tg.oomScoreAdj = atomicbitops.FromInt32(t.tg.oomScoreAdj.Load()) rseqAddr = t.rseqAddr rseqSignature = t.rseqSignature } diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index c03769123..56ebea439 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -18,7 +18,6 @@ import ( "fmt" "runtime" "runtime/trace" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -113,7 +112,7 @@ func (t *Task) run(threadID uintptr) { // doStop is called by Task.run to block until the task is not stopped. func (t *Task) doStop() { - if atomic.LoadInt32(&t.stopCount) == 0 { + if t.stopCount.Load() == 0 { return } t.Deactivate() @@ -128,7 +127,7 @@ func (t *Task) doStop() { defer t.tg.pidns.owner.runningGoroutines.Add(1) t.goroutineStopped.Add(-1) defer t.goroutineStopped.Add(1) - for t.stopCount > 0 { + for t.stopCount.RacyLoad() > 0 { t.endStopCond.Wait() } } @@ -148,11 +147,11 @@ func (app *runApp) execute(t *Task) taskRunState { } // Execute any task work callbacks before returning to user space. - if atomic.LoadInt32(&t.taskWorkCount) > 0 { + if t.taskWorkCount.Load() > 0 { t.taskWorkMu.Lock() queue := t.taskWork t.taskWork = nil - atomic.StoreInt32(&t.taskWorkCount, 0) + t.taskWorkCount.Store(0) t.taskWorkMu.Unlock() // Do not hold taskWorkMu while executing task work, which may register diff --git a/pkg/sentry/kernel/task_sched.go b/pkg/sentry/kernel/task_sched.go index 199483cf4..4cfc46ed7 100644 --- a/pkg/sentry/kernel/task_sched.go +++ b/pkg/sentry/kernel/task_sched.go @@ -19,7 +19,6 @@ package kernel import ( "fmt" "math/rand" - "sync/atomic" "time" "gvisor.dev/gvisor/pkg/abi/linux" @@ -365,7 +364,7 @@ func (ticker *kernelCPUClockTicker) NotifyTimer(exp uint64, setting ktime.Settin // Check thread group CPU timers. tgs := ticker.k.tasks.Root.ThreadGroupsAppend(ticker.tgs) for _, tg := range tgs { - if atomic.LoadUint32(&tg.cpuTimersEnabled) == 0 { + if tg.cpuTimersEnabled.Load() == 0 { continue } @@ -522,9 +521,9 @@ func (t *Task) NotifyRlimitCPUUpdated() { func (tg *ThreadGroup) updateCPUTimersEnabledLocked() { rlimitCPU := tg.limits.Get(limits.CPU) if tg.itimerVirtSetting.Enabled || tg.itimerProfSetting.Enabled || tg.rlimitCPUSoftSetting.Enabled || rlimitCPU.Max != limits.Infinity { - atomic.StoreUint32(&tg.cpuTimersEnabled, 1) + tg.cpuTimersEnabled.Store(1) } else { - atomic.StoreUint32(&tg.cpuTimersEnabled, 0) + tg.cpuTimersEnabled.Store(0) } } @@ -612,7 +611,7 @@ func (t *Task) SetCPUMask(mask sched.CPUSet) error { t.mu.Lock() defer t.mu.Unlock() t.allowedCPUMask = mask - atomic.StoreInt32(&t.cpu, assignCPU(mask, rootTID)) + t.cpu.Store(assignCPU(mask, rootTID)) return nil } @@ -622,7 +621,7 @@ func (t *Task) CPU() int32 { return int32(hostcpu.GetCPU()) } - return atomic.LoadInt32(&t.cpu) + return t.cpu.Load() } // assignCPU returns the virtualized CPU number for the task with global TID diff --git a/pkg/sentry/kernel/task_start.go b/pkg/sentry/kernel/task_start.go index fdf10dca6..0770f908c 100644 --- a/pkg/sentry/kernel/task_start.go +++ b/pkg/sentry/kernel/task_start.go @@ -224,12 +224,13 @@ func (ts *TaskSet) newTask(cfg *TaskConfig) (*Task, error) { tg.activeTasks++ // Propagate external TaskSet stops to the new task. - t.stopCount = ts.stopCount + t.stopCount = atomicbitops.FromInt32(ts.stopCount) t.mu.Lock() defer t.mu.Unlock() - t.cpu = assignCPU(t.allowedCPUMask, ts.Root.tids[t]) + t.cpu = atomicbitops.FromInt32(assignCPU(t.allowedCPUMask, ts.Root.tids[t])) + t.startTime = t.k.RealtimeClock().Now() // As a final step, initialize the platform context. This may require diff --git a/pkg/sentry/kernel/task_stop.go b/pkg/sentry/kernel/task_stop.go index a35948a5f..7068431d4 100644 --- a/pkg/sentry/kernel/task_stop.go +++ b/pkg/sentry/kernel/task_stop.go @@ -63,7 +63,6 @@ package kernel import ( "fmt" - "sync/atomic" ) // A TaskStop is a condition visible to the task control flow graph that @@ -168,7 +167,7 @@ func (t *Task) EndExternalStop() { // // Preconditions: The signal mutex must be locked. func (t *Task) beginStopLocked() { - if newval := atomic.AddInt32(&t.stopCount, 1); newval <= 0 { + if newval := t.stopCount.Add(1); newval <= 0 { // Most likely overflow. panic(fmt.Sprintf("Invalid stopCount: %d", newval)) } @@ -179,7 +178,7 @@ func (t *Task) beginStopLocked() { // // Preconditions: The signal mutex must be locked. func (t *Task) endStopLocked() { - if newval := atomic.AddInt32(&t.stopCount, -1); newval < 0 { + if newval := t.stopCount.Add(-1); newval < 0 { panic(fmt.Sprintf("Invalid stopCount: %d", newval)) } else if newval == 0 { t.endStopCond.Signal() diff --git a/pkg/sentry/kernel/task_work.go b/pkg/sentry/kernel/task_work.go index dda5a433a..72dc4de60 100644 --- a/pkg/sentry/kernel/task_work.go +++ b/pkg/sentry/kernel/task_work.go @@ -14,8 +14,6 @@ package kernel -import "sync/atomic" - // TaskWorker is a deferred task. // // This must be savable. @@ -33,6 +31,6 @@ type TaskWorker interface { func (t *Task) RegisterWork(work TaskWorker) { t.taskWorkMu.Lock() defer t.taskWorkMu.Unlock() - atomic.AddInt32(&t.taskWorkCount, 1) + t.taskWorkCount.Add(1) t.taskWork = append(t.taskWork, work) } diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go index 080cbb83a..48a58f10f 100644 --- a/pkg/sentry/kernel/thread_group.go +++ b/pkg/sentry/kernel/thread_group.go @@ -18,6 +18,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/sentry/fs" @@ -183,9 +184,8 @@ type ThreadGroup struct { // itimerProfSetting.Enabled is true, rlimitCPUSoftSetting.Enabled is true, // or limits.Get(CPU) is finite. // - // cpuTimersEnabled is protected by the signal mutex. cpuTimersEnabled is - // accessed using atomic memory operations. - cpuTimersEnabled uint32 + // cpuTimersEnabled is protected by the signal mutex. + cpuTimersEnabled atomicbitops.Uint32 // timers is the thread group's POSIX interval timers. nextTimerID is the // TimerID at which allocation should begin searching for an unused ID. @@ -258,9 +258,7 @@ type ThreadGroup struct { // oomScoreAdj is the thread group's OOM score adjustment. This is // currently not used but is maintained for consistency. // TODO(gvisor.dev/issue/1967) - // - // oomScoreAdj is accessed using atomic memory operations. - oomScoreAdj int32 + oomScoreAdj atomicbitops.Int32 } // NewThreadGroup returns a new, empty thread group in PID namespace pidns. The diff --git a/pkg/sentry/seccheck/BUILD b/pkg/sentry/seccheck/BUILD index f301bfdf1..3ff67958f 100644 --- a/pkg/sentry/seccheck/BUILD +++ b/pkg/sentry/seccheck/BUILD @@ -26,6 +26,7 @@ go_library( visibility = ["//:sandbox"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/gohacks", "//pkg/sentry/kernel/time", diff --git a/pkg/sentry/seccheck/seccheck.go b/pkg/sentry/seccheck/seccheck.go index b64508ff5..102571d69 100644 --- a/pkg/sentry/seccheck/seccheck.go +++ b/pkg/sentry/seccheck/seccheck.go @@ -17,8 +17,7 @@ package seccheck import ( - "sync/atomic" - + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" pb "gvisor.dev/gvisor/pkg/sentry/seccheck/points/points_go_proto" "gvisor.dev/gvisor/pkg/sync" @@ -159,9 +158,8 @@ type State struct { // enabledPoints is a bitmask of checkpoints for which at least one Checker // is registered. // - // enabledPoints is accessed using atomic memory operations. Mutation of - // enabledPoints is serialized by registrationMu. - enabledPoints [numPointBitmaskUint32s]uint32 + // Mutation of enabledPoints is serialized by registrationMu. + enabledPoints [numPointBitmaskUint32s]atomicbitops.Uint32 // registrationSeq supports store-free atomic reads of registeredCheckers. registrationSeq sync.SeqCount @@ -188,7 +186,7 @@ func (s *State) AppendChecker(c Checker, reqs []PointReq) { } for _, req := range reqs { word, bit := req.Pt/32, req.Pt%32 - atomic.StoreUint32(&s.enabledPoints[word], s.enabledPoints[word]|(uint32(1)<