diff --git a/pkg/sentry/fs/BUILD b/pkg/sentry/fs/BUILD index 4e573d249..8774e8bfa 100644 --- a/pkg/sentry/fs/BUILD +++ b/pkg/sentry/fs/BUILD @@ -46,7 +46,6 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", - "//pkg/amutex", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/fs/file.go b/pkg/sentry/fs/file.go index e5d542419..55d7bf809 100644 --- a/pkg/sentry/fs/file.go +++ b/pkg/sentry/fs/file.go @@ -18,7 +18,6 @@ import ( "math" "sync/atomic" - "gvisor.dev/gvisor/pkg/amutex" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/refs" @@ -91,7 +90,7 @@ type File struct { // mu is dual-purpose: first, to make read(2) and write(2) thread-safe // in conformity with POSIX, and second, to cancel operations before they // begin in response to interruptions (i.e. signals). - mu amutex.AbortableMutex `state:"nosave"` + mu sync.Mutex `state:"nosave"` // FileOperations implements file system specific behavior for this File. FileOperations FileOperations `state:"wait"` @@ -113,7 +112,6 @@ func NewFile(ctx context.Context, dirent *Dirent, flags FileFlags, fops FileOper FileOperations: fops, flags: flags, } - f.mu.Init() f.EnableLeakCheck("fs.File") return &f } @@ -197,9 +195,7 @@ func (f *File) EventUnregister(e *waiter.Entry) { // // Returns linuxerr.ErrInterrupted if seeking was interrupted. func (f *File) Seek(ctx context.Context, whence SeekWhence, offset int64) (int64, error) { - if !f.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } + f.mu.Lock() defer f.mu.Unlock() newOffset, err := f.FileOperations.Seek(ctx, f, whence, offset) @@ -219,9 +215,7 @@ func (f *File) Seek(ctx context.Context, whence SeekWhence, offset int64) (int64 // // Returns linuxerr.ErrInterrupted if reading was interrupted. func (f *File) Readdir(ctx context.Context, serializer DentrySerializer) error { - if !f.mu.Lock(ctx) { - return linuxerr.ErrInterrupted - } + f.mu.Lock() defer f.mu.Unlock() offset, err := f.FileOperations.Readdir(ctx, f, serializer) @@ -236,17 +230,13 @@ func (f *File) Readdir(ctx context.Context, serializer DentrySerializer) error { func (f *File) Readv(ctx context.Context, dst usermem.IOSequence) (int64, error) { start := fsmetric.StartReadWait() defer fsmetric.FinishReadWait(fsmetric.ReadWait, start) - - if !f.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } - + f.mu.Lock() + defer f.mu.Unlock() fsmetric.Reads.Increment() n, err := f.FileOperations.Read(ctx, f, dst, f.offset) if n > 0 && !f.flags.NonSeekable { atomic.AddInt64(&f.offset, n) } - f.mu.Unlock() return n, err } @@ -258,14 +248,10 @@ func (f *File) Readv(ctx context.Context, dst usermem.IOSequence) (int64, error) func (f *File) Preadv(ctx context.Context, dst usermem.IOSequence, offset int64) (int64, error) { start := fsmetric.StartReadWait() defer fsmetric.FinishReadWait(fsmetric.ReadWait, start) - - if !f.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } - + f.mu.Lock() + defer f.mu.Unlock() fsmetric.Reads.Increment() n, err := f.FileOperations.Read(ctx, f, dst, offset) - f.mu.Unlock() return n, err } @@ -278,15 +264,13 @@ func (f *File) Preadv(ctx context.Context, dst usermem.IOSequence, offset int64) // // Returns linuxerr.ErrInterrupted if writing was interrupted. func (f *File) Writev(ctx context.Context, src usermem.IOSequence) (int64, error) { - if !f.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } + f.mu.Lock() + defer f.mu.Unlock() unlockAppendMu := f.Dirent.Inode.lockAppendMu(f.Flags().Append) // Handle append mode. if f.Flags().Append { if err := f.offsetForAppend(ctx, &f.offset); err != nil { unlockAppendMu() - f.mu.Unlock() return 0, err } } @@ -296,7 +280,6 @@ func (f *File) Writev(ctx context.Context, src usermem.IOSequence) (int64, error switch { case ok && limit == 0: unlockAppendMu() - f.mu.Unlock() return 0, linuxerr.ErrExceedsFileSizeLimit case ok: src = src.TakeFirst64(limit) @@ -308,7 +291,6 @@ func (f *File) Writev(ctx context.Context, src usermem.IOSequence) (int64, error atomic.StoreInt64(&f.offset, f.offset+n) } unlockAppendMu() - f.mu.Unlock() return n, err } @@ -383,11 +365,8 @@ func (f *File) checkLimit(ctx context.Context, offset int64) (int64, bool) { // // Returns linuxerr.ErrInterrupted if syncing was interrupted. func (f *File) Fsync(ctx context.Context, start int64, end int64, syncType SyncType) error { - if !f.mu.Lock(ctx) { - return linuxerr.ErrInterrupted - } + f.mu.Lock() defer f.mu.Unlock() - return f.FileOperations.Fsync(ctx, f, start, end, syncType) } @@ -395,11 +374,8 @@ func (f *File) Fsync(ctx context.Context, start int64, end int64, syncType SyncT // // Returns linuxerr.ErrInterrupted if syncing was interrupted. func (f *File) Flush(ctx context.Context) error { - if !f.mu.Lock(ctx) { - return linuxerr.ErrInterrupted - } + f.mu.Lock() defer f.mu.Unlock() - return f.FileOperations.Flush(ctx, f) } @@ -407,11 +383,8 @@ func (f *File) Flush(ctx context.Context) error { // // Returns linuxerr.ErrInterrupted if interrupted. func (f *File) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpts) error { - if !f.mu.Lock(ctx) { - return linuxerr.ErrInterrupted - } + f.mu.Lock() defer f.mu.Unlock() - return f.FileOperations.ConfigureMMap(ctx, f, opts) } @@ -419,11 +392,8 @@ func (f *File) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpts) error { // // Returns linuxerr.ErrInterrupted if interrupted. func (f *File) UnstableAttr(ctx context.Context) (UnstableAttr, error) { - if !f.mu.Lock(ctx) { - return UnstableAttr{}, linuxerr.ErrInterrupted - } + f.mu.Lock() defer f.mu.Unlock() - return f.FileOperations.UnstableAttr(ctx, f) } diff --git a/pkg/sentry/fs/file_state.go b/pkg/sentry/fs/file_state.go index 523182d59..a8d14357a 100644 --- a/pkg/sentry/fs/file_state.go +++ b/pkg/sentry/fs/file_state.go @@ -24,7 +24,6 @@ func (f *File) beforeSave() { // afterLoad is invoked by stateify. func (f *File) afterLoad() { - f.mu.Init() if f.flags.Async && f.async != nil { f.async.Register(f) } diff --git a/pkg/sentry/fs/splice.go b/pkg/sentry/fs/splice.go index 266140f6f..474b8ddde 100644 --- a/pkg/sentry/fs/splice.go +++ b/pkg/sentry/fs/splice.go @@ -25,6 +25,7 @@ import ( // Splice moves data to this file, directly from another. // // Offsets are updated only if DstOffset and SrcOffset are set. +// +checklocksignore func Splice(ctx context.Context, dst *File, src *File, opts SpliceOpts) (int64, error) { // Verify basic file flag permissions. if !dst.Flags().Write || !src.Flags().Read { @@ -53,44 +54,28 @@ func Splice(ctx context.Context, dst *File, src *File, opts SpliceOpts) (int64, switch { case dst.UniqueID < src.UniqueID: // Acquire dst first. - if !dst.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } - if !src.mu.Lock(ctx) { - dst.mu.Unlock() - return 0, linuxerr.ErrInterrupted - } + dst.mu.Lock() + src.mu.Lock() case dst.UniqueID > src.UniqueID: // Acquire src first. - if !src.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } - if !dst.mu.Lock(ctx) { - src.mu.Unlock() - return 0, linuxerr.ErrInterrupted - } + src.mu.Lock() + dst.mu.Lock() case dst.UniqueID == src.UniqueID: // Acquire only one lock; it's the same file. This is a // bit of a edge case, but presumably it's possible. - if !dst.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } - srcLock = false // Only need one unlock. + dst.mu.Lock() + srcLock = false } // Use both offsets (locked). opts.DstStart = dst.offset opts.SrcStart = src.offset case dstLock: // Acquire only dst. - if !dst.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } + dst.mu.Lock() opts.DstStart = dst.offset // Safe: locked. case srcLock: // Acquire only src. - if !src.mu.Lock(ctx) { - return 0, linuxerr.ErrInterrupted - } + src.mu.Lock() opts.SrcStart = src.offset // Safe: locked. } diff --git a/pkg/sentry/fsimpl/tmpfs/BUILD b/pkg/sentry/fsimpl/tmpfs/BUILD index 94486bb63..1e7a817e8 100644 --- a/pkg/sentry/fsimpl/tmpfs/BUILD +++ b/pkg/sentry/fsimpl/tmpfs/BUILD @@ -56,7 +56,6 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", - "//pkg/amutex", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", diff --git a/pkg/sentry/kernel/BUILD b/pkg/sentry/kernel/BUILD index f3f16eb7a..147c09adf 100644 --- a/pkg/sentry/kernel/BUILD +++ b/pkg/sentry/kernel/BUILD @@ -219,7 +219,6 @@ go_library( "//pkg/abi", "//pkg/abi/linux", "//pkg/abi/linux/errno", - "//pkg/amutex", "//pkg/bitmap", "//pkg/bits", "//pkg/bpf",