diff --git a/pkg/lisafs/client.go b/pkg/lisafs/client.go index 933e7a0bb..4e4f12513 100644 --- a/pkg/lisafs/client.go +++ b/pkg/lisafs/client.go @@ -72,7 +72,7 @@ type Client struct { // the server and creates channels for fast IPC. NewClient takes ownership over // the passed socket. On success, it returns the initialized client along with // the root Inode. -func NewClient(sock *unet.Socket, mountPath string) (*Client, *Inode, error) { +func NewClient(sock *unet.Socket) (*Client, *Inode, error) { maxChans := maxChannels() c := &Client{ sockComm: newSockComm(sock), @@ -97,11 +97,8 @@ func NewClient(sock *unet.Socket, mountPath string) (*Client, *Inode, error) { // Mount RPC below. c.supported = make([]bool, Mount+1) c.supported[Mount] = true - mountMsg := MountReq{ - MountPath: SizedString(mountPath), - } var mountResp MountResp - if err := c.SndRcvMessage(Mount, uint32(mountMsg.SizeBytes()), mountMsg.MarshalBytes, mountResp.CheckedUnmarshal, nil); err != nil { + if err := c.SndRcvMessage(Mount, 0, NoopMarshal, mountResp.CheckedUnmarshal, nil); err != nil { return nil, nil, err } diff --git a/pkg/lisafs/connection.go b/pkg/lisafs/connection.go index f53b4ccf1..38d769e7d 100644 --- a/pkg/lisafs/connection.go +++ b/pkg/lisafs/connection.go @@ -15,6 +15,8 @@ package lisafs import ( + "path" + "path/filepath" "runtime/debug" "golang.org/x/sys/unix" @@ -44,13 +46,15 @@ type Connection struct { // associated with it for its entire lifetime. server *Server + // mountPath is the path to a file inside the server that is served to this + // connection as its root FD. IOW, this connection is mounted at this path. + // mountPath is trusted because it is configured by the server (trusted) as + // per the user's sandbox configuration. mountPath is immutable. + mountPath string + // maxMessageSize is the cached value of server.impl.MaxMessageSize(). maxMessageSize uint32 - // mounted is a one way flag indicating whether this connection has been - // mounted correctly and the server is initialized properly. - mounted bool - // readonly indicates if this connection is readonly. All write operations // will fail with EROFS. readonly bool @@ -79,13 +83,20 @@ type Connection struct { nextFDID FDID } -// CreateConnection initializes a new connection - creating a server if -// required. The connection must be started separately. -func (s *Server) CreateConnection(sock *unet.Socket, readonly bool) (*Connection, error) { +// CreateConnection initializes a new connection which will be mounted at +// mountPath. The connection must be started separately. +func (s *Server) CreateConnection(sock *unet.Socket, mountPath string, readonly bool) (*Connection, error) { + mountPath = path.Clean(mountPath) + if !filepath.IsAbs(mountPath) { + log.Warningf("mountPath %q is not absolute", mountPath) + return nil, unix.EINVAL + } + c := &Connection{ sockComm: newSockComm(sock), server: s, maxMessageSize: s.impl.MaxMessageSize(), + mountPath: mountPath, readonly: readonly, channels: make([]*channel, 0, maxChannels()), fds: make(map[FDID]genericFD), @@ -183,11 +194,6 @@ func (c *Connection) handleMsg(comm Communicator, m MID, payloadLen uint32) (ret } }() - if !c.mounted && m != Mount { - log.Warningf("connection must first be mounted") - return c.respondError(comm, unix.EINVAL) - } - // Check if the message is supported for forward compatibility. if int(m) >= len(c.server.handlers) || c.server.handlers[m] == nil { log.Warningf("received request which is not supported by the server, MID = %d", m) diff --git a/pkg/lisafs/connection_test.go b/pkg/lisafs/connection_test.go index 2d3b40692..1d1e430de 100644 --- a/pkg/lisafs/connection_test.go +++ b/pkg/lisafs/connection_test.go @@ -57,9 +57,10 @@ func (fd *testControlFD) FD() *lisafs.ControlFD { func (fd *testControlFD) Close() {} // Mount implements lisafs.Mount. -func (s *testServer) Mount(c *lisafs.Connection) (*lisafs.ControlFD, linux.Statx, error) { +func (s *testServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lisafs.ControlFD, linux.Statx, error) { dummyRoot := &testControlFD{} - dummyRoot.Init(c, s.Root(), linux.ModeDirectory, dummyRoot) + mountNode.IncRef() // Ref is transferred to ControlFD. + dummyRoot.Init(c, mountNode, linux.ModeDirectory, dummyRoot) return dummyRoot.FD(), linux.Statx{Mode: linux.S_IFDIR}, nil } @@ -85,16 +86,16 @@ func runServerClient(t testing.TB, clientFn func(c *lisafs.Client)) { } ts := &testServer{} - ts.Server.Init(ts, lisafs.ServerOpts{}) - ts.Server.SetHandlers(handlers[:]) - conn, err := ts.CreateConnection(serverSocket, false /* readonly */) + ts.Init(ts, lisafs.ServerOpts{}) + ts.SetHandlers(handlers[:]) + conn, err := ts.CreateConnection(serverSocket, "/" /* mountPath */, false /* readonly */) if err != nil { t.Fatalf("starting connection failed: %v", err) return } ts.StartConnection(conn) - c, _, err := lisafs.NewClient(clientSocket, "/") + c, _, err := lisafs.NewClient(clientSocket) if err != nil { t.Fatalf("client creation failed: %v", err) } diff --git a/pkg/lisafs/handlers.go b/pkg/lisafs/handlers.go index 8eb8b2437..02a1701d3 100644 --- a/pkg/lisafs/handlers.go +++ b/pkg/lisafs/handlers.go @@ -17,8 +17,6 @@ package lisafs import ( "fmt" "math" - "path" - "path/filepath" "strings" "golang.org/x/sys/unix" @@ -90,53 +88,60 @@ func ErrorHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32, // that Mount is the first message on the connection. Only after the connection // has been successfully mounted can other channels be created. func MountHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32, error) { - var req MountReq - if _, ok := req.CheckedUnmarshal(comm.PayloadBuf(payloadLen)); !ok { - return 0, unix.EIO - } - - mountPath := path.Clean(string(req.MountPath)) - if !filepath.IsAbs(mountPath) { - log.Warningf("mountPath %q is not absolute", mountPath) - return 0, unix.EINVAL - } - - if c.mounted { - log.Warningf("connection has already been mounted at %q", mountPath) - return 0, unix.EBUSY - } - var ( mountPointFD *ControlFD mountPointStat linux.Statx + mountNode = c.server.root ) if err := c.server.withRenameReadLock(func() (err error) { - mountPointFD, mountPointStat, err = c.ServerImpl().Mount(c) - if err != nil { - return err - } + // Maintain extra ref on mountNode to ensure existence during walk. + mountNode.IncRef() + defer func() { + // Drop extra ref on mountNode. Wrap the defer call with a func so that + // mountNode is evaluated on execution, not on defer itself. + mountNode.DecRef(nil) + }() - for pit := fspath.Parse(mountPath).Begin; pit.Ok(); pit = pit.Next() { - mountPointFD.node.opMu.RLock() - nextFD, nextStat, err := mountPointFD.impl.Walk(pit.String()) - mountPointFD.node.opMu.RUnlock() - if err != nil { + // Walk to the mountpoint. + pit := fspath.Parse(c.mountPath).Begin + for pit.Ok() { + curName := pit.String() + if err := checkSafeName(curName); err != nil { return err } - - c.removeControlFDLocked(mountPointFD.id) - mountPointFD = nextFD - mountPointStat = nextStat + mountNode.opMu.RLock() + if mountNode.isDeleted() { + mountNode.opMu.RUnlock() + return unix.ENOENT + } + mountNode.childrenMu.Lock() + next := mountNode.LookupChildLocked(curName) + if next == nil { + next = &Node{} + next.InitLocked(curName, mountNode) + } else { + next.IncRef() + } + mountNode.childrenMu.Unlock() + mountNode.opMu.RUnlock() + // next has an extra ref as needed. Drop extra ref on mountNode. + mountNode.DecRef(nil) + pit = pit.Next() + mountNode = next } + + // Provide Mount with read concurrency guarantee. + mountNode.opMu.RLock() + defer mountNode.opMu.RUnlock() + if mountNode.isDeleted() { + return unix.ENOENT + } + mountPointFD, mountPointStat, err = c.ServerImpl().Mount(c, mountNode) return err }); err != nil { - if mountPointFD != nil { - c.removeFD(mountPointFD.id) - } return 0, err } - c.mounted = true resp := MountResp{ Root: Inode{ ControlFD: mountPointFD.id, diff --git a/pkg/lisafs/message.go b/pkg/lisafs/message.go index 9da87e771..d0370647c 100644 --- a/pkg/lisafs/message.go +++ b/pkg/lisafs/message.go @@ -268,26 +268,6 @@ type Inode struct { Stat linux.Statx } -// MountReq represents a Mount request. -type MountReq struct { - MountPath SizedString -} - -// SizeBytes implements marshal.Marshallable.SizeBytes. -func (m *MountReq) SizeBytes() int { - return m.MountPath.SizeBytes() -} - -// MarshalBytes implements marshal.Marshallable.MarshalBytes. -func (m *MountReq) MarshalBytes(dst []byte) []byte { - return m.MountPath.MarshalBytes(dst) -} - -// CheckedUnmarshal implements marshal.CheckedMarshallable.CheckedUnmarshal. -func (m *MountReq) CheckedUnmarshal(src []byte) ([]byte, bool) { - return m.MountPath.CheckedUnmarshal(src) -} - // MountResp represents a Mount response. type MountResp struct { Root Inode diff --git a/pkg/lisafs/server.go b/pkg/lisafs/server.go index de02aec06..681532960 100644 --- a/pkg/lisafs/server.go +++ b/pkg/lisafs/server.go @@ -103,20 +103,15 @@ func (s *Server) Wait() { s.connWg.Wait() } -// Root returns the server's root node. -func (s *Server) Root() *Node { - return s.root -} - // ServerImpl contains the implementation details for a Server. // Implementations of ServerImpl should contain their associated Server by // value as their first field. type ServerImpl interface { // Mount is called when a Mount RPC is made. It mounts the connection on - // filesystem root. + // mountNode. // - // Mount has rename read concurrency guarantee. - Mount(c *Connection) (*ControlFD, linux.Statx, error) + // Mount has a read concurrency guarantee on mountNode. + Mount(c *Connection, mountNode *Node) (*ControlFD, linux.Statx, error) // SupportedMessages returns a list of messages that the server // implementation supports. diff --git a/pkg/lisafs/testsuite/testsuite.go b/pkg/lisafs/testsuite/testsuite.go index 83883b184..936d70f77 100644 --- a/pkg/lisafs/testsuite/testsuite.go +++ b/pkg/lisafs/testsuite/testsuite.go @@ -91,14 +91,14 @@ func runServerClient(t *testing.T, tester Tester, testFn testFunc) { } server := tester.NewServer(t) - conn, err := server.CreateConnection(serverSocket, false /* readonly */) + conn, err := server.CreateConnection(serverSocket, mountPath, false /* readonly */) if err != nil { t.Fatalf("starting connection failed: %v", err) return } server.StartConnection(conn) - c, root, err := lisafs.NewClient(clientSocket, mountPath) + c, root, err := lisafs.NewClient(clientSocket) if err != nil { t.Fatalf("client creation failed: %v", err) } diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 0399e2233..3fd65ee7c 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -39,6 +39,7 @@ package gofer import ( "fmt" + "path" "strconv" "strings" "sync/atomic" @@ -350,7 +351,11 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt fsopts.aname = "/" if aname, ok := mopts[moptAname]; ok { delete(mopts, moptAname) - fsopts.aname = aname + if !path.IsAbs(aname) { + ctx.Warningf("gofer.FilesystemType.GetFilesystem: aname is not absolute: %s=%s", moptAname, aname) + return nil, nil, linuxerr.EINVAL + } + fsopts.aname = path.Clean(aname) } // Parse the cache policy. For historical reasons, this defaults to the @@ -527,9 +532,39 @@ func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error) var rootInode *lisafs.Inode ctx.UninterruptibleSleepStart(false) - fs.clientLisa, rootInode, err = lisafs.NewClient(sock, fs.opts.aname) + fs.clientLisa, rootInode, err = lisafs.NewClient(sock) ctx.UninterruptibleSleepFinish(false) - return rootInode, err + if err != nil { + return nil, err + } + if fs.opts.aname == "/" { + return rootInode, nil + } + + // Walk to the attach point from root inode. + rootFD := fs.clientLisa.NewFD(rootInode.ControlFD) + status, inodes, err := rootFD.WalkMultiple(ctx, strings.Split(fs.opts.aname, "/")) + rootFD.CloseBatched(ctx) + if err != nil { + return nil, err + } + + // Close all intermediate FDs to the attach point. + numInodes := len(inodes) + for _, inode := range inodes[:numInodes-1] { + curFD := fs.clientLisa.NewFD(inode.ControlFD) + curFD.CloseBatched(ctx) + } + + switch status { + case lisafs.WalkSuccess: + return &inodes[numInodes-1], nil + default: + last := fs.clientLisa.NewFD(inodes[numInodes-1].ControlFD) + last.CloseBatched(ctx) + log.Warningf("initClientLisa failed because walk to attach point %q failed: lisafs.WalkStatus = %v", fs.opts.aname, status) + return nil, unix.ENOENT + } } func (fs *filesystem) initClient(ctx context.Context) (*dentry, error) { diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index f02818743..3ef746b54 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -189,7 +189,7 @@ func compileMounts(spec *specs.Spec, conf *config.Config, vfs2Enabled bool) []sp } // goferMountData creates a slice of gofer mount data. -func goferMountData(fd int, fa config.FileAccessType, attachPath string, vfs2 bool, lisafs bool) []string { +func goferMountData(fd int, fa config.FileAccessType, vfs2 bool, lisafs bool) []string { opts := []string{ "trans=fd", "rfdno=" + strconv.Itoa(fd), @@ -205,7 +205,6 @@ func goferMountData(fd int, fa config.FileAccessType, attachPath string, vfs2 bo } if vfs2 && lisafs { opts = append(opts, "lisafs=true") - opts = append(opts, "aname="+attachPath) } return opts } @@ -784,7 +783,7 @@ func (c *containerMounter) createRootMount(ctx context.Context, conf *config.Con fd := c.fds.remove() log.Infof("Mounting root over 9P, ioFD: %d", fd) p9FS := mustFindFilesystem("9p") - opts := goferMountData(fd, conf.FileAccess, "/", false /* vfs2 */, false /* lisafs */) + opts := goferMountData(fd, conf.FileAccess, false /* vfs2 */, false /* lisafs */) // We can't check for overlayfs here because sandbox is chroot'ed and gofer // can only send mount options for specs.Mounts (specs.Root is missing @@ -845,7 +844,7 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *specs. case bind: fd := c.fds.remove() fsName = gofervfs2.Name - opts = goferMountData(fd, c.getMountAccessType(conf, m), m.Destination, conf.VFS2, conf.Lisafs) + opts = goferMountData(fd, c.getMountAccessType(conf, m), conf.VFS2, conf.Lisafs) // If configured, add overlay to all writable mounts. useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly case cgroupfs.Name: @@ -1003,7 +1002,7 @@ func (c *containerMounter) createRestoreEnvironment(conf *config.Config) (*fs.Re // Add root mount. fd := c.fds.remove() - opts := goferMountData(fd, conf.FileAccess, "/", conf.VFS2, false /* lisafs */) + opts := goferMountData(fd, conf.FileAccess, conf.VFS2, false /* lisafs */) mf := fs.MountSourceFlags{} if c.root.Readonly || conf.Overlay { diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index a2b18abb9..aaf7e5672 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -213,7 +213,7 @@ func (c *containerMounter) mountAll(conf *config.Config, procArgs *kernel.Create // createMountNamespaceVFS2 creates the container's root mount and namespace. func (c *containerMounter) createMountNamespaceVFS2(ctx context.Context, conf *config.Config, creds *auth.Credentials) (*vfs.MountNamespace, error) { fd := c.fds.remove() - data := goferMountData(fd, conf.FileAccess, "/", true /* vfs2 */, conf.Lisafs) + data := goferMountData(fd, conf.FileAccess, true /* vfs2 */, conf.Lisafs) // We can't check for overlayfs here because sandbox is chroot'ed and gofer // can only send mount options for specs.Mounts (specs.Root is missing @@ -520,7 +520,7 @@ func (c *containerMounter) getMountNameAndOptionsVFS2(conf *config.Config, m *mo // but unlikely to be correct in this context. return "", nil, false, fmt.Errorf("9P mount requires a connection FD") } - data = goferMountData(m.fd, c.getMountAccessType(conf, m.mount), m.mount.Destination, true /* vfs2 */, conf.Lisafs) + data = goferMountData(m.fd, c.getMountAccessType(conf, m.mount), true /* vfs2 */, conf.Lisafs) internalData = gofer.InternalFilesystemOptions{ UniqueID: m.mount.Destination, } diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index c65e0267a..7c22439c7 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -189,8 +189,9 @@ func newSocket(ioFD int) *unet.Socket { func (g *Gofer) serveLisafs(spec *specs.Spec, conf *config.Config, root string) subcommands.ExitStatus { type connectionConfig struct { - sock *unet.Socket - readonly bool + sock *unet.Socket + mountPath string + readonly bool } cfgs := make([]connectionConfig, 0, len(spec.Mounts)+1) server := fsgofer.NewLisafsServer(fsgofer.Config{ @@ -202,8 +203,9 @@ func (g *Gofer) serveLisafs(spec *specs.Spec, conf *config.Config, root string) // Start with root mount, then add any other additional mount as needed. cfgs = append(cfgs, connectionConfig{ - sock: newSocket(g.ioFDs[0]), - readonly: spec.Root.Readonly || conf.Overlay, + sock: newSocket(g.ioFDs[0]), + mountPath: "/", // fsgofer process is always chroot()ed. So serve root. + readonly: spec.Root.Readonly || conf.Overlay, }) log.Infof("Serving %q mapped to %q on FD %d (ro: %t)", "/", root, g.ioFDs[0], cfgs[0].readonly) @@ -221,8 +223,9 @@ func (g *Gofer) serveLisafs(spec *specs.Spec, conf *config.Config, root string) } cfgs = append(cfgs, connectionConfig{ - sock: newSocket(g.ioFDs[mountIdx]), - readonly: isReadonlyMount(m.Options) || conf.Overlay, + sock: newSocket(g.ioFDs[mountIdx]), + mountPath: m.Destination, + readonly: isReadonlyMount(m.Options) || conf.Overlay, }) log.Infof("Serving %q mapped on FD %d (ro: %t)", m.Destination, g.ioFDs[mountIdx], cfgs[mountIdx].readonly) @@ -235,7 +238,7 @@ func (g *Gofer) serveLisafs(spec *specs.Spec, conf *config.Config, root string) cfgs = cfgs[:mountIdx] for _, cfg := range cfgs { - conn, err := server.CreateConnection(cfg.sock, cfg.readonly) + conn, err := server.CreateConnection(cfg.sock, cfg.mountPath, cfg.readonly) if err != nil { Fatalf("starting connection on FD %d for gofer mount failed: %v", cfg.sock.FD(), err) } diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go index e7b1ff508..b2408787b 100644 --- a/runsc/fsgofer/lisafs.go +++ b/runsc/fsgofer/lisafs.go @@ -50,9 +50,10 @@ func NewLisafsServer(config Config) *LisafsServer { } // Mount implements lisafs.ServerImpl.Mount. -func (s *LisafsServer) Mount(c *lisafs.Connection) (*lisafs.ControlFD, linux.Statx, error) { +func (s *LisafsServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lisafs.ControlFD, linux.Statx, error) { + mountPath := mountNode.FilePath() rootHostFD, err := tryOpen(func(flags int) (int, error) { - return unix.Open("/", flags, 0) + return unix.Open(mountPath, flags, 0) }) if err != nil { return nil, linux.Statx{}, err @@ -67,7 +68,8 @@ func (s *LisafsServer) Mount(c *lisafs.Connection) (*lisafs.ControlFD, linux.Sta hostFD: rootHostFD, writableHostFD: -1, } - rootFD.ControlFD.Init(c, s.Root(), linux.FileMode(stat.Mode), rootFD) + mountNode.IncRef() // Ref is transferred to ControlFD. + rootFD.ControlFD.Init(c, mountNode, linux.FileMode(stat.Mode), rootFD) return rootFD.FD(), stat, nil }