From 4885931ac342e033b39ba9645b8e6a584f4d9844 Mon Sep 17 00:00:00 2001 From: Chong Cai Date: Mon, 12 Oct 2020 17:28:58 -0700 Subject: Change verity mu to be per file system verity Mu should be per file system instead of global, so that enabling and verifying in different file systems won't block each other. Also Lock verity Mu in PRead. PiperOrigin-RevId: 336779356 --- pkg/sentry/fsimpl/verity/filesystem.go | 8 ++++---- pkg/sentry/fsimpl/verity/verity.go | 24 +++++++++++++++--------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/pkg/sentry/fsimpl/verity/filesystem.go b/pkg/sentry/fsimpl/verity/filesystem.go index 34e2c9d7c..3b3c8725f 100644 --- a/pkg/sentry/fsimpl/verity/filesystem.go +++ b/pkg/sentry/fsimpl/verity/filesystem.go @@ -174,8 +174,8 @@ func (fs *filesystem) verifyChild(ctx context.Context, parent *dentry, child *de return nil, err } - verityMu.RLock() - defer verityMu.RUnlock() + fs.verityMu.RLock() + defer fs.verityMu.RUnlock() // Read the offset of the child from the extended attributes of the // corresponding Merkle tree file. // This is the offset of the hash for child in its parent's Merkle tree @@ -302,8 +302,8 @@ func (fs *filesystem) verifyStat(ctx context.Context, d *dentry, stat linux.Stat return err } - verityMu.RLock() - defer verityMu.RUnlock() + fs.verityMu.RLock() + defer fs.verityMu.RUnlock() fd, err := vfsObj.OpenAt(ctx, fs.creds, &vfs.PathOperation{ Root: d.lowerMerkleVD, diff --git a/pkg/sentry/fsimpl/verity/verity.go b/pkg/sentry/fsimpl/verity/verity.go index 4a6708633..4f11487a9 100644 --- a/pkg/sentry/fsimpl/verity/verity.go +++ b/pkg/sentry/fsimpl/verity/verity.go @@ -68,11 +68,6 @@ const sizeOfStringInt32 = 10 // flag. var noCrashOnVerificationFailure bool -// verityMu synchronizes enabling verity files, protects files or directories -// from being enabled by different threads simultaneously. It also ensures that -// verity does not access files that are being enabled. -var verityMu sync.RWMutex - // FilesystemType implements vfs.FilesystemType. // // +stateify savable @@ -106,6 +101,17 @@ type filesystem struct { // to ensure consistent lock ordering between dentry.dirMu in different // dentries. renameMu sync.RWMutex `state:"nosave"` + + // verityMu synchronizes enabling verity files, protects files or + // directories from being enabled by different threads simultaneously. + // It also ensures that verity does not access files that are being + // enabled. + // + // Also, the directory Merkle trees depends on the generated trees of + // its children. So they shouldn't be enabled the same time. This lock + // is for the whole file system to ensure that no more than one file is + // enabled the same time. + verityMu sync.RWMutex } // InternalFilesystemOptions may be passed as @@ -594,10 +600,8 @@ func (fd *fileDescription) enableVerity(ctx context.Context, uio usermem.IO) (ui return 0, syserror.EPERM } - // Lock to prevent other threads performing enable or access the file - // while it's being enabled. - verityMu.Lock() - defer verityMu.Unlock() + fd.d.fs.verityMu.Lock() + defer fd.d.fs.verityMu.Unlock() // In allowRuntimeEnable mode, the underlying fd and read/write fd for // the Merkle tree file should have all been initialized. For any file @@ -723,6 +727,8 @@ func (fd *fileDescription) PRead(ctx context.Context, dst usermem.IOSequence, of return fd.lowerFD.PRead(ctx, dst, offset, opts) } + fd.d.fs.verityMu.RLock() + defer fd.d.fs.verityMu.RUnlock() // dataSize is the size of the whole file. dataSize, err := fd.merkleReader.GetXattr(ctx, &vfs.GetXattrOptions{ Name: merkleSizeXattr, -- cgit v1.2.3