From 6f5e9e7b74f37bf0fedc35e3762a8e1aff917eb4 Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Tue, 22 Feb 2022 16:38:02 -0800 Subject: [PATCH] Rename p9.ConnectFlags to p9.SocketType. Since the socket type can be passed to Bind as well as Connect, it makes sense to have a more general name. Also consolidate the logic for converting between p9 and linux socket types. Note that despite the name change, nothing in the 9p or LISA wire protocols have changed, so there is no compatibility issues. PiperOrigin-RevId: 430323611 --- pkg/p9/BUILD | 1 + pkg/p9/buffer.go | 10 +++--- pkg/p9/client_file.go | 4 +-- pkg/p9/file.go | 4 +-- pkg/p9/handlers.go | 2 +- pkg/p9/messages.go | 10 +++--- pkg/p9/p9.go | 51 ++++++++++++++++++++++------- pkg/p9/p9test/client_test.go | 12 +++---- pkg/sentry/fs/gofer/context_file.go | 4 +-- pkg/sentry/fs/gofer/socket.go | 15 +-------- pkg/sentry/fsimpl/gofer/p9file.go | 4 +-- pkg/sentry/fsimpl/gofer/socket.go | 14 +------- runsc/fsgofer/fsgofer.go | 15 +++------ 13 files changed, 72 insertions(+), 74 deletions(-) diff --git a/pkg/p9/BUILD b/pkg/p9/BUILD index 2a307df38..d1b77ff46 100644 --- a/pkg/p9/BUILD +++ b/pkg/p9/BUILD @@ -22,6 +22,7 @@ go_library( "version.go", ], deps = [ + "//pkg/abi/linux", "//pkg/errors", "//pkg/errors/linuxerr", "//pkg/fd", diff --git a/pkg/p9/buffer.go b/pkg/p9/buffer.go index 6a4951821..71177cfc3 100644 --- a/pkg/p9/buffer.go +++ b/pkg/p9/buffer.go @@ -158,9 +158,9 @@ func (b *buffer) ReadOpenFlags() OpenFlags { return OpenFlags(b.Read32()) } -// ReadConnectFlags reads a ConnectFlags. -func (b *buffer) ReadConnectFlags() ConnectFlags { - return ConnectFlags(b.Read32()) +// ReadSocketType reads a SocketType. +func (b *buffer) ReadSocketType() SocketType { + return SocketType(b.Read32()) } // ReadMsgType writes a MsgType. @@ -244,8 +244,8 @@ func (b *buffer) WriteOpenFlags(flags OpenFlags) { b.Write32(uint32(flags)) } -// WriteConnectFlags writes a ConnectFlags. -func (b *buffer) WriteConnectFlags(flags ConnectFlags) { +// WriteSocketType writes a SocketType. +func (b *buffer) WriteSocketType(flags SocketType) { b.Write32(uint32(flags)) } diff --git a/pkg/p9/client_file.go b/pkg/p9/client_file.go index 3382af540..fed893934 100644 --- a/pkg/p9/client_file.go +++ b/pkg/p9/client_file.go @@ -424,7 +424,7 @@ func (c *clientFile) Bind(sockType uint32, sockName string, uid UID, gid GID) (F } // Connect implements File.Connect. -func (c *clientFile) Connect(flags ConnectFlags) (*fd.FD, error) { +func (c *clientFile) Connect(socketType SocketType) (*fd.FD, error) { if atomic.LoadUint32(&c.closed) != 0 { return nil, unix.EBADF } @@ -434,7 +434,7 @@ func (c *clientFile) Connect(flags ConnectFlags) (*fd.FD, error) { } rlconnect := Rlconnect{} - if err := c.client.sendRecv(&Tlconnect{FID: c.fid, Flags: flags}, &rlconnect); err != nil { + if err := c.client.sendRecv(&Tlconnect{FID: c.fid, SocketType: socketType}, &rlconnect); err != nil { return nil, err } diff --git a/pkg/p9/file.go b/pkg/p9/file.go index 6ebcd0cbe..b209f4ce7 100644 --- a/pkg/p9/file.go +++ b/pkg/p9/file.go @@ -304,7 +304,7 @@ type File interface { // // Bind is an extension to 9P2000.L, see version.go. // - // On the server, UnlinkAt has a write concurrency guarantee. + // On the server, Bind has a write concurrency guarantee. Bind(sockType uint32, sockName string, uid UID, gid GID) (File, QID, AttrMask, Attr, error) // Connect establishes a new host-socket backed connection with a @@ -319,7 +319,7 @@ type File interface { // Flags indicates the requested type of socket. // // On the server, Connect has a read concurrency guarantee. - Connect(flags ConnectFlags) (*fd.FD, error) + Connect(socketType SocketType) (*fd.FD, error) // Renamed is called when this node is renamed. // diff --git a/pkg/p9/handlers.go b/pkg/p9/handlers.go index 4d331a950..7d4188ad9 100644 --- a/pkg/p9/handlers.go +++ b/pkg/p9/handlers.go @@ -1469,7 +1469,7 @@ func (t *Tlconnect) handle(cs *connState) message { } // Do the connect. - osFile, err = ref.file.Connect(t.Flags) + osFile, err = ref.file.Connect(t.SocketType) return err }); err != nil { return newErr(err) diff --git a/pkg/p9/messages.go b/pkg/p9/messages.go index e363b47f1..2e7b714b3 100644 --- a/pkg/p9/messages.go +++ b/pkg/p9/messages.go @@ -2532,20 +2532,20 @@ type Tlconnect struct { // FID is the FID to be connected. FID FID - // Flags are the connect flags. - Flags ConnectFlags + // SocketType is the socket type to be connected to. + SocketType SocketType } // decode implements encoder.decode. func (t *Tlconnect) decode(b *buffer) { t.FID = b.ReadFID() - t.Flags = b.ReadConnectFlags() + t.SocketType = b.ReadSocketType() } // encode implements encoder.encode. func (t *Tlconnect) encode(b *buffer) { b.WriteFID(t.FID) - b.WriteConnectFlags(t.Flags) + b.WriteSocketType(t.SocketType) } // Type implements message.Type. @@ -2555,7 +2555,7 @@ func (*Tlconnect) Type() MsgType { // String implements fmt.Stringer. func (t *Tlconnect) String() string { - return fmt.Sprintf("Tlconnect{FID: %d, Flags: %v}", t.FID, t.Flags) + return fmt.Sprintf("Tlconnect{FID: %d, SocketType: %v}", t.FID, t.SocketType) } // Rlconnect is a connect response. diff --git a/pkg/p9/p9.go b/pkg/p9/p9.go index 28d44a120..4c6c4c357 100644 --- a/pkg/p9/p9.go +++ b/pkg/p9/p9.go @@ -24,6 +24,7 @@ import ( "syscall" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/abi/linux" ) // OpenFlags is the mode passed to Open and Create operations. @@ -49,26 +50,54 @@ const ( OpenTruncate OpenFlags = 01000 ) -// ConnectFlags is the mode passed to Connect operations. +// SocketType is the socket type passed in Connect and Bind operations. // // These correspond to bits sent over the wire. -type ConnectFlags uint32 +type SocketType uint32 const ( - // StreamSocket is a Tlconnect flag indicating SOCK_STREAM mode. - StreamSocket ConnectFlags = 0 + // StreamSocket indicates SOCK_STREAM mode. + StreamSocket SocketType = 0 - // DgramSocket is a Tlconnect flag indicating SOCK_DGRAM mode. - DgramSocket ConnectFlags = 1 + // DgramSocket indicates SOCK_DGRAM mode. + DgramSocket SocketType = 1 - // SeqpacketSocket is a Tlconnect flag indicating SOCK_SEQPACKET mode. - SeqpacketSocket ConnectFlags = 2 + // SeqpacketSocket indicates SOCK_SEQPACKET mode. + SeqpacketSocket SocketType = 2 - // AnonymousSocket is a Tlconnect flag indicating that the mode does not - // matter and that the requester will accept any socket type. - AnonymousSocket ConnectFlags = 3 + // AnonymousSocket is only valid for Connect calls, and indicates that + // the caller will accept any socket type. + AnonymousSocket SocketType = 3 ) +// ToLinux maps the SocketType to a Linux socket type. +func (st SocketType) ToLinux() (linux.SockType, bool) { + switch st { + case StreamSocket: + return linux.SOCK_STREAM, true + case DgramSocket: + return linux.SOCK_DGRAM, true + case SeqpacketSocket: + return linux.SOCK_SEQPACKET, true + default: + return 0, false + } +} + +// SocketTypeFromLinux maps a Linux socket type to a SocketType. +func SocketTypeFromLinux(st linux.SockType) (SocketType, bool) { + switch st { + case linux.SOCK_STREAM: + return StreamSocket, true + case linux.SOCK_DGRAM: + return DgramSocket, true + case linux.SOCK_SEQPACKET: + return SeqpacketSocket, true + default: + return 0, false + } +} + // OSFlags converts a p9.OpenFlags to an int compatible with open(2). func (o OpenFlags) OSFlags() int { // "flags contains Linux open(2) flags bits" - 9P2000.L diff --git a/pkg/p9/p9test/client_test.go b/pkg/p9/p9test/client_test.go index e6be333ec..e7ed08c06 100644 --- a/pkg/p9/p9test/client_test.go +++ b/pkg/p9/p9test/client_test.go @@ -379,7 +379,7 @@ func checkDeleted(h *Harness, file p9.File) { if _, err := file.Readdir(0, 1); err != unix.EINVAL { h.t.Errorf("readdir while deleted, got %v, want EINVAL", err) } - if _, err := file.Connect(p9.ConnectFlags(0)); err != unix.EINVAL { + if _, err := file.Connect(p9.SocketType(0)); err != unix.EINVAL { h.t.Errorf("connect while deleted, got %v, want EINVAL", err) } @@ -998,7 +998,7 @@ func TestConnect(t *testing.T) { // Catch all the non-socket cases. if !backend.Attr.Mode.IsSocket() { // This has been set up to fail if Connect is called. - if _, err := f.Connect(p9.ConnectFlags(0)); err != unix.EINVAL { + if _, err := f.Connect(p9.SocketType(0)); err != unix.EINVAL { t.Errorf("connect got %v, wanted EINVAL", err) } return @@ -1006,8 +1006,8 @@ func TestConnect(t *testing.T) { // Ensure the fd exchange works. fdTest(t, func(send *fd.FD) *fd.FD { - backend.EXPECT().Connect(p9.ConnectFlags(0)).Return(send, nil) - recv, err := backend.Connect(p9.ConnectFlags(0)) + backend.EXPECT().Connect(p9.SocketType(0)).Return(send, nil) + recv, err := backend.Connect(p9.SocketType(0)) if err != nil { t.Fatalf("connect got %v, wanted nil", err) } @@ -1334,7 +1334,7 @@ func TestClose(t *testing.T) { if _, _, _, _, err := f.WalkGetAttr(nil); err != unix.EBADF { t.Errorf("walkgetattr got %v, wanted EBADF", err) } - if _, err := f.Connect(p9.ConnectFlags(0)); err != unix.EBADF { + if _, err := f.Connect(p9.SocketType(0)); err != unix.EBADF { t.Errorf("connect got %v, wanted EBADF", err) } }) @@ -1953,7 +1953,7 @@ func TestConcurrency(t *testing.T) { name: "connect", match: func(mode p9.FileMode) bool { return mode.IsSocket() }, op: func(h *Harness, backend *Mock, f p9.File, callback func()) { - backend.EXPECT().Connect(gomock.Any()).Do(func(p9.ConnectFlags) { + backend.EXPECT().Connect(gomock.Any()).Do(func(p9.SocketType) { callback() }) f.Connect(0) diff --git a/pkg/sentry/fs/gofer/context_file.go b/pkg/sentry/fs/gofer/context_file.go index 125907d70..096351cf6 100644 --- a/pkg/sentry/fs/gofer/context_file.go +++ b/pkg/sentry/fs/gofer/context_file.go @@ -210,9 +210,9 @@ func (c *contextFile) walkGetAttr(ctx context.Context, names []string) ([]p9.QID return q, contextFile{file: f}, m, a, nil } -func (c *contextFile) connect(ctx context.Context, flags p9.ConnectFlags) (*fd.FD, error) { +func (c *contextFile) connect(ctx context.Context, socketType p9.SocketType) (*fd.FD, error) { ctx.UninterruptibleSleepStart(false) - f, err := c.file.Connect(flags) + f, err := c.file.Connect(socketType) ctx.UninterruptibleSleepFinish(false) return f, err } diff --git a/pkg/sentry/fs/gofer/socket.go b/pkg/sentry/fs/gofer/socket.go index 396de7f9d..d47507f3f 100644 --- a/pkg/sentry/fs/gofer/socket.go +++ b/pkg/sentry/fs/gofer/socket.go @@ -15,7 +15,6 @@ package gofer import ( - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/p9" @@ -66,21 +65,9 @@ type endpoint struct { path string } -func sockTypeToP9(t linux.SockType) (p9.ConnectFlags, bool) { - switch t { - case linux.SOCK_STREAM: - return p9.StreamSocket, true - case linux.SOCK_SEQPACKET: - return p9.SeqpacketSocket, true - case linux.SOCK_DGRAM: - return p9.DgramSocket, true - } - return 0, false -} - // BidirectionalConnect implements BoundEndpoint.BidirectionalConnect. func (e *endpoint) BidirectionalConnect(ctx context.Context, ce transport.ConnectingEndpoint, returnConnect func(transport.Receiver, transport.ConnectedEndpoint)) *syserr.Error { - cf, ok := sockTypeToP9(ce.Type()) + cf, ok := p9.SocketTypeFromLinux(ce.Type()) if !ok { return syserr.ErrConnectionRefused } diff --git a/pkg/sentry/fsimpl/gofer/p9file.go b/pkg/sentry/fsimpl/gofer/p9file.go index 0d97b60fd..e26c4aba5 100644 --- a/pkg/sentry/fsimpl/gofer/p9file.go +++ b/pkg/sentry/fsimpl/gofer/p9file.go @@ -232,9 +232,9 @@ func (f p9file) flush(ctx context.Context) error { return err } -func (f p9file) connect(ctx context.Context, flags p9.ConnectFlags) (*fd.FD, error) { +func (f p9file) connect(ctx context.Context, socketType p9.SocketType) (*fd.FD, error) { ctx.UninterruptibleSleepStart(false) - fdobj, err := f.file.Connect(flags) + fdobj, err := f.file.Connect(socketType) ctx.UninterruptibleSleepFinish(false) return fdobj, err } diff --git a/pkg/sentry/fsimpl/gofer/socket.go b/pkg/sentry/fsimpl/gofer/socket.go index bf36c2323..407f86553 100644 --- a/pkg/sentry/fsimpl/gofer/socket.go +++ b/pkg/sentry/fsimpl/gofer/socket.go @@ -44,18 +44,6 @@ type endpoint struct { path string } -func sockTypeToP9(t linux.SockType) (p9.ConnectFlags, bool) { - switch t { - case linux.SOCK_STREAM: - return p9.StreamSocket, true - case linux.SOCK_SEQPACKET: - return p9.SeqpacketSocket, true - case linux.SOCK_DGRAM: - return p9.DgramSocket, true - } - return 0, false -} - // BidirectionalConnect implements BoundEndpoint.BidirectionalConnect. func (e *endpoint) BidirectionalConnect(ctx context.Context, ce transport.ConnectingEndpoint, returnConnect func(transport.Receiver, transport.ConnectedEndpoint)) *syserr.Error { // No lock ordering required as only the ConnectingEndpoint has a mutex. @@ -121,7 +109,7 @@ func (e *endpoint) newConnectedEndpoint(ctx context.Context, sockType linux.Sock return c, nil } - flags, ok := sockTypeToP9(sockType) + flags, ok := p9.SocketTypeFromLinux(sockType) if !ok { return nil, syserr.ErrConnectionRefused } diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index a317e0c7a..1b0b1647c 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -1209,7 +1209,7 @@ func (l *localFile) Bind(sockType uint32, sockName string, uid p9.UID, gid p9.GI } // Connect implements p9.File. -func (l *localFile) Connect(flags p9.ConnectFlags) (*fd.FD, error) { +func (l *localFile) Connect(socketType p9.SocketType) (*fd.FD, error) { if !l.attachPoint.conf.HostUDS { return nil, unix.ECONNREFUSED } @@ -1222,19 +1222,12 @@ func (l *localFile) Connect(flags p9.ConnectFlags) (*fd.FD, error) { return nil, unix.ECONNREFUSED } - var stype int - switch flags { - case p9.StreamSocket: - stype = unix.SOCK_STREAM - case p9.DgramSocket: - stype = unix.SOCK_DGRAM - case p9.SeqpacketSocket: - stype = unix.SOCK_SEQPACKET - default: + stype, ok := socketType.ToLinux() + if !ok { return nil, unix.ENXIO } - f, err := unix.Socket(unix.AF_UNIX, stype, 0) + f, err := unix.Socket(unix.AF_UNIX, int(stype), 0) if err != nil { return nil, err }