netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Fix logic bug in attaching mounts.

Browse Source 
Before this fix, the vd provided to connectMountAt wouldn't be consumed if
we ran into a dead mount, the caller was responsible. This dead mount
could be rooted at a different vd than the one provided, which meant that we
were DecRef'ing the wrong vd if the method returned an error. The method now
always consumes the vd to simplify its use.

Reported-by: syzbot+e8a05606f54650464cc0@syzkaller.appspotmail.com
PiperOrigin-RevId: 532636615
This commit is contained in:
Lucas Manning
2023-05-16 18:53:33 -07:00
committed by gVisor bot
parent 7743f313c3
commit 283b80a456
2 changed files with 34 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/vfs/mount.go
+13 -12
View File
@@ -276,18 +276,18 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
tree := vfs.preparePropagationTree(mnt, vd)
cleanup := cleanup.Make(func() {
vfs.abortPropagationTree(ctx, tree) // +checklocksforce
})
defer cleanup.Clean()
// Check if the new mount + all the propagation mounts puts us over the max.
if uint32(len(tree)+1)+vd.mount.ns.mounts > MountMax {
// We need to unlock mountMu first because DecRef takes a lock on the
// filesystem mutex in some implementations, which can lead to circular
// locking.
vfs.mountMu.Unlock()
vd.DecRef(ctx)
})
defer cleanup.Clean()
// Check if the new mount + all the propagation mounts puts us over the max.
if uint32(len(tree)+1)+vd.mount.ns.mounts > MountMax {
return linuxerr.ENOSPC
}
if err := vfs.connectMountAt(ctx, mnt, vd); err != nil {
if err := vfs.connectMountAtLocked(ctx, mnt, vd); err != nil {
return err
}
vfs.commitPropagationTree(ctx, tree)
@@ -296,20 +296,20 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
return nil
}
// connectMountAtLocked attaches mnt at vd. If the method returns an error that
// is not nil, then it did not consume a reference on vd and the caller is
// responsible for calling DecRef.
// connectMountAtLocked attaches mnt at vd. This method consumes a reference on
// vd.
//
// Preconditions:
// - mnt must be disconnected.
// - vfs.mountMu must be locked.
//
// +checklocks:vfs.mountMu
func (vfs *VirtualFilesystem) connectMountAt(ctx context.Context, mnt *Mount, vd VirtualDentry) error {
func (vfs *VirtualFilesystem) connectMountAtLocked(ctx context.Context, mnt *Mount, vd VirtualDentry) error {
vd.dentry.mu.Lock()
for {
if vd.mount.umounted || vd.dentry.dead {
vd.dentry.mu.Unlock()
vd.DecRef(ctx)
return linuxerr.ENOENT
}
// vd might have been mounted over between vfs.GetDentryAt() and
@@ -408,14 +408,15 @@ func (vfs *VirtualFilesystem) BindAt(ctx context.Context, creds *auth.Credential
// Checklocks doesn't work with anon functions.
vfs.setPropagation(clone, Private) // +checklocksforce
vfs.abortPropagationTree(ctx, tree) // +checklocksforce
vfs.mountMu.Unlock()
targetVd.DecRef(ctx)
})
defer cleanup.Clean()
if uint32(1+len(tree))+targetVd.mount.ns.mounts > MountMax {
vfs.mountMu.Unlock()
targetVd.DecRef(ctx)
return nil, linuxerr.ENOSPC
}
if err := vfs.connectMountAt(ctx, clone, targetVd); err != nil {
if err := vfs.connectMountAtLocked(ctx, clone, targetVd); err != nil {
vfs.mountMu.Unlock()
return nil, err
}
vfs.commitPropagationTree(ctx, tree)
test/syscalls/linux/mount.cc
+21
View File
@@ -24,6 +24,7 @@
#include <unistd.h>
#include <cstdint>
#include <cstdlib>
#include <functional>
#include <iostream>
#include <memory>
@@ -1418,6 +1419,26 @@ TEST(MountTest, MountInfoHasRoot) {
}
}
TEST(MountTest, DeadMountsAreDecRefd) {
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
DisableSave ds;
std::string home = NewTempAbsPath();
ASSERT_NO_ERRNO(Mkdir(home));
ASSERT_THAT(chdir(home.c_str()), SyscallSucceeds());
constexpr char dirpath[] = "./file";
for (int i = 0; i < 10; ++i) {
const auto rest = [&] {
mkdir(dirpath, 0);
mount(dirpath, ".", 0, MS_BIND, 0);
rmdir(dirpath);
mkdir(dirpath, 0);
mount(dirpath, ".", 0, MS_BIND, 0);
};
EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));
}
}
} // namespace
} // namespace testing