From fe562179fea178b4044a4a30fe60264e2a6fa869 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Sun, 22 Jan 2023 18:57:15 -0800
Subject: [PATCH] Handle absolute symlink target '/' correctly in VFS layer.

vfs.ResolvingPath.relpathPrepend() has a precondition which was being violated
when handling resolveAbsSymlinkError. This was only happening when
`rp.absSymlinkTarget = "/"` because it has no path components. Added check for
that and also added regression test.

Reported-by: syzbot+48846f91b6252b56382f@syzkaller.appspotmail.com
PiperOrigin-RevId: 503862090
---
 pkg/sentry/fsimpl/gofer/dentry_impl.go   | 2 +-
 pkg/sentry/fsimpl/gofer/lisafs_dentry.go | 2 +-
 pkg/sentry/vfs/resolving_path.go         | 6 ++++--
 test/syscalls/linux/symlink.cc           | 9 +++++++++
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go
index 3de18e986..0ee85b248 100644
--- a/pkg/sentry/fsimpl/gofer/dentry_impl.go
+++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go
@@ -201,7 +201,7 @@ func (d *dentry) getRemoteChild(ctx context.Context, name string) (*dentry, erro
 //   - fs.renameMu must be locked.
 //   - parent.dirMu must be locked.
 //   - parent.isDir().
-//   - name is not "." or "..".
+//   - !rp.Done() && rp.Component() is not "." or "..".
 //   - dentry at name must not already exist in dentry tree.
 //
 // Postcondition: The returned dentry is already cached appropriately.
diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go
index fd1df694c..4899cb39a 100644
--- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go
+++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go
@@ -241,7 +241,7 @@ func (d *lisafsDentry) getRemoteChild(ctx context.Context, name string) (*dentry
 //   - fs.renameMu must be locked.
 //   - parent.dirMu must be locked.
 //   - parent.isDir().
-//   - name is not "." or "..".
+//   - !rp.Done().
 //   - dentry at name must not already exist in dentry tree.
 func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) {
 	// Walk as much of the path as possible in 1 RPC.
diff --git a/pkg/sentry/vfs/resolving_path.go b/pkg/sentry/vfs/resolving_path.go
index 169b80aed..3abbc37d8 100644
--- a/pkg/sentry/vfs/resolving_path.go
+++ b/pkg/sentry/vfs/resolving_path.go
@@ -438,8 +438,10 @@ func (rp *ResolvingPath) handleError(ctx context.Context, err error) bool {
 		rp.flags &^= rpflagsHaveMountRef | rpflagsHaveStartRef
 		// Consume the path component that represented the symlink.
 		rp.Advance()
-		// Prepend the symlink target to the relative path.
-		rp.relpathPrepend(rp.absSymlinkTarget)
+		if rp.absSymlinkTarget.HasComponents() {
+			// Prepend the symlink target to the relative path.
+			rp.relpathPrepend(rp.absSymlinkTarget)
+		}
 		// Restart path resolution on the new Mount.
 		rp.releaseErrorState(ctx)
 		return true
diff --git a/test/syscalls/linux/symlink.cc b/test/syscalls/linux/symlink.cc
index a88cae050..b629b71cd 100644
--- a/test/syscalls/linux/symlink.cc
+++ b/test/syscalls/linux/symlink.cc
@@ -365,6 +365,15 @@ TEST(SymlinkTest, SymlinkAtEmptyPath) {
               SyscallFailsWithErrno(ENOENT));
 }
 
+// NOTE(b/266111750): Regression test.
+TEST(SymlinkTest, AbsoluteSymlinkDouble) {
+  const std::string symlinkPath = NewTempAbsPath();
+  EXPECT_THAT(symlink("/", symlinkPath.c_str()), SyscallSucceeds());
+  auto doubleSymlinkPath = symlinkPath + symlinkPath;
+  EXPECT_THAT(mkdir(doubleSymlinkPath.c_str(), 0777),
+              SyscallFailsWithErrno(EEXIST));
+}
+
 class ParamSymlinkTest : public ::testing::TestWithParam<std::string> {};
 
 // Test that creating an existing symlink with creat will create the target.