From 68fd6aba43eeb776b45d74c52525d2e7da5532d8 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Mon, 26 Sep 2022 16:46:43 -0700 Subject: [PATCH] Update BindAt RPC to include other creation options. BindAt should also be setting the socket file mode and owners. All file creation RPCs do this. This is required for correct behavior. PiperOrigin-RevId: 477023055 --- pkg/lisafs/client_file.go | 14 ++++++++------ pkg/lisafs/fd.go | 2 +- pkg/lisafs/handlers.go | 2 +- pkg/lisafs/message.go | 10 +++++----- pkg/sentry/fsimpl/gofer/gofer.go | 2 +- runsc/fsgofer/lisafs.go | 12 +++++++++++- 6 files changed, 27 insertions(+), 15 deletions(-) diff --git a/pkg/lisafs/client_file.go b/pkg/lisafs/client_file.go index 73f8a5684..1de9c4cb3 100644 --- a/pkg/lisafs/client_file.go +++ b/pkg/lisafs/client_file.go @@ -421,16 +421,18 @@ func (f *ClientFD) Flush(ctx context.Context) error { } // BindAt makes the BindAt RPC. -func (f *ClientFD) BindAt(ctx context.Context, sockType linux.SockType, name string) (Inode, *ClientBoundSocketFD, error) { - req := BindAtReq{ - DirFD: f.fd, - SockType: primitive.Uint32(sockType), - Name: SizedString(name), - } +func (f *ClientFD) BindAt(ctx context.Context, sockType linux.SockType, name string, mode linux.FileMode, uid UID, gid GID) (Inode, *ClientBoundSocketFD, error) { var ( + req BindAtReq resp BindAtResp hostSocketFD [1]int ) + req.DirFD = f.fd + req.SockType = primitive.Uint32(sockType) + req.Name = SizedString(name) + req.Mode = mode + req.UID = uid + req.GID = gid ctx.UninterruptibleSleepStart(false) err := f.client.SndRcvMessage(BindAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, hostSocketFD[:], req.String, resp.String) ctx.UninterruptibleSleepFinish(false) diff --git a/pkg/lisafs/fd.go b/pkg/lisafs/fd.go index 32b811085..b1ccb8d1c 100644 --- a/pkg/lisafs/fd.go +++ b/pkg/lisafs/fd.go @@ -492,7 +492,7 @@ type ControlFDImpl interface { // connections). // // On the server, BindAt has a write concurrency guarantee. - BindAt(name string, sockType uint32) (*ControlFD, linux.Statx, *BoundSocketFD, int, error) + BindAt(name string, sockType uint32, mode linux.FileMode, uid UID, gid GID) (*ControlFD, linux.Statx, *BoundSocketFD, int, error) // UnlinkAt the file identified by name in this directory. // diff --git a/pkg/lisafs/handlers.go b/pkg/lisafs/handlers.go index adf5773b3..3c61a8671 100644 --- a/pkg/lisafs/handlers.go +++ b/pkg/lisafs/handlers.go @@ -1103,7 +1103,7 @@ func BindAtHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32, if dir.node.isDeleted() { return unix.EINVAL } - childFD, childStat, boundSocketFD, hostSocketFD, err = dir.impl.BindAt(name, uint32(req.SockType)) + childFD, childStat, boundSocketFD, hostSocketFD, err = dir.impl.BindAt(name, uint32(req.SockType), req.Mode, req.UID, req.GID) return err }); err != nil { return 0, err diff --git a/pkg/lisafs/message.go b/pkg/lisafs/message.go index e539d55cd..41e2f804a 100644 --- a/pkg/lisafs/message.go +++ b/pkg/lisafs/message.go @@ -1290,19 +1290,19 @@ func (*ConnectResp) String() string { // BindAtReq is used to make BindAt requests. type BindAtReq struct { - DirFD FDID + createCommon SockType primitive.Uint32 Name SizedString } // SizeBytes implements marshal.Marshallable.SizeBytes. func (b *BindAtReq) SizeBytes() int { - return b.DirFD.SizeBytes() + b.SockType.SizeBytes() + b.Name.SizeBytes() + return b.createCommon.SizeBytes() + b.SockType.SizeBytes() + b.Name.SizeBytes() } // MarshalBytes implements marshal.Marshallable.MarshalBytes. func (b *BindAtReq) MarshalBytes(dst []byte) []byte { - dst = b.DirFD.MarshalUnsafe(dst) + dst = b.createCommon.MarshalUnsafe(dst) dst = b.SockType.MarshalUnsafe(dst) return b.Name.MarshalBytes(dst) } @@ -1313,7 +1313,7 @@ func (b *BindAtReq) CheckedUnmarshal(src []byte) ([]byte, bool) { if b.SizeBytes() > len(src) { return src, false } - srcRemain := b.DirFD.UnmarshalUnsafe(src) + srcRemain := b.createCommon.UnmarshalUnsafe(src) srcRemain = b.SockType.UnmarshalUnsafe(srcRemain) if srcRemain, ok := b.Name.CheckedUnmarshal(srcRemain); ok { return srcRemain, ok @@ -1323,7 +1323,7 @@ func (b *BindAtReq) CheckedUnmarshal(src []byte) ([]byte, bool) { // String implements fmt.Stringer.String. func (b *BindAtReq) String() string { - return fmt.Sprintf("BindAtReq{DirFD: %d, SockType: %d, Name: %q}", b.DirFD, b.SockType, b.Name) + return fmt.Sprintf("BindAtReq{DirFD: %d, Mode: %s, UID: %d, GID: %d, SockType: %d, Name: %q}", b.DirFD, b.Mode, b.UID, b.GID, b.SockType, b.Name) } // BindAtResp is used to communicate BindAt response. diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 93f5b0aed..db0d34cdd 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -1638,7 +1638,7 @@ func (d *dentry) mknodLisaLocked(ctx context.Context, name string, creds *auth.C // This mknod(2) is coming from unix bind(2), as opts.Endpoint is set. sockType := opts.Endpoint.(transport.Endpoint).Type() - childInode, boundSocketFD, err := d.controlFDLisa.BindAt(ctx, sockType, name) + childInode, boundSocketFD, err := d.controlFDLisa.BindAt(ctx, sockType, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) if err != nil { return err } diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go index 9361fae09..77a38a14b 100644 --- a/runsc/fsgofer/lisafs.go +++ b/runsc/fsgofer/lisafs.go @@ -682,7 +682,7 @@ func (fd *controlFDLisa) Connect(sockType uint32) (int, error) { } // BindAt implements lisafs.ControlFDImpl.BindAt. -func (fd *controlFDLisa) BindAt(name string, sockType uint32) (*lisafs.ControlFD, linux.Statx, *lisafs.BoundSocketFD, int, error) { +func (fd *controlFDLisa) BindAt(name string, sockType uint32, mode linux.FileMode, uid lisafs.UID, gid lisafs.GID) (*lisafs.ControlFD, linux.Statx, *lisafs.BoundSocketFD, int, error) { if !fd.Conn().ServerImpl().(*LisafsServer).config.HostUDS { return nil, linux.Statx{}, nil, -1, unix.EPERM } @@ -716,6 +716,12 @@ func (fd *controlFDLisa) BindAt(name string, sockType uint32) (*lisafs.ControlFD }) defer cu.Clean() + // fchmod(2) has to happen *before* the bind(2). sockFD's file mode will + // be used in creating the filesystem-object in bind(2). + if err := unix.Fchmod(sockFD, uint32(mode&^linux.FileTypeMask)); err != nil { + return nil, linux.Statx{}, nil, -1, err + } + if err := unix.Bind(sockFD, &unix.SockaddrUnix{Name: socketPath}); err != nil { return nil, linux.Statx{}, nil, -1, err } @@ -733,6 +739,10 @@ func (fd *controlFDLisa) BindAt(name string, sockType uint32) (*lisafs.ControlFD _ = unix.Close(sockFileFD) }) + if err := unix.Fchownat(sockFileFD, "", int(uid), int(gid), unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW); err != nil { + return nil, linux.Statx{}, nil, -1, err + } + // Stat the socket. sockStat, err := fstatTo(sockFileFD) if err != nil {