From 18932476167ecf16b7d3e85ae6addaaba193ceed Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Wed, 29 Aug 2018 11:21:21 -0700 Subject: [PATCH] fs: Drop reference to over-written file before renaming over it. dirent.go:Rename() walks to the file being replaced and defers replaced.DecRef(). After the rename, the reference is dropped, triggering a writeout and SettAttr call to the gofer. Because of lazyOpenForWrite, the gofer opens the replaced file BY ITS OLD NAME and calls ftruncate on it. This CL changes Remove to drop the reference on replaced (and thus trigger writeout) before the actual rename call. PiperOrigin-RevId: 210756097 Change-Id: I01ea09a5ee6c2e2d464560362f09943641638e0f --- pkg/sentry/fs/dirent.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go index 9417e808f..30545de7e 100644 --- a/pkg/sentry/fs/dirent.go +++ b/pkg/sentry/fs/dirent.go @@ -1549,16 +1549,19 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string // Check constraints on the object being replaced, if any. replaced, err := newParent.walk(ctx, root, newName, false /* may unlock */) if err == nil { - defer replaced.DecRef() + // NOTE: We don't want to keep replaced alive + // across the Rename, so must call DecRef manually (no defer). // Target should not be an ancestor of source. if replaced == oldParent { + replaced.DecRef() // Why is this not EINVAL? See fs/namei.c. return syscall.ENOTEMPTY } // Is the thing we're trying to replace busy? if replaced.Busy() { + replaced.DecRef() return syscall.EBUSY } @@ -1566,9 +1569,11 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string oldIsDir := IsDir(renamed.Inode.StableAttr) newIsDir := IsDir(replaced.Inode.StableAttr) if !newIsDir && oldIsDir { + replaced.DecRef() return syscall.ENOTDIR } if !oldIsDir && newIsDir { + replaced.DecRef() return syscall.EISDIR } @@ -1583,6 +1588,9 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string replaced.mu.Lock() replaced.flush() replaced.mu.Unlock() + + // Done with replaced. + replaced.DecRef() } if err := renamed.Inode.Rename(ctx, oldParent, renamed, newParent, newName); err != nil {