Re: How to prevent a mutex being _enter()ed from being _destroy()ed?

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

> Date: Fri, 10 Aug 2018 08:03:55 -0400
> From: Greg Troxel <gdt%lexort.com@localhost>
> 
> Edgar FuÃ? <ef%math.uni-bonn.de@localhost> writes:
> 
> > Is there a general scheme/rule/proposal how to prevent a mutex that someone 
> > is trying to mutex_enter() from being teared down by someone else calling 
> > mutex_destroy() on it during that?
> 
> Not really.  Basically it's a bug to destroy a mutex that could possibly
> be in use.  So there has to be something else protecting getting at the
> mutex that is being destroyed.

Correct.

> > Specifically, I'm trying to understand what should prevent a thread from 
> > destroying a socket's lock (by sofree()/soput()) while unp_gc() is trying 
> > to acquire that lock.

The unp_gc thread assumes that holding a reference to the associated
struct file, by incrementing f_count, prevents the socket from being
freed.  Certainly that prevents closef from calling fo_close, which
for a socket is soo_close, which calls soclose, which calls sofree,
which calls soput.

There are other calls to soclose:

- kern/subr_tftproot.c, tftproot_getfile
  => only for in-kernel use, not assocated with files
- kern/uipc_syscalls.c, makesocket
  => but only before the socket has been associated with a file
- kern/uipc_syscalls.c, sys_socketpair
  => looks wrong to me but unlikely to trigger
  => maybe fd_abort should wait for unp_gc
  => or maybe this should call closef, not soclose
- kern/uipc_syscalls.c, pipe1 (if PIPE_SOCKETPAIR is enabled)
  => same as sys_socketpair

The remaining calls to soclose are internal to various file systems or
rump and I don't think the sockets they work on have been assocated
with anyone's struct file:

- miscfs/fifofs/fifo_vnops.c
- netsmb/smb_trantcp.c
- nfs/krpc_subr.c
- nfs/nfs_boot.c
- nfs/nfs_bootdhcp.c
- nfs/nfs_socket.c
- rump/net/lib/libnetinet/netinet_component.c
- rump/net/lib/libsockin/sockin.c

There are also other calls to sofree:

- kern/uipc_socket.c, socreate error branch
  => not relevant, hasn't been associated with a file yet
- kern/uipc_socket.c, soabort
  => only for sockets that haven't been associated with files yet

The remaining calls to sofree are in network protocols, mostly in the
detach routines.  When called via ->pr_detach, the socket in question
is not associated with a file.  But there are other callers and I've
run out of steam to audit all of the other callers of these functions
to confirm that they apply only to sockets not associated with files,
or to find where they don't.

- kern/uipc_usrreq.c, unp_drop
- net/raw_cb.c, raw_detach
  => called by raw->pr_detach, route->pr_detach, key->pr_detach
  => called by raw_disconnect only if SS_NOFDREF is set, meaning
     there is no associated file
- netatalk/ddp_usrreq.c, ddp_detach
  => called by atalk->pr_detach and atalk->pr_abort, no associated file
- netcan/can_pcb.c, can_pcbdetach
  => called by can->pr_detach
  => called by can_pcbdisconnect only if SS_NOFDREF is set
- netinet/in_pcb.c, in_pcbdetach
  => called by dccp_close
     ---> oops, we have a callout_stop/halt bug here
     ---> out of steam to study further
  => called by in_pcbdisconnect only if SS_NOFDREF is set
  => called by rip->pr_detach
  => called by tcp_close
     ---> out of steam to study further
  => called by tcp->pr_attach
  => called by udp->pr_detach
- netinet/sctp_pcb.c, sctp_incpb_free
  => out of steam to study further
- netinet6/in6_pcb.c, in6_pcbdetach
  => out of steam to study further, looks the same as in_pcbdetach
- netnatm/natm.c, natm_detach
  => called by natm->pr_detach

There's also one other call to soput:

- sonewconn
  => only for sockets that haven't been assocated with files yet

More eyeballs on these code paths might find a problem, if indeed the
problem is a use-after-free arising from soclose/sofree/soput that
happened too early.