From 6669003321e402bcb270300aefa1f916e3c2fbb4 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Wed, 15 Mar 2023 12:50:56 -0700 Subject: [PATCH] Make HostBountEndpoint.SetBoundSocketFD take ownership of bound socket FD. Earlier SetBoundSocketFD() was taking ownership of bound socket FD only on success. Having it take ownership unconditionally is cleaner. PiperOrigin-RevId: 516903597 --- pkg/sentry/fsimpl/gofer/directfs_dentry.go | 3 +-- pkg/sentry/fsimpl/gofer/lisafs_dentry.go | 3 +-- pkg/sentry/socket/unix/transport/connectioned.go | 3 ++- pkg/sentry/socket/unix/transport/unix.go | 6 +++--- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index 74e910a27..934a183db 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/directfs_dentry.go @@ -476,8 +476,7 @@ func (d *directfsDentry) bindAt(ctx context.Context, name string, creds *auth.Cr } bsFD := &boundSocketFD{sockFD} hbep := opts.Endpoint.(transport.HostBoundEndpoint) - if err := hbep.SetBoundSocketFD(bsFD); err != nil { - bsFD.Close(ctx) + if err := hbep.SetBoundSocketFD(ctx, bsFD); err != nil { return nil, err } diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index e2a11f938..045d7b630 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -406,8 +406,7 @@ func (d *lisafsDentry) mknod(ctx context.Context, name string, creds *auth.Crede return nil, err } hbep := opts.Endpoint.(transport.HostBoundEndpoint) - if err := hbep.SetBoundSocketFD(boundSocketFD); err != nil { - boundSocketFD.Close(ctx) + if err := hbep.SetBoundSocketFD(ctx, boundSocketFD); err != nil { if err := d.controlFD.UnlinkAt(ctx, name, 0 /* flags */); err != nil { log.Warningf("failed to clean up socket which was created by BindAt RPC: %v", err) } diff --git a/pkg/sentry/socket/unix/transport/connectioned.go b/pkg/sentry/socket/unix/transport/connectioned.go index e2698de2d..87f9bb460 100644 --- a/pkg/sentry/socket/unix/transport/connectioned.go +++ b/pkg/sentry/socket/unix/transport/connectioned.go @@ -595,10 +595,11 @@ func (e *connectionedEndpoint) OnSetSendBufferSize(v int64) (newSz int64) { func (e *connectionedEndpoint) WakeupWriters() {} // SetBoundSocketFD implement HostBountEndpoint.SetBoundSocketFD. -func (e *connectionedEndpoint) SetBoundSocketFD(bsFD BoundSocketFD) error { +func (e *connectionedEndpoint) SetBoundSocketFD(ctx context.Context, bsFD BoundSocketFD) error { e.Lock() defer e.Unlock() if e.path != "" || e.boundSocketFD != nil { + bsFD.Close(ctx) return syserr.ErrAlreadyBound.ToError() } e.boundSocketFD = bsFD diff --git a/pkg/sentry/socket/unix/transport/unix.go b/pkg/sentry/socket/unix/transport/unix.go index 2e3135069..ce1e96616 100644 --- a/pkg/sentry/socket/unix/transport/unix.go +++ b/pkg/sentry/socket/unix/transport/unix.go @@ -269,9 +269,9 @@ type BoundEndpoint interface { type HostBoundEndpoint interface { // SetBoundSocketFD will be called on supporting endpoints after // binding a socket on the host filesystem. Implementations should - // delegate Listen and Accept calls to the BoundSocketFD. On success, - // the ownership of bsFD is transferred to the endpoint. - SetBoundSocketFD(bsFD BoundSocketFD) error + // delegate Listen and Accept calls to the BoundSocketFD. The ownership + // of bsFD is transferred to the endpoint. + SetBoundSocketFD(ctx context.Context, bsFD BoundSocketFD) error // ResetBoundSocketFD cleans up the BoundSocketFD set by the last successful // SetBoundSocketFD call.