From 82719be42e636f86780d21b01e10ecb2c9a25e53 Mon Sep 17 00:00:00 2001 From: Brian Geffon Date: Tue, 4 Dec 2018 14:31:08 -0800 Subject: Max link traversals should be for an entire path. The number of symbolic links that are allowed to be followed are for a full path and not just a chain of symbolic links. PiperOrigin-RevId: 224047321 Change-Id: I5e3c4caf66a93c17eeddcc7f046d1e8bb9434a40 --- pkg/sentry/fs/copy_up_test.go | 3 ++- pkg/sentry/fs/host/fs.go | 3 ++- pkg/sentry/fs/host/fs_test.go | 3 ++- pkg/sentry/fs/inode_overlay_test.go | 3 ++- pkg/sentry/fs/mount_test.go | 10 +++++++--- pkg/sentry/fs/mounts.go | 22 ++++++++++++---------- pkg/sentry/fs/mounts_test.go | 6 ++++-- pkg/sentry/fs/ramfs/tree_test.go | 3 ++- 8 files changed, 33 insertions(+), 20 deletions(-) (limited to 'pkg/sentry/fs') diff --git a/pkg/sentry/fs/copy_up_test.go b/pkg/sentry/fs/copy_up_test.go index 64f030f72..fcba14ed4 100644 --- a/pkg/sentry/fs/copy_up_test.go +++ b/pkg/sentry/fs/copy_up_test.go @@ -166,7 +166,8 @@ func makeOverlayTestFiles(t *testing.T) []*overlayTestFile { // Walk to all of the files in the overlay, open them readable. for _, f := range files { - d, err := mns.FindInode(ctx, mns.Root(), mns.Root(), f.name, 0) + maxTraversals := uint(0) + d, err := mns.FindInode(ctx, mns.Root(), mns.Root(), f.name, &maxTraversals) if err != nil { t.Fatalf("failed to find %q: %v", f.name, err) } diff --git a/pkg/sentry/fs/host/fs.go b/pkg/sentry/fs/host/fs.go index fec890964..54cbb94f9 100644 --- a/pkg/sentry/fs/host/fs.go +++ b/pkg/sentry/fs/host/fs.go @@ -170,7 +170,8 @@ func installWhitelist(ctx context.Context, m *fs.MountNamespace, paths []string) current := paths[i][:j] // Lookup the given component in the tree. - d, err := m.FindLink(ctx, root, nil, current, maxTraversals) + remainingTraversals := uint(maxTraversals) + d, err := m.FindLink(ctx, root, nil, current, &remainingTraversals) if err != nil { log.Warningf("populate failed for %q: %v", current, err) continue diff --git a/pkg/sentry/fs/host/fs_test.go b/pkg/sentry/fs/host/fs_test.go index e69559aac..44db61ecd 100644 --- a/pkg/sentry/fs/host/fs_test.go +++ b/pkg/sentry/fs/host/fs_test.go @@ -150,7 +150,8 @@ func allPaths(ctx context.Context, t *testing.T, m *fs.MountNamespace, base stri root := m.Root() defer root.DecRef() - d, err := m.FindLink(ctx, root, nil, base, 1) + maxTraversals := uint(1) + d, err := m.FindLink(ctx, root, nil, base, &maxTraversals) if err != nil { t.Logf("FindLink failed for %q", base) return paths, err diff --git a/pkg/sentry/fs/inode_overlay_test.go b/pkg/sentry/fs/inode_overlay_test.go index bba20da14..acdb2b4f8 100644 --- a/pkg/sentry/fs/inode_overlay_test.go +++ b/pkg/sentry/fs/inode_overlay_test.go @@ -324,7 +324,8 @@ func TestCacheFlush(t *testing.T) { for _, fileName := range []string{upperFileName, lowerFileName} { // Walk to the file. - dirent, err := mns.FindInode(ctx, root, nil, fileName, 0) + maxTraversals := uint(0) + dirent, err := mns.FindInode(ctx, root, nil, fileName, &maxTraversals) if err != nil { t.Fatalf("FindInode(%q) failed: %v", fileName, err) } diff --git a/pkg/sentry/fs/mount_test.go b/pkg/sentry/fs/mount_test.go index a1c9f4f79..269d6b9da 100644 --- a/pkg/sentry/fs/mount_test.go +++ b/pkg/sentry/fs/mount_test.go @@ -115,8 +115,10 @@ func TestMountSourceParentChildRelationship(t *testing.T) { "/waldo", } + var maxTraversals uint for _, p := range paths { - d, err := mm.FindLink(ctx, rootDirent, nil, p, 0) + maxTraversals = 0 + d, err := mm.FindLink(ctx, rootDirent, nil, p, &maxTraversals) if err != nil { t.Fatalf("could not find path %q in mount manager: %v", p, err) } @@ -164,7 +166,8 @@ func TestMountSourceParentChildRelationship(t *testing.T) { } // "foo" mount should have two children: /foo/bar, and /foo/qux. - d, err := mm.FindLink(ctx, rootDirent, nil, "/foo", 0) + maxTraversals = 0 + d, err := mm.FindLink(ctx, rootDirent, nil, "/foo", &maxTraversals) if err != nil { t.Fatalf("could not find path %q in mount manager: %v", "/foo", err) } @@ -185,7 +188,8 @@ func TestMountSourceParentChildRelationship(t *testing.T) { } // "waldo" mount should have no submounts or children. - waldo, err := mm.FindLink(ctx, rootDirent, nil, "/waldo", 0) + maxTraversals = 0 + waldo, err := mm.FindLink(ctx, rootDirent, nil, "/waldo", &maxTraversals) if err != nil { t.Fatalf("could not find path %q in mount manager: %v", "/waldo", err) } diff --git a/pkg/sentry/fs/mounts.go b/pkg/sentry/fs/mounts.go index 7c5348cce..f6f7be0aa 100644 --- a/pkg/sentry/fs/mounts.go +++ b/pkg/sentry/fs/mounts.go @@ -350,7 +350,7 @@ func (mns *MountNamespace) Unmount(ctx context.Context, node *Dirent, detachOnly // // Precondition: root must be non-nil. // Precondition: the path must be non-empty. -func (mns *MountNamespace) FindLink(ctx context.Context, root, wd *Dirent, path string, maxTraversals uint) (*Dirent, error) { +func (mns *MountNamespace) FindLink(ctx context.Context, root, wd *Dirent, path string, remainingTraversals *uint) (*Dirent, error) { if root == nil { panic("MountNamespace.FindLink: root must not be nil") } @@ -419,7 +419,7 @@ func (mns *MountNamespace) FindLink(ctx context.Context, root, wd *Dirent, path // // See resolve for reference semantics; on err next // will have one dropped. - current, err = mns.resolve(ctx, root, next, maxTraversals) + current, err = mns.resolve(ctx, root, next, remainingTraversals) if err != nil { return nil, err } @@ -439,15 +439,15 @@ func (mns *MountNamespace) FindLink(ctx context.Context, root, wd *Dirent, path // FindInode is identical to FindLink except the return value is resolved. // //go:nosplit -func (mns *MountNamespace) FindInode(ctx context.Context, root, wd *Dirent, path string, maxTraversals uint) (*Dirent, error) { - d, err := mns.FindLink(ctx, root, wd, path, maxTraversals) +func (mns *MountNamespace) FindInode(ctx context.Context, root, wd *Dirent, path string, remainingTraversals *uint) (*Dirent, error) { + d, err := mns.FindLink(ctx, root, wd, path, remainingTraversals) if err != nil { return nil, err } // See resolve for reference semantics; on err d will have the // reference dropped. - return mns.resolve(ctx, root, d, maxTraversals) + return mns.resolve(ctx, root, d, remainingTraversals) } // resolve resolves the given link. @@ -458,14 +458,14 @@ func (mns *MountNamespace) FindInode(ctx context.Context, root, wd *Dirent, path // If not successful, a reference is _also_ dropped on the node and an error // returned. This is for convenience in using resolve directly as a return // value. -func (mns *MountNamespace) resolve(ctx context.Context, root, node *Dirent, maxTraversals uint) (*Dirent, error) { +func (mns *MountNamespace) resolve(ctx context.Context, root, node *Dirent, remainingTraversals *uint) (*Dirent, error) { // Resolve the path. target, err := node.Inode.Getlink(ctx) switch err { case nil: // Make sure we didn't exhaust the traversal budget. - if maxTraversals == 0 { + if *remainingTraversals == 0 { target.DecRef() return nil, syscall.ELOOP } @@ -481,7 +481,7 @@ func (mns *MountNamespace) resolve(ctx context.Context, root, node *Dirent, maxT defer node.DecRef() // See above. // First, check if we should traverse. - if maxTraversals == 0 { + if *remainingTraversals == 0 { return nil, syscall.ELOOP } @@ -492,7 +492,8 @@ func (mns *MountNamespace) resolve(ctx context.Context, root, node *Dirent, maxT } // Find the node; we resolve relative to the current symlink's parent. - d, err := mns.FindInode(ctx, root, node.parent, targetPath, maxTraversals-1) + *remainingTraversals-- + d, err := mns.FindInode(ctx, root, node.parent, targetPath, remainingTraversals) if err != nil { return nil, err } @@ -544,7 +545,8 @@ func (mns *MountNamespace) ResolveExecutablePath(ctx context.Context, wd, name s defer root.DecRef() for _, p := range paths { binPath := path.Join(p, name) - d, err := mns.FindInode(ctx, root, nil, binPath, linux.MaxSymlinkTraversals) + traversals := uint(linux.MaxSymlinkTraversals) + d, err := mns.FindInode(ctx, root, nil, binPath, &traversals) if err == syserror.ENOENT || err == syserror.EACCES { // Didn't find it here. continue diff --git a/pkg/sentry/fs/mounts_test.go b/pkg/sentry/fs/mounts_test.go index cc7c32c9b..2f7a1710f 100644 --- a/pkg/sentry/fs/mounts_test.go +++ b/pkg/sentry/fs/mounts_test.go @@ -77,7 +77,8 @@ func TestFindLink(t *testing.T) { {"bar", foo, "/foo/bar"}, } { wdPath, _ := tc.wd.FullName(root) - if d, err := mm.FindLink(ctx, root, tc.wd, tc.findPath, 0); err != nil { + maxTraversals := uint(0) + if d, err := mm.FindLink(ctx, root, tc.wd, tc.findPath, &maxTraversals); err != nil { t.Errorf("FindLink(%q, wd=%q) failed: %v", tc.findPath, wdPath, err) } else if got, _ := d.FullName(root); got != tc.wantPath { t.Errorf("FindLink(%q, wd=%q) got dirent %q, want %q", tc.findPath, wdPath, got, tc.wantPath) @@ -95,7 +96,8 @@ func TestFindLink(t *testing.T) { {"foo", foo}, } { wdPath, _ := tc.wd.FullName(root) - if _, err := mm.FindLink(ctx, root, tc.wd, tc.findPath, 0); err == nil { + maxTraversals := uint(0) + if _, err := mm.FindLink(ctx, root, tc.wd, tc.findPath, &maxTraversals); err == nil { t.Errorf("FindLink(%q, wd=%q) did not return error", tc.findPath, wdPath) } } diff --git a/pkg/sentry/fs/ramfs/tree_test.go b/pkg/sentry/fs/ramfs/tree_test.go index d5567d9e1..54df2143c 100644 --- a/pkg/sentry/fs/ramfs/tree_test.go +++ b/pkg/sentry/fs/ramfs/tree_test.go @@ -70,7 +70,8 @@ func TestMakeDirectoryTree(t *testing.T) { defer mm.DecRef() for _, p := range test.subdirs { - if _, err := mm.FindInode(ctx, root, nil, p, 0); err != nil { + maxTraversals := uint(0) + if _, err := mm.FindInode(ctx, root, nil, p, &maxTraversals); err != nil { t.Errorf("%s: failed to find node %s: %v", test.name, p, err) break } -- cgit v1.2.3