TCP socket shutdown race condition

[Ed Maste]

>Well, I guess the spl() fix is probably going to be the quickest here
>then, please send it to me once you've pounded on it, Ed.

So my spl() fix seems to eliminate the problem for me but while I'm looking
at this stuff I want to make sure there aren't any related cases left in.
My current patch is at the end of this message.

One potential problem is a crfree() interrupting code that's incrementing a
ucred reference.  For example, uipc_socket.c:socreate():

	so->so_cred = p->p_ucred;
	crhold(so->so_cred);

However, I don't think a crfree() interrupting between these should cause a
problem.  The refcount would always be at least 2 to begin with, so the
ucred wouldn't be free()d.

Another possible problem is a crfree() interrupting a crfree() for the same
ucred.  This would result in a double free, leading to who knows what
corruption.  I did try the test against a kernel with invariants on, but
nothing happened; presumably the timing changed enough that the race
condition wouldn't occur.

I protected the if (--cr->cr_ref == 0) in crfree() with splhigh() but left
the actuall free() out of splhigh.

The cr->cr_ref++ in crhold() assembles to an incl (%eax) which will be
atomic on a single processor.  However I don't think anything guarantees gcc
will emit that code for that source instruction, so I put a splhigh around
it too.  Probably atomic_add_int would be better -- I'll try that out too.

Our stress test is running against the patch below, and I'll report any
findings.  If you have any comments on this, please let me know.



Index: kern_prot.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_prot.c,v
retrieving revision 1.53.2.9
diff -c -3 -r1.53.2.9 kern_prot.c
*** kern_prot.c 9 Mar 2002 05:20:26 -0000       1.53.2.9
--- kern_prot.c 20 Aug 2003 22:12:29 -0000
***************
*** 997,1003 ****
--- 997,1005 ----
  crhold(cr) 
          struct ucred *cr;
  {
+         int s = splhigh();
        cr->cr_ref++;
+         splx(s);
  }
  
  /*
***************
*** 1008,1017 ****
--- 1010,1021 ----
  crfree(cr)
        struct ucred *cr;
  {
+         int s = splhigh();
        if (cr->cr_ref == 0)
                panic("Freeing already free credential! %p", cr);
        
        if (--cr->cr_ref == 0) {
+                 splx(s);
                /*
                 * Some callers of crget(), such as nfs_statfs(),
                 * allocate a temporary credential, but don't
***************
*** 1020,1026 ****
--- 1024,1032 ----
                if (cr->cr_uidinfo != NULL)
                        uifree(cr->cr_uidinfo);
                FREE((caddr_t)cr, M_CRED);
+                 return;
        }
+         splx(s);
  }
  
  /*