From 26dd42a0ea84b800d708aa623bbd7a7281b67c9c Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 12 Mar 2024 16:53:21 -0700 Subject: [PATCH] Allow host FD to be restored with a different FD FD numbers can vary between depending on the options used with runsc command. For example, there are extra FDs passed to `runsc boot` if `debug-log` is enabled. So instead of requiring all FDs to have the exact same numbering during restore, provide a mechanism to remap the FD. Each host FD has a unique identifier with a map to their corresponding FD. Then during restore, FD numbers are remapped to the correct ones. Updates #1956 PiperOrigin-RevId: 615215783 --- pkg/sentry/control/lifecycle.go | 3 +- pkg/sentry/control/proc.go | 4 ++- pkg/sentry/fdimport/fdimport.go | 3 +- pkg/sentry/fsimpl/erofs/save_restore.go | 2 +- pkg/sentry/fsimpl/gofer/save_restore.go | 2 +- pkg/sentry/fsimpl/host/host.go | 37 +++++++++++++++---------- pkg/sentry/fsimpl/host/save_restore.go | 22 ++++++++++++++- pkg/sentry/vfs/context.go | 18 ++++++------ runsc/boot/loader.go | 2 +- runsc/boot/restore.go | 11 ++++++++ 10 files changed, 75 insertions(+), 29 deletions(-) diff --git a/pkg/sentry/control/lifecycle.go b/pkg/sentry/control/lifecycle.go index 554c205df..214b95d8e 100644 --- a/pkg/sentry/control/lifecycle.go +++ b/pkg/sentry/control/lifecycle.go @@ -255,7 +255,8 @@ func (l *Lifecycle) StartContainer(args *StartContainerArgs, _ *uint32) error { for i, appFD := range args.DonatedFDs { fdMap[appFD] = hostFDs[i] } - if _, err := fdimport.Import(ctx, fdTable, false, args.KUID, args.KGID, fdMap); err != nil { + // Use ContainerID since containers don't have names here. + if _, err := fdimport.Import(ctx, fdTable, false, args.KUID, args.KGID, fdMap, initArgs.ContainerID); err != nil { return fmt.Errorf("error importing host files: %w", err) } initArgs.FDTable = fdTable diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index 65b9a0c76..7fd4ffaf0 100644 --- a/pkg/sentry/control/proc.go +++ b/pkg/sentry/control/proc.go @@ -266,7 +266,9 @@ func (proc *Proc) execAsync(args *ExecArgs) (*kernel.ThreadGroup, kernel.ThreadI initArgs.Filename = resolved } - ttyFile, err := fdimport.Import(ctx, fdTable, args.StdioIsPty, args.KUID, args.KGID, fdMap) + // TODO(gvisor.dev/issue/1956): Container name is not really needed because + // exec processes are not restored, but add it for completeness. + ttyFile, err := fdimport.Import(ctx, fdTable, args.StdioIsPty, args.KUID, args.KGID, fdMap, "") if err != nil { return nil, 0, nil, err } diff --git a/pkg/sentry/fdimport/fdimport.go b/pkg/sentry/fdimport/fdimport.go index 718182352..33118a333 100644 --- a/pkg/sentry/fdimport/fdimport.go +++ b/pkg/sentry/fdimport/fdimport.go @@ -31,7 +31,7 @@ import ( // sets up TTY for sentry stdin, stdout, and stderr FDs. Used FDs are either // closed or released. It's safe for the caller to close any remaining files // upon return. -func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth.KUID, gid auth.KGID, fds map[int]*fd.FD) (*host.TTYFileDescription, error) { +func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth.KUID, gid auth.KGID, fds map[int]*fd.FD, containerName string) (*host.TTYFileDescription, error) { k := kernel.KernelFromContext(ctx) if k == nil { return nil, fmt.Errorf("cannot find kernel from context") @@ -62,6 +62,7 @@ func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth } var appFile *vfs.FileDescription + fdOpts.RestoreKey = host.MakeRestoreID(containerName, appFD) if console && appFD < 3 { // Import the file as a host TTY file. if ttyFile == nil { diff --git a/pkg/sentry/fsimpl/erofs/save_restore.go b/pkg/sentry/fsimpl/erofs/save_restore.go index cb405fbf1..d7001fe3d 100644 --- a/pkg/sentry/fsimpl/erofs/save_restore.go +++ b/pkg/sentry/fsimpl/erofs/save_restore.go @@ -25,7 +25,7 @@ import ( // afterLoad is called by stateify. func (fs *filesystem) afterLoad(ctx context.Context) { - fdmap := vfs.FilesystemFDMapFromContext(ctx) + fdmap := vfs.RestoreFilesystemFDMapFromContext(ctx) fd, ok := fdmap[fs.iopts.UniqueID] if !ok { panic(fmt.Sprintf("no image FD available for filesystem with unique ID %q", fs.iopts.UniqueID)) diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index 9e8decba8..ea537eb28 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -178,7 +178,7 @@ func (d *dentry) loadParent(_ goContext.Context, parent *dentry) { // CompleteRestore implements // vfs.FilesystemImplSaveRestoreExtension.CompleteRestore. func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRestoreOptions) error { - fdmap := vfs.FilesystemFDMapFromContext(ctx) + fdmap := vfs.RestoreFilesystemFDMapFromContext(ctx) if fdmap == nil { return fmt.Errorf("no server FD map available") } diff --git a/pkg/sentry/fsimpl/host/host.go b/pkg/sentry/fsimpl/host/host.go index b64315ae6..5087f13c1 100644 --- a/pkg/sentry/fsimpl/host/host.go +++ b/pkg/sentry/fsimpl/host/host.go @@ -107,12 +107,16 @@ type inode struct { // When the reference count reaches zero, the host fd is closed. inodeRefs - // hostFD contains the host fd that this file was originally created from, - // which must be available at time of restore. + // hostFD contains the host fd that this file was originally created from. + // It must be available at time of restore by being set to the same value or + // remapped using restoreKey and vfs.CtxRestoreFilesystemFDMap in the context. // // This field is initialized at creation time and is immutable. hostFD int + // restoreKey is used to identify the `hostFD` after a restore is performed. + restoreKey vfs.RestoreID + // ino is an inode number unique within this filesystem. // // This field is initialized at creation time and is immutable. @@ -167,7 +171,7 @@ type inode struct { buf []byte } -func newInode(ctx context.Context, fs *filesystem, hostFD int, savable bool, fileType linux.FileMode, isTTY bool, readonly bool) (*inode, error) { +func newInode(ctx context.Context, fs *filesystem, hostFD int, savable bool, restoreKey vfs.RestoreID, fileType linux.FileMode, isTTY bool, readonly bool) (*inode, error) { // Determine if hostFD is seekable. _, err := unix.Seek(hostFD, 0, linux.SEEK_CUR) seekable := !linuxerr.Equals(linuxerr.ESPIPE, err) @@ -179,14 +183,15 @@ func newInode(ctx context.Context, fs *filesystem, hostFD int, savable bool, fil } i := &inode{ - hostFD: hostFD, - ino: fs.NextIno(), - ftype: uint16(fileType), - epollable: isEpollable(hostFD), - seekable: seekable, - isTTY: isTTY, - savable: savable, - readonly: readonly, + hostFD: hostFD, + ino: fs.NextIno(), + ftype: uint16(fileType), + epollable: isEpollable(hostFD), + seekable: seekable, + isTTY: isTTY, + savable: savable, + restoreKey: restoreKey, + readonly: readonly, } i.InitRefs() i.CachedMappable.Init(hostFD) @@ -207,10 +212,14 @@ func newInode(ctx context.Context, fs *filesystem, hostFD int, savable bool, fil // NewFDOptions contains options to NewFD. type NewFDOptions struct { // If Savable is true, the host file descriptor may be saved/restored by - // numeric value; the sandbox API requires a corresponding host FD with the - // same numeric value to be provided at time of restore. + // numeric value. RestoreKey is used to map the FD after restore. Savable bool + // RestoreKey is only used when Savable==true. It uniquely identifies the + // host FD so that a mapping to the corresponding FD can be provided during + // restore. + RestoreKey vfs.RestoreID + // If IsTTY is true, the file descriptor is a TTY. IsTTY bool @@ -272,7 +281,7 @@ func NewFD(ctx context.Context, mnt *vfs.Mount, hostFD int, opts *NewFDOptions) } fileType := linux.FileMode(stat.Mode).FileType() - i, err := newInode(ctx, fs, hostFD, opts.Savable, fileType, opts.IsTTY, opts.Readonly) + i, err := newInode(ctx, fs, hostFD, opts.Savable, opts.RestoreKey, fileType, opts.IsTTY, opts.Readonly) if err != nil { return nil, err } diff --git a/pkg/sentry/fsimpl/host/save_restore.go b/pkg/sentry/fsimpl/host/save_restore.go index 0f654bc29..3cf29b93a 100644 --- a/pkg/sentry/fsimpl/host/save_restore.go +++ b/pkg/sentry/fsimpl/host/save_restore.go @@ -24,8 +24,20 @@ import ( "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/safemem" "gvisor.dev/gvisor/pkg/sentry/hostfd" + "gvisor.dev/gvisor/pkg/sentry/vfs" ) +// MakeRestoreID creates a RestoreID for a given application FD. The application +// FD remains the same between restores, e.g. stdout=2 before and after restore, +// but the host FD that is maps to can change between restores. This ID is used +// to map application FDs to their respective FD after a restore happens. +func MakeRestoreID(containerName string, fd int) vfs.RestoreID { + return vfs.RestoreID{ + ContainerName: containerName, + Path: fmt.Sprintf("host:%d", fd), + } +} + // beforeSave is invoked by stateify. func (i *inode) beforeSave() { if !i.savable { @@ -58,7 +70,15 @@ func (i *inode) beforeSave() { } // afterLoad is invoked by stateify. -func (i *inode) afterLoad(context.Context) { +func (i *inode) afterLoad(ctx context.Context) { + fdmap := vfs.RestoreFilesystemFDMapFromContext(ctx) + fd, ok := fdmap[i.restoreKey] + if ok { + // Remap FD if a new mapping is provided. Otherwise, keep the old FD and + // expect that caller will use the same FDs numbers. + i.hostFD = fd + } + if i.epollable { if err := unix.SetNonblock(i.hostFD, true); err != nil { panic(fmt.Sprintf("host.inode.afterLoad: failed to set host FD %d non-blocking: %v", i.hostFD, err)) diff --git a/pkg/sentry/vfs/context.go b/pkg/sentry/vfs/context.go index 92ba0d5ca..ebf1b41c3 100644 --- a/pkg/sentry/vfs/context.go +++ b/pkg/sentry/vfs/context.go @@ -47,6 +47,16 @@ func MountNamespaceFromContext(ctx goContext.Context) *MountNamespace { return nil } +// RestoreFilesystemFDMapFromContext returns the RestoreFilesystemFDMap used +// by ctx. If ctx is not associated with a RestoreFilesystemFDMap, returns nil. +func RestoreFilesystemFDMapFromContext(ctx goContext.Context) map[RestoreID]int { + fdmap, ok := ctx.Value(CtxRestoreFilesystemFDMap).(map[RestoreID]int) + if !ok { + return nil + } + return fdmap +} + type mountNamespaceContext struct { context.Context mntns *MountNamespace @@ -104,11 +114,3 @@ func (rc rootContext) Value(key any) any { return rc.Context.Value(key) } } - -// FilesystemFDMapFromContext returns the CtxRestoreFilesystemFDMap from ctx. -func FilesystemFDMapFromContext(ctx goContext.Context) map[RestoreID]int { - if v := ctx.Value(CtxRestoreFilesystemFDMap); v != nil { - return v.(map[RestoreID]int) - } - return nil -} diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index a526400a1..b4b2c7948 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -1584,7 +1584,7 @@ func createFDTable(ctx context.Context, console bool, stdioFDs []*fd.FD, passFDs k := kernel.KernelFromContext(ctx) fdTable := k.NewFDTable() - ttyFile, err := fdimport.Import(ctx, fdTable, console, auth.KUID(user.UID), auth.KGID(user.GID), fdMap) + ttyFile, err := fdimport.Import(ctx, fdTable, console, auth.KUID(user.UID), auth.KGID(user.GID), fdMap, containerName) if err != nil { fdTable.DecRef(ctx) return nil, nil, err diff --git a/runsc/boot/restore.go b/runsc/boot/restore.go index da44402a5..76d931c39 100644 --- a/runsc/boot/restore.go +++ b/runsc/boot/restore.go @@ -19,6 +19,7 @@ import ( "os" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/sentry/fsimpl/host" "gvisor.dev/gvisor/pkg/sentry/inet" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/socket/hostinet" @@ -118,6 +119,16 @@ func (r *restorer) restore(l *Loader) error { return fmt.Errorf("configuring filesystem restore: %v", err) } + fdmap := vfs.RestoreFilesystemFDMapFromContext(ctx) + for appFD, fd := range r.container.stdioFDs { + key := host.MakeRestoreID(r.container.containerName, appFD) + fdmap[key] = fd.Release() + } + for _, customFD := range r.container.passFDs { + key := host.MakeRestoreID(r.container.containerName, customFD.guest) + fdmap[key] = customFD.host.FD() + } + // Load the state. loadOpts := state.LoadOpts{Source: r.stateFile} if err := loadOpts.Load(ctx, l.k, nil, netns.Stack(), time.NewCalibratedClocks(), &vfs.CompleteRestoreOptions{}); err != nil {