From d12e5c3406ca9b5f47bff741cc0c31ff0876a6d9 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Wed, 1 Mar 2023 19:42:36 -0800 Subject: [PATCH] Avoid calling openat with unix.AT_FDCWD from the sentry in directfs. We can achieve this by opening the current working directory for the sandbox process right before the application starts. We use this cached FD to restore CWD after DoInDir() operations. Since the package which provides DoInDir() has to be aware of the sandbox process and requirements around initializing the CWD, I have moved this to pkg/sentry/fsutil - which can contain sentry aware code. This will be used by directfs. This helps because this allows us to block AT_FDCWD in directfs seccomp filters. PiperOrigin-RevId: 513409592 --- pkg/fsutil/BUILD | 2 -- pkg/sentry/fsimpl/gofer/BUILD | 1 + pkg/sentry/fsimpl/gofer/directfs_dentry.go | 5 +-- pkg/sentry/fsutil/chdir/BUILD | 13 ++++++++ pkg/{fsutil => sentry/fsutil/chdir}/chdir.go | 35 ++++++++++++++++---- 5 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 pkg/sentry/fsutil/chdir/BUILD rename pkg/{fsutil => sentry/fsutil/chdir}/chdir.go (57%) diff --git a/pkg/fsutil/BUILD b/pkg/fsutil/BUILD index f3ab1ba90..ee85a1c58 100644 --- a/pkg/fsutil/BUILD +++ b/pkg/fsutil/BUILD @@ -5,7 +5,6 @@ licenses(["notice"]) go_library( name = "fsutil", srcs = [ - "chdir.go", "fsutil.go", "fsutil_amd64_unsafe.go", "fsutil_arm64_unsafe.go", @@ -13,7 +12,6 @@ go_library( ], visibility = ["//visibility:public"], deps = [ - "//pkg/sync", "//pkg/syserr", "@org_golang_x_sys//unix:go_default_library", ], diff --git a/pkg/sentry/fsimpl/gofer/BUILD b/pkg/sentry/fsimpl/gofer/BUILD index 102df1aea..d3df59a97 100644 --- a/pkg/sentry/fsimpl/gofer/BUILD +++ b/pkg/sentry/fsimpl/gofer/BUILD @@ -96,6 +96,7 @@ go_library( "//pkg/sentry/fsimpl/lock", "//pkg/sentry/fsmetric", "//pkg/sentry/fsutil", + "//pkg/sentry/fsutil/chdir", "//pkg/sentry/hostfd", "//pkg/sentry/kernel", "//pkg/sentry/kernel/auth", diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index 1b802a778..74e910a27 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/directfs_dentry.go @@ -27,6 +27,7 @@ import ( "gvisor.dev/gvisor/pkg/fsutil" "gvisor.dev/gvisor/pkg/lisafs" "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/fsutil/chdir" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/socket/unix/transport" "gvisor.dev/gvisor/pkg/sentry/vfs" @@ -490,7 +491,7 @@ func (d *directfsDentry) bindAt(ctx context.Context, name string, creds *auth.Cr // There are no filesystems mounted in the sandbox process's mount namespace. // So we can't perform absolute path traversals. So fchdir(2) to this // directory and bind at name (relative path traversal). - if err := fsutil.DoInDir(d.controlFD, func() error { + if err := chdir.DoInDir(d.controlFD, func() error { return unix.Bind(sockFD, &unix.SockaddrUnix{Name: name}) }); err != nil { hbep.ResetBoundSocketFD(ctx) @@ -617,7 +618,7 @@ func (d *directfsDentry) connect(ctx context.Context, sockType linux.SockType) ( // There are no filesystems mounted in the sandbox process's mount namespace. // So we can't perform absolute path traversals. So fchdir(2) to parent // and connect to this socket at name (relative path traversal). - if err := fsutil.DoInDir(d.parent.impl.(*directfsDentry).controlFD, func() error { + if err := chdir.DoInDir(d.parent.impl.(*directfsDentry).controlFD, func() error { return unix.Connect(sock, &unix.SockaddrUnix{Name: d.name}) }); err != nil { unix.Close(sock) diff --git a/pkg/sentry/fsutil/chdir/BUILD b/pkg/sentry/fsutil/chdir/BUILD new file mode 100644 index 000000000..98db91e10 --- /dev/null +++ b/pkg/sentry/fsutil/chdir/BUILD @@ -0,0 +1,13 @@ +load("//tools:defs.bzl", "go_library") + +package(licenses = ["notice"]) + +go_library( + name = "chdir", + srcs = ["chdir.go"], + visibility = ["//pkg/sentry:internal"], + deps = [ + "//pkg/sync", + "@org_golang_x_sys//unix:go_default_library", + ], +) diff --git a/pkg/fsutil/chdir.go b/pkg/sentry/fsutil/chdir/chdir.go similarity index 57% rename from pkg/fsutil/chdir.go rename to pkg/sentry/fsutil/chdir/chdir.go index 8d8c1fcd0..ac1fca9f9 100644 --- a/pkg/fsutil/chdir.go +++ b/pkg/sentry/fsutil/chdir/chdir.go @@ -12,31 +12,52 @@ // See the License for the specific language governing permissions and // limitations under the License. -package fsutil +// Package chdir provides utilities to control the sandbox process's current +// working directory. +package chdir import ( "fmt" + "os" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/sync" ) -// chdirMu is the global mutex that synchronizes host chdir operations. +// chdirMu is the global mutex that synchronizes host chdir operations for the +// sandbox process. var chdirMu sync.Mutex +// cwd is the current working directory for the sandbox process. The sandbox +// process usually runs in an empty chroot so cwd should be pointing to '/'. +// cwd is protected by chdirMu. +var cwd *os.File + +// InitCWD initializes the global cwd FD. InitCWD must be called after the +// sandbox process has been configured with pivot_root(2)/chroot(2). +func InitCWD() (err error) { + chdirMu.Lock() + defer chdirMu.Unlock() + if cwd != nil { + panic("InitCWD() called twice") + } + cwd, err = os.Open(".") + return +} + // DoInDir performs fn after chdir-ing to dirFD and then reverts back to the // original CWD. +// +// Precondition: InitCWD() must have been called. func DoInDir(dirFD int, fn func() error) error { chdirMu.Lock() defer chdirMu.Unlock() - - oldCWD, err := unix.Openat(unix.AT_FDCWD, ".", unix.O_PATH, 0 /* mode */) - if err != nil { - return err + if cwd == nil { + panic("DoInDir() called without calling InitCWD()") } defer func() { - if err := unix.Fchdir(oldCWD); err != nil { + if err := unix.Fchdir(int(cwd.Fd())); err != nil { panic(fmt.Errorf("restoring orginial CWD failed: %v", err)) } }()