Subject: kern/7583: serious race condition in sosend() and soreceive()
To: None <gnats-bugs@gnats.netbsd.org>
From: None <tv@pobox.com>
List: netbsd-bugs
Date: 05/14/1999 12:51:01
>Number:         7583
>Category:       kern
>Synopsis:       serious race condition in sosend() and soreceive()
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri May 14 12:50:00 1999
>Last-Modified:
>Originator:     Todd Vierling
>Organization:
	DuhNet: Pointing out the obvious since 1994.

>Release:        1.4-release
>Environment:

System: NetBSD duhnet.net 1.4 NetBSD 1.4 (DUH) #5: Thu May 13 19:22:40 EDT 1999 tv@duhnet.net:/usr/SRC/netbsd/src/sys/arch/alpha/compile/DUH alpha

>Description:

There's a pretty serious race condition in sosend() in uipc_socket.c (1.4)
which I am bumping into once a day (sometimes twice) ever since my system
load increased from a typical 0.6 to a typical 1.6.

The following happens:

        do { /* line 403 */
                s = splsoftnet();
                if (so->so_state & SS_CANTSENDMORE)
                        snderr(EPIPE); /* which doesn't happen */
...
                splx(s); /* line 434 */
...
                        /* line 481: */
                        error = uiomove(mtod(m, caddr_t), (int)len, uio);
...
                    /* line 499: */
                    s = splsoftnet();                           /* XXX */
                    error = (*so->so_proto->pr_usrreq)(so,
                        (flags & MSG_OOB) ? PRU_SENDOOB : PRU_SEND,
                        top, addr, control, p);  
                    splx(s);

Nowhere else is SS_CANTSENDMORE (part of the flags set when the socket's
other end is disconnecting) checked inside a splsoftnet().  Classic race
condition.

A similar thing happens in soreceive(), with a smaller window, but the
uiomove is enough to trigger a page fault and context switch:

                        splx(s); /* line 743 */
                        error = uiomove(mtod(m, caddr_t) + moff, (int)len, uio);
                        s = splsoftnet();

Which is after SS_CANTRCVMORE is checked, of course.

>How-To-Repeat:

Heavily load the system, and open lots of pipes to programs which
disconnect the pipes early.  (In this case, procmail was attempting to
pipe to test(1), which obviously wouldn't work).

>Fix:

I'm frankly afraid of touching weaving mbuf code like this, but Bill
Sommerfeld had a suggestion which echoed my thoughts:

: i bet the right fix here is to check SS_CANTSENDMORE again
: immediately before the call to so_proto->pr_usrreq, and, if
: so, free the mbuf chain and bail with EPIPE.
: (and stretch the size of the splsoftnet-protected region to
: cover the code immediately above and below which touches
: so_options and so_state)

If someone has a free half hour and knows the code better than I, please
check up on this.
>Audit-Trail:
>Unformatted: