network - Fix race in accept()
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 17 Sep 2010 06:00:26 +0000 (23:00 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 17 Sep 2010 06:00:26 +0000 (23:00 -0700)
* Fix a race where a socket undergoing an accept() was not being
  referenced soon enough, resulting in a window of opportunity for
  the kernel to attempt to free it if the tcp connection resets
  before userland can finish the accept.

  This resulted in an assertion panic.

Reported-by: Peter Avalos
sys/kern/uipc_socket.c
sys/kern/uipc_syscalls.c
sys/netgraph/ksocket/ng_ksocket.c

index 811ba03..7cb7624 100644 (file)
@@ -452,6 +452,10 @@ soabort_oncpu(struct socket *so)
        so_pru_abort_oncpu(so);
 }
 
+/*
+ * so is passed in ref'd, which becomes owned by
+ * the cleared SS_NOFDREF flag.
+ */
 int
 soaccept(struct socket *so, struct sockaddr **nam)
 {
@@ -459,7 +463,6 @@ soaccept(struct socket *so, struct sockaddr **nam)
 
        if ((so->so_state & SS_NOFDREF) == 0)
                panic("soaccept: !NOFDREF");
-       soreference(so);                /* create ref */
        soclrstate(so, SS_NOFDREF);     /* owned by lack of SS_NOFDREF */
        error = so_pru_accept_direct(so, nam);
        return (error);
index 16f7cb5..6686d02 100644 (file)
@@ -219,6 +219,7 @@ static boolean_t
 soaccept_predicate(struct netmsg_so_notify *msg)
 {
        struct socket *head = msg->base.nm_so;
+       struct socket *so;
 
        if (head->so_error != 0) {
                msg->base.lmsg.ms_error = head->so_error;
@@ -227,12 +228,17 @@ soaccept_predicate(struct netmsg_so_notify *msg)
        lwkt_gettoken(&head->so_rcv.ssb_token);
        if (!TAILQ_EMPTY(&head->so_comp)) {
                /* Abuse nm_so field as copy in/copy out parameter. XXX JH */
-               msg->base.nm_so = TAILQ_FIRST(&head->so_comp);
-               TAILQ_REMOVE(&head->so_comp, msg->base.nm_so, so_list);
+               so = TAILQ_FIRST(&head->so_comp);
+               TAILQ_REMOVE(&head->so_comp, so, so_list);
                head->so_qlen--;
+               soclrstate(so, SS_COMP);
+               so->so_head = NULL;
+               soreference(so);
 
-               msg->base.lmsg.ms_error = 0;
                lwkt_reltoken(&head->so_rcv.ssb_token);
+
+               msg->base.lmsg.ms_error = 0;
+               msg->base.nm_so = so;
                return (TRUE);
        }
        lwkt_reltoken(&head->so_rcv.ssb_token);
@@ -306,6 +312,9 @@ kern_accept(int s, int fflags, struct sockaddr **name, int *namelen, int *res)
 
        /*
         * At this point we have the connection that's ready to be accepted.
+        *
+        * NOTE! soaccept_predicate() ref'd so for us, and soaccept() expects
+        *       to eat the ref and turn it into a descriptor.
         */
        so = msg.base.nm_so;
 
@@ -314,8 +323,6 @@ kern_accept(int s, int fflags, struct sockaddr **name, int *namelen, int *res)
        /* connection has been removed from the listen queue */
        KNOTE(&head->so_rcv.ssb_kq.ki_note, 0);
 
-       soclrstate(so, SS_COMP);
-       so->so_head = NULL;
        if (head->so_sigio != NULL)
                fsetown(fgetown(head->so_sigio), &so->so_sigio);
 
index 0ada544..51e2221 100644 (file)
@@ -1174,13 +1174,14 @@ ng_ksocket_finish_accept(priv_p priv, struct ng_mesg **rptr)
        }
        TAILQ_REMOVE(&head->so_comp, so, so_list);
        head->so_qlen--;
+       soclrstate(so, SS_COMP);
+       so->so_head = NULL;
+       soreference(so);
+
        lwkt_reltoken(&head->so_rcv.ssb_token);
 
        /* XXX KNOTE(&head->so_rcv.ssb_sel.si_note, 0); */
 
-       soclrstate(so, SS_COMP);
-       so->so_head = NULL;
-
        soaccept(so, &sa);
 
        len = OFFSETOF(struct ng_ksocket_accept, addr);