From 14b5ff5a2f323dbb30c850d548cb3b1f6bbc8e41 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 15 Dec 2022 10:13:44 -0800 Subject: [PATCH] overlayfs: don't call SetStat and StatAt under dentry.mapsMu This allows to avoid lock order inversions with inodeMutex and filesystemRWMutex. PiperOrigin-RevId: 495629138 --- pkg/sentry/fsimpl/overlay/copy_up.go | 6 ++---- pkg/sentry/fsimpl/overlay/overlay.go | 4 ++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/sentry/fsimpl/overlay/copy_up.go b/pkg/sentry/fsimpl/overlay/copy_up.go index f073b2369..e6c12e0c3 100644 --- a/pkg/sentry/fsimpl/overlay/copy_up.go +++ b/pkg/sentry/fsimpl/overlay/copy_up.go @@ -144,8 +144,6 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy cleanupUndoCopyUp() return err } - d.mapsMu.Lock() - defer d.mapsMu.Unlock() if d.wrappedMappable != nil { // We may have memory mappings of the file on the lower layer. // Switch to mapping the file on the upper layer instead. @@ -305,8 +303,8 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy } if mmapOpts != nil && mmapOpts.Mappable != nil { - // Note that if mmapOpts != nil, then d.mapsMu is locked for writing - // (from the S_IFREG path above). + d.mapsMu.Lock() + defer d.mapsMu.Unlock() // Propagate mappings of d to the new Mappable. Remember which mappings // we added so we can remove them on failure. diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index c2e3e783c..84027a557 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -533,6 +533,10 @@ type dentry struct { // // - isMappable is non-zero iff wrappedMappable is non-nil. isMappable is // accessed using atomic memory operations. + // + // - wrappedMappable is protected by mapsMu and dataMu. In addition, + // it has to be immutable if copyMu is taken for write. + // copyUpMaybeSyntheticMountpointLocked relies on this behavior. mapsMu mapsMutex `state:"nosave"` lowerMappings memmap.MappingSet dataMu dataRWMutex `state:"nosave"`