Optionally donate a host FD to the mount point in lisafs.

This is not used as of now. This is required for directfs later. The impacted RPCs are Mount and Open. Mount has been modified to now donate an optional FD to the mount point. As the mount point can be a socket or a symlink, which needs to be opened with O_PATH, we can no longer expect unix.SetNonblock() to work always while donating FD in lisafs. So allow it to fail. Just log a warning in such cases. Also split out the logic to start channels from lisafs.NewClient(). Not all clients require channels. PiperOrigin-RevId: 504631423
2026-05-22 17:12:49 -07:00 · 2023-01-25 12:30:20 -08:00
parent 492f4c95c9
commit dbfc591d94
10 changed files with 106 additions and 80 deletions
@@ -278,7 +278,7 @@ paths.
 MID | Message      | Request         | Response                                                           | Description
 --- | ------------ | --------------- | ------------------------------------------------------------------ | -----------
 0   | Error        | N/A             | ErrorResp                                                          | Returned from server to indicate error while handling RPC. If Error is returned, the failed RPC should have no side effects. Any intermediate changes made should be rolled back. This is a response-only message. ErrorResp.errno should be interpreted as a Linux error code.
-1   | Mount        |                 | MountResp                                                          | Mount establishes a connection. MountResp.root is a Control FD for the mountpoint, which becomes the root for this connection. The location of the connection’s mountpoint on the server is predetermined as per sandbox configuration. Clients can not request to mount the connection at a certain path, unlike mount(2). MountResp.maxMessageSize dictates the maximum message size the server can tolerate across all communicators. This limit does not include the communicator’s header size. MountResp.supportedMs contains all the MIDs that the server supports. Clients can use this information for checking feature support. The server must provide a read concurrency guarantee on the root node during this operation.
+1   | Mount        |                 | MountResp<br><br>Optionally donates: \[mountPointHostFD\]          | Mount establishes a connection. MountResp.root is a Control FD for the mountpoint, which becomes the root for this connection. The location of the connection’s mountpoint on the server is predetermined as per sandbox configuration. Clients can not request to mount the connection at a certain path, unlike mount(2). MountResp.maxMessageSize dictates the maximum message size the server can tolerate across all communicators. This limit does not include the communicator’s header size. MountResp.supportedMs contains all the MIDs that the server supports. Clients can use this information for checking feature support. The server must provide a read concurrency guarantee on the root node during this operation.
 2   | Channel      |                 | ChannelResp<br><br>Donates: \[dataFD, fdSock\]                     | Channel sets up a new communicator based on a shared memory region between the client and server. dataFD is the host FD for the shared memory file. fdSock is a host socket FD that the server will use to donate FDs over this channel. ChannelResp’s dataOffset and dataLength describe the shared memory file region owned by this channel. No concurrency guarantees are needed. ENOMEM is returned to indicate that the server hit the max channels limit.
 3   | FStat        | StatReq         | [struct statx](https://man7.org/linux/man-pages/man2/statx.2.html) | Fstat is analogous to fstat(2). It returns struct statx for the file represented by StatReq.fd. FStat may be called on a Control FD or Open FD. The server must provide a read concurrency guarantee on the file node during this operation.
 4   | SetStat      | SetStatReq      | SetStatResp                                                        | SetStat does not correspond to any particular syscall. It serves the purpose of fchmod(2), fchown(2), ftruncate(2) and futimesat(2) in one message. This enables client-side optimizations where the client is able to change multiple attributes in 1 RPC. It must be called on Control FDs only. One instance where this is helpful is in overlayfs implementation which requires changing multiple attributes at the same time. The failure of setting one attribute does not terminate the entire operation. SetStatResp.failureMask should be interpreted as stx\_mask and indicates all attributes that failed to be modified. In case failureMask != 0, SetStatResp.failiureErrno indicates any one of the failure errnos. The server must provide a write concurrency guarantee on the file node during this operation.
@@ -19,7 +19,6 @@ import (
 	"math"

 	"golang.org/x/sys/unix"
-	"gvisor.dev/gvisor/pkg/cleanup"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/flipcall"
 	"gvisor.dev/gvisor/pkg/log"
@@ -72,14 +71,11 @@ 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) (*Client, Inode, error) {
-	maxChans := maxChannels()
+func NewClient(sock *unet.Socket) (*Client, Inode, int, error) {
 	c := &Client{
-		sockComm:          newSockComm(sock),
-		channels:          make([]*channel, 0, maxChans),
-		availableChannels: make([]*channel, 0, maxChans),
-		maxMessageSize:    1 << 20, // 1 MB for now.
-		fdsToClose:        make([]FDID, 0, fdsToCloseBatchSize),
+		sockComm:       newSockComm(sock),
+		maxMessageSize: 1 << 20, // 1 MB for now.
+		fdsToClose:     make([]FDID, 0, fdsToCloseBatchSize),
 	}

 	// Start a goroutine to check socket health. This goroutine is also
@@ -87,22 +83,18 @@ func NewClient(sock *unet.Socket) (*Client, Inode, error) {
 	c.watchdogWg.Add(1)
 	go c.watchdog()

-	// Clean everything up if anything fails.
-	cu := cleanup.Make(func() {
-		c.Close()
-	})
-	defer cu.Clean()
-
 	// Mount the server first. Assume Mount is supported so that we can make the
 	// Mount RPC below.
 	c.supported = make([]bool, Mount+1)
 	c.supported[Mount] = true
 	var (
-		mountReq  MountReq
-		mountResp MountResp
+		mountReq    MountReq
+		mountResp   MountResp
+		mountHostFD = [1]int{-1}
 	)
-	if err := c.SndRcvMessage(Mount, uint32(mountReq.SizeBytes()), mountReq.MarshalBytes, mountResp.CheckedUnmarshal, nil, mountReq.String, mountResp.String); err != nil {
-		return nil, Inode{}, err
+	if err := c.SndRcvMessage(Mount, uint32(mountReq.SizeBytes()), mountReq.MarshalBytes, mountResp.CheckedUnmarshal, mountHostFD[:], mountReq.String, mountResp.String); err != nil {
+		c.Close()
+		return nil, Inode{}, -1, err
 	}

 	// Initialize client.
@@ -117,6 +109,16 @@ func NewClient(sock *unet.Socket) (*Client, Inode, error) {
 	for _, suppMID := range mountResp.SupportedMs {
 		c.supported[suppMID] = true
 	}
+	return c, mountResp.Root, mountHostFD[0], nil
+}
+
+// StartChannels starts maxChannels() channel communicators.
+func (c *Client) StartChannels() error {
+	maxChans := maxChannels()
+	c.channelsMu.Lock()
+	c.channels = make([]*channel, 0, maxChans)
+	c.availableChannels = make([]*channel, 0, maxChans)
+	c.channelsMu.Unlock()

 	// Create channels parallely so that channels can be used to create more
 	// channels and costly initialization like flipcall.Endpoint.Connect can
@@ -150,11 +152,9 @@ func NewClient(sock *unet.Socket) (*Client, Inode, error) {
 	c.channelsMu.Unlock()
 	if maxChans > 0 && numChannels == 0 {
 		log.Warningf("all channel RPCs failed")
-		return nil, Inode{}, unix.ENOMEM
+		return unix.ENOMEM
 	}
-
-	cu.Release()
-	return c, mountResp.Root, nil
+	return nil
 }

 func (c *Client) watchdog() {
@@ -18,6 +18,7 @@ import (
 	"fmt"

 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/log"
 )

 // Communicator is a server side utility which represents exactly how the
@@ -41,10 +42,10 @@ type Communicator interface {
 	// response message along with the response payload length.
 	SndRcvMessage(m MID, payloadLen uint32, wantFDs uint8) (MID, uint32, error)

-	// DonateFD makes fd non-blocking and starts tracking it. The next call to
-	// ReleaseFDs will include fd in the order it was added. Communicator takes
-	// ownership of fd. Server side should call this.
-	DonateFD(fd int) error
+	// DonateFD attempts to make fd non-blocking and starts tracking it. The next
+	// call to ReleaseFDs will include fd in the order it was added. Communicator
+	// takes ownership of fd. Server side should call this.
+	DonateFD(fd int)

 	// Track starts tracking fd. The next call to ReleaseFDs will include fd in
 	// the order it was added. Communicator takes ownership of fd. Client side
@@ -63,14 +64,14 @@ type fdTracker struct {
 }

 // DonateFD implements Communicator.DonateFD.
-func (d *fdTracker) DonateFD(fd int) error {
-	// Make sure the FD is non-blocking.
+func (d *fdTracker) DonateFD(fd int) {
+	// Try to make the FD non-blocking.
 	if err := unix.SetNonblock(fd, true); err != nil {
-		unix.Close(fd)
-		return err
+		// This may fail if fd was opened with O_PATH, because fcntl(F_SETFL) fails
+		// with EBADF on O_PATH FDs.
+		log.Warningf("DonateFD: unix.SetNonblock() failed on FD %d: %v", fd, err)
 	}
 	d.TrackFD(fd)
-	return nil
 }

 // TrackFD implements Communicator.TrackFD.
@@ -57,11 +57,11 @@ func (fd *testControlFD) FD() *lisafs.ControlFD {
 func (fd *testControlFD) Close() {}

 // Mount implements lisafs.Mount.
-func (s *testServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lisafs.ControlFD, linux.Statx, error) {
+func (s *testServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lisafs.ControlFD, linux.Statx, int, error) {
 	dummyRoot := &testControlFD{}
 	mountNode.IncRef() // Ref is transferred to ControlFD.
 	dummyRoot.Init(c, mountNode, linux.ModeDirectory, dummyRoot)
-	return dummyRoot.FD(), linux.Statx{Mode: linux.S_IFDIR}, nil
+	return dummyRoot.FD(), linux.Statx{Mode: linux.S_IFDIR}, -1, nil
 }

 // MaxMessageSize implements lisafs.MaxMessageSize.
@@ -95,10 +95,13 @@ func runServerClient(t testing.TB, clientFn func(c *lisafs.Client)) {
 	}
 	ts.StartConnection(conn)

-	c, _, err := lisafs.NewClient(clientSocket)
+	c, _, _, err := lisafs.NewClient(clientSocket)
 	if err != nil {
 		t.Fatalf("client creation failed: %v", err)
 	}
+	if err := c.StartChannels(); err != nil {
+		t.Fatalf("failed to start channels: %v", err)
+	}

 	clientFn(c)

@@ -92,9 +92,10 @@ func ErrorHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32,
 // has been successfully mounted can other channels be created.
 func MountHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32, error) {
 	var (
-		mountPointFD   *ControlFD
-		mountPointStat linux.Statx
-		mountNode      = c.server.root
+		mountPointFD     *ControlFD
+		mountPointHostFD = -1
+		mountPointStat   linux.Statx
+		mountNode        = c.server.root
 	)
 	if err := c.server.withRenameReadLock(func() (err error) {
 		// Maintain extra ref on mountNode to ensure existence during walk.
@@ -139,12 +140,15 @@ func MountHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32,
 		if mountNode.isDeleted() {
 			return unix.ENOENT
 		}
-		mountPointFD, mountPointStat, err = c.ServerImpl().Mount(c, mountNode)
+		mountPointFD, mountPointStat, mountPointHostFD, err = c.ServerImpl().Mount(c, mountNode)
 		return err
 	}); err != nil {
 		return 0, err
 	}

+	if mountPointHostFD >= 0 {
+		comm.DonateFD(mountPointHostFD)
+	}
 	resp := MountResp{
 		Root: Inode{
 			ControlFD: mountPointFD.id,
@@ -185,12 +189,8 @@ func ChannelHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32
 	}

 	// Respond to client with successful channel creation message.
-	if err := comm.DonateFD(clientDataFD); err != nil {
-		return 0, err
-	}
-	if err := comm.DonateFD(fdSock); err != nil {
-		return 0, err
-	}
+	comm.DonateFD(clientDataFD)
+	comm.DonateFD(fdSock)
 	resp := ChannelResp{
 		dataOffset: desc.Offset,
 		dataLength: uint64(desc.Length),
@@ -534,9 +534,7 @@ func OpenAtHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32,
 	}

 	if hostOpenFD >= 0 {
-		if err := comm.DonateFD(hostOpenFD); err != nil {
-			return 0, err
-		}
+		comm.DonateFD(hostOpenFD)
 	}
 	resp := OpenAtResp{OpenFD: openFD.id}
 	respLen := uint32(resp.SizeBytes())
@@ -591,9 +589,7 @@ func OpenCreateAtHandler(c *Connection, comm Communicator, payloadLen uint32) (u
 	}

 	if hostOpenFD >= 0 {
-		if err := comm.DonateFD(hostOpenFD); err != nil {
-			return 0, err
-		}
+		comm.DonateFD(hostOpenFD)
 	}
 	resp := OpenCreateAtResp{
 		NewFD: openFD.id,
@@ -1068,7 +1064,8 @@ func ConnectHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32
 		return 0, err
 	}

-	return 0, comm.DonateFD(sock)
+	comm.DonateFD(sock)
+	return 0, nil
 }

 // BindAtHandler handles the BindAt RPC.
@@ -1109,10 +1106,7 @@ func BindAtHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32,
 		return 0, err
 	}

-	if err := comm.DonateFD(hostSocketFD); err != nil {
-		return 0, err
-	}
-
+	comm.DonateFD(hostSocketFD)
 	resp := BindAtResp{
 		Child: Inode{
 			ControlFD: childFD.id,
@@ -1170,9 +1164,7 @@ func AcceptHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32,
 	}); err != nil {
 		return 0, err
 	}
-	if err := comm.DonateFD(newSock); err != nil {
-		return 0, err
-	}
+	comm.DonateFD(newSock)
 	resp := AcceptResp{
 		PeerAddr: SizedString(peerAddr),
 	}
@@ -331,7 +331,7 @@ type Inode struct {
 	Stat      linux.Statx
 }

-// MountReq is an empty requent to Mount on the connection.
+// MountReq is an empty request to Mount on the connection.
 type MountReq struct{ EmptyMessage }

 // String implements fmt.Stringer.String.
@@ -64,7 +64,7 @@ type ServerOpts struct {
 	AllocateOnDeleted bool
 }

-// Init must be called before first use of server.
+// Init must be called before first use of the server.
 func (s *Server) Init(impl ServerImpl, opts ServerOpts) {
 	s.impl = impl
 	s.opts = opts
@@ -112,10 +112,10 @@ func (s *Server) Destroy() {
 // value as their first field.
 type ServerImpl interface {
 	// Mount is called when a Mount RPC is made. It mounts the connection on
-	// mountNode.
+	// mountNode. Mount may optionally donate a host FD to the mount point.
 	//
 	// Mount has a read concurrency guarantee on mountNode.
-	Mount(c *Connection, mountNode *Node) (*ControlFD, linux.Statx, error)
+	Mount(c *Connection, mountNode *Node) (*ControlFD, linux.Statx, int, error)

 	// SupportedMessages returns a list of messages that the server
 	// implementation supports.
@@ -100,10 +100,13 @@ func RunTest(t *testing.T, tester Tester, testName string, testFn TestFunc, moun
 	}
 	server.StartConnection(conn)

-	c, root, err := lisafs.NewClient(clientSocket)
+	c, root, _, err := lisafs.NewClient(clientSocket)
 	if err != nil {
 		t.Fatalf("client creation failed: %v", err)
 	}
+	if err := c.StartChannels(); err != nil {
+		t.Fatalf("failed to start channels: %v", err)
+	}

 	if !root.ControlFD.Ok() {
 		t.Fatalf("root control FD is not valid")
@@ -540,7 +540,13 @@ func (fs *filesystem) initClient(ctx context.Context) (lisafs.Inode, error) {

 	var rootInode lisafs.Inode
 	ctx.UninterruptibleSleepStart(false)
-	fs.client, rootInode, err = lisafs.NewClient(sock)
+	fs.client, rootInode, _, err = lisafs.NewClient(sock)
+	ctx.UninterruptibleSleepFinish(false)
+	if err != nil {
+		return lisafs.Inode{}, err
+	}
+	ctx.UninterruptibleSleepStart(false)
+	err = fs.client.StartChannels()
 	ctx.UninterruptibleSleepFinish(false)
 	if err != nil {
 		return lisafs.Inode{}, err
@@ -56,6 +56,10 @@ type Config struct {

 	// HostFifo signals whether the gofer can connect to host FIFOs.
 	HostFifo config.HostFifo
+
+	// DonateMountPointFD indicates whether a host FD to the mount point should
+	// be donated to the client on Mount RPC.
+	DonateMountPointFD bool
 }

 var procSelfFD *rwfd.FD
@@ -91,13 +95,13 @@ func NewLisafsServer(config Config) *LisafsServer {
 }

 // Mount implements lisafs.ServerImpl.Mount.
-func (s *LisafsServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lisafs.ControlFD, linux.Statx, error) {
+func (s *LisafsServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lisafs.ControlFD, linux.Statx, int, error) {
 	mountPath := mountNode.FilePath()
 	rootHostFD, err := tryOpen(func(flags int) (int, error) {
 		return unix.Open(mountPath, flags, 0)
 	})
 	if err != nil {
-		return nil, linux.Statx{}, err
+		return nil, linux.Statx{}, -1, err
 	}
 	cu := cleanup.Make(func() {
 		_ = unix.Close(rootHostFD)
@@ -106,22 +110,31 @@ func (s *LisafsServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lis

 	stat, err := fstatTo(rootHostFD)
 	if err != nil {
-		return nil, linux.Statx{}, err
+		return nil, linux.Statx{}, -1, err
 	}

 	if err := checkSupportedFileType(uint32(stat.Mode), &s.config); err != nil {
 		log.Warningf("Mount: checkSupportedFileType() failed for file %q with mode %o: %v", mountPath, stat.Mode, err)
-		return nil, linux.Statx{}, err
+		return nil, linux.Statx{}, -1, err
+	}
+
+	clientHostFD := -1
+	if s.config.DonateMountPointFD {
+		clientHostFD, err = unix.Dup(rootHostFD)
+		if err != nil {
+			return nil, linux.Statx{}, -1, err
+		}
 	}
 	cu.Release()

 	rootFD := &controlFDLisa{
 		hostFD:         rootHostFD,
 		writableHostFD: atomicbitops.FromInt32(-1),
+		isMountPoint:   true,
 	}
 	mountNode.IncRef() // Ref is transferred to ControlFD.
 	rootFD.ControlFD.Init(c, mountNode, linux.FileMode(stat.Mode), rootFD)
-	return rootFD.FD(), stat, nil
+	return rootFD.FD(), stat, clientHostFD, nil
 }

 // MaxMessageSize implements lisafs.ServerImpl.MaxMessageSize.
@@ -175,6 +188,10 @@ type controlFDLisa struct {
 	// the same FD as `hostFD`. It is initialized to -1, and can change in value
 	// exactly once.
 	writableHostFD atomicbitops.Int32
+
+	// isMountpoint indicates whether this FD represents the mount point for its
+	// owning connection. isMountPoint is immutable.
+	isMountPoint bool
 }

 var _ lisafs.ControlFDImpl = (*controlFDLisa)(nil)
@@ -454,31 +471,35 @@ func (fd *controlFDLisa) WalkStat(path lisafs.StringArray, recordStat func(linux
 // Open implements lisafs.ControlFDImpl.Open.
 func (fd *controlFDLisa) Open(flags uint32) (*lisafs.OpenFD, int, error) {
 	flags |= openFlags
-	newHostFD, err := unix.Openat(int(procSelfFD.FD()), strconv.Itoa(fd.hostFD), int(flags)&^unix.O_NOFOLLOW, 0)
+	openHostFD, err := unix.Openat(int(procSelfFD.FD()), strconv.Itoa(fd.hostFD), int(flags)&^unix.O_NOFOLLOW, 0)
 	if err != nil {
 		return nil, -1, err
 	}
-	openFD := fd.newOpenFDLisa(newHostFD, flags)

-	hostOpenFD := -1
-	switch fd.FileType() {
-	case unix.S_IFREG:
+	hostFDToDonate := -1
+	ftype := fd.FileType()
+	switch {
+	case ftype == unix.S_IFREG:
 		// Best effort to donate file to the Sentry (for performance only).
-		hostOpenFD, _ = unix.Dup(openFD.hostFD)
+		hostFDToDonate, _ = unix.Dup(openHostFD)

-	case unix.S_IFIFO, unix.S_IFCHR:
+	case ftype == unix.S_IFIFO,
+		ftype == unix.S_IFCHR,
+		fd.isMountPoint && fd.Conn().ServerImpl().(*LisafsServer).config.DonateMountPointFD:
 		// Character devices and pipes can block indefinitely during reads/writes,
 		// which is not allowed for gofer operations. Ensure that it donates an FD
 		// back to the caller, so it can wait on the FD when reads/writes return
-		// EWOULDBLOCK.
+		// EWOULDBLOCK. For mount points, if DonateMountPointFD option is set, an
+		// FD must be donated.
 		var err error
-		hostOpenFD, err = unix.Dup(openFD.hostFD)
+		hostFDToDonate, err = unix.Dup(openHostFD)
 		if err != nil {
 			return nil, 0, err
 		}
 	}

-	return openFD.FD(), hostOpenFD, nil
+	openFD := fd.newOpenFDLisa(openHostFD, flags)
+	return openFD.FD(), hostFDToDonate, nil
 }

 // OpenCreate implements lisafs.ControlFDImpl.OpenCreate.