diff --git a/pkg/lisafs/README.md b/pkg/lisafs/README.md
index 75b1684cc..2666a518d 100644
--- a/pkg/lisafs/README.md
+++ b/pkg/lisafs/README.md
@@ -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
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
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.
diff --git a/pkg/lisafs/client.go b/pkg/lisafs/client.go
index a3ae7a57f..8a720ec1d 100644
--- a/pkg/lisafs/client.go
+++ b/pkg/lisafs/client.go
@@ -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() {
diff --git a/pkg/lisafs/communicator.go b/pkg/lisafs/communicator.go
index 0946ec8bb..4d424c0e1 100644
--- a/pkg/lisafs/communicator.go
+++ b/pkg/lisafs/communicator.go
@@ -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.
diff --git a/pkg/lisafs/connection_test.go b/pkg/lisafs/connection_test.go
index b15413702..2743d23b2 100644
--- a/pkg/lisafs/connection_test.go
+++ b/pkg/lisafs/connection_test.go
@@ -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)
diff --git a/pkg/lisafs/handlers.go b/pkg/lisafs/handlers.go
index b24b399b2..cb7167bb7 100644
--- a/pkg/lisafs/handlers.go
+++ b/pkg/lisafs/handlers.go
@@ -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),
}
diff --git a/pkg/lisafs/message.go b/pkg/lisafs/message.go
index 06923788b..94aa3c657 100644
--- a/pkg/lisafs/message.go
+++ b/pkg/lisafs/message.go
@@ -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.
diff --git a/pkg/lisafs/server.go b/pkg/lisafs/server.go
index d419f4228..4e26d3332 100644
--- a/pkg/lisafs/server.go
+++ b/pkg/lisafs/server.go
@@ -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.
diff --git a/pkg/lisafs/testsuite/testsuite.go b/pkg/lisafs/testsuite/testsuite.go
index 2b38a04c6..e9815baa2 100644
--- a/pkg/lisafs/testsuite/testsuite.go
+++ b/pkg/lisafs/testsuite/testsuite.go
@@ -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")
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 650470873..663125850 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -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
diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go
index fb2100f67..33091fd49 100644
--- a/runsc/fsgofer/lisafs.go
+++ b/runsc/fsgofer/lisafs.go
@@ -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.