soclose() & so->so_upcall() = race?

[Robert Watson]

On Fri, 7 Mar 2008, Alexander Motin wrote:

> As I can see so_upcall() callback is called with SOCKBUF_MTX unlocked. It 
> means that SB_UPCALL flag can be removed during call and socket can be 
> closed and deallocated with soclose() while callback is running. Am I right 
> or I have missed something? How in that situation socket pointer protected 
> from being used after free?

There are known problems with so_upcall and locking, including this one.  The 
other problems include:

- The locking condition on entering the upcall depends on the invocation point
   and is inconsistent.

- The protection of the upcall field and flag are inconsistent.

- Consumers of so_upcall, such as socket accept filters, don't properly
   respect the locking environment they run in.

- Some (all) accept filters produce nastily convoluted stack traces involving
   recursion across really odd combinations of sockets and protocols.  For
   example, you can see soisdisconnected() calling soisconnected().

Some of this is inherent to the design of accept filters and so_upcall and 
follows from using a single function pointer for many different cases.  I have 
an 8.x todo list item to try and figure out how to make so_upcall and accept 
filters rational in the context of fine-grained locking, but I've not yet 
reached that point on my todo list.  I think we can continue to incrementally 
hack so_upcall and accept filters to fix many races, but the reality is that 
we need a more coherent model for dealing with accept filters.  One idea that 
Colin Percival (I think) suggested is that we separate socket upcalls from 
accept filters, and that accept filters consistent of a predicate for 
completion rather than directly invoking socket state transitions.  I've not 
explored the implications, but think it might well be a good idea to avoid the 
weird stack traces.

Robert N M Watson
Computer Laboratory
University of Cambridge