[src/bouyer-socketcan]: src/sys/netcan Add locking and refcounting to canpcb.

[bouyer <bouyer%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/22307e9d6b6e
branches:  bouyer-socketcan
changeset: 820902:22307e9d6b6e
user:      bouyer <bouyer%NetBSD.org@localhost>
date:      Sun Apr 23 21:05:09 2017 +0000

description:
Add locking and refcounting to canpcb.
Store the canpcb in the in the mbuf tag on send instead of the socket's address.
This should protect against a race where the socket cloud be closed before
we get back the mbuf from the adapter's driver.

diffstat:

 sys/netcan/can.c     |  41 ++++++++++++++++++++++++++---------
 sys/netcan/can_pcb.c |  58 +++++++++++++++++++++++++++++++++++++++++----------
 sys/netcan/can_pcb.h |  16 ++++++++++---
 3 files changed, 88 insertions(+), 27 deletions(-)

diffs (truncated from 329 to 300 lines):

diff -r d23335a39927 -r 22307e9d6b6e sys/netcan/can.c
--- a/sys/netcan/can.c  Sun Apr 23 11:17:25 2017 +0000
+++ b/sys/netcan/can.c  Sun Apr 23 21:05:09 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: can.c,v 1.1.2.12 2017/04/20 17:29:10 bouyer Exp $      */
+/*     $NetBSD: can.c,v 1.1.2.13 2017/04/23 21:05:09 bouyer Exp $      */
 
 /*-
  * Copyright (c) 2003, 2017 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: can.c,v 1.1.2.12 2017/04/20 17:29:10 bouyer Exp $");
+__KERNEL_RCSID(0, "$NetBSD: can.c,v 1.1.2.13 2017/04/23 21:05:09 bouyer Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -235,7 +235,10 @@
                ifp->if_oerrors++;
                return ENOMEM;
        }
-       *(struct socket **)(sotag + 1) = canp->canp_socket;
+       mutex_enter(&canp->canp_mtx);
+       canp_ref(canp);
+       mutex_exit(&canp->canp_mtx);
+       *(struct canpcb **)(sotag + 1) = canp;
        m_tag_prepend(m, sotag);
 
        if (m->m_len <= ifp->if_mtu) {
@@ -302,7 +305,6 @@
        struct sockaddr_can from;
        struct canpcb   *canp;
        struct m_tag    *sotag;
-       struct socket   *so;
        struct canpcb   *sender_canp;
 
        mutex_enter(softnet_lock);
@@ -319,13 +321,13 @@
 #endif
                sotag = m_tag_find(m, PACKET_TAG_SO, NULL);
                if (sotag) {
-                       so = *(struct socket **)(sotag + 1);
-                       sender_canp = sotocanpcb(so);
+                       sender_canp = *(struct canpcb **)(sotag + 1);
                        m_tag_delete(m, sotag);
+                       KASSERT(sender_canp != NULL);
                        /* if the sender doesn't want loopback, don't do it */
-                       if (sender_canp &&
-                           (sender_canp->canp_flags & CANP_NO_LOOPBACK) != 0) {
+                       if ((sender_canp->canp_flags & CANP_NO_LOOPBACK) != 0) {
                                m_freem(m);
+                               canp_unref(sender_canp);
                                continue;
                        }
                } else {
@@ -340,19 +342,31 @@
                TAILQ_FOREACH(canp, &cbtable.canpt_queue, canp_queue) {
                        struct mbuf *mc;
 
+                       mutex_enter(&canp->canp_mtx);
+                       /* skip if we're detached */
+                       if (canp->canp_state == CANP_DETACHED) {
+                               mutex_exit(&canp->canp_mtx);
+                               continue;
+                       }
+
                        /* don't loop back to sockets on other interfaces */
                        if (canp->canp_ifp != NULL &&
                            canp->canp_ifp->if_index != rcv_ifindex) {
+                               mutex_exit(&canp->canp_mtx);
                                continue;
                        }
                        /* don't loop back to myself if I don't want it */
                        if (canp == sender_canp &&
-                           (canp->canp_flags & CANP_RECEIVE_OWN) == 0)
+                           (canp->canp_flags & CANP_RECEIVE_OWN) == 0) {
+                               mutex_exit(&canp->canp_mtx);
                                continue;
+                       }
 
                        /* skip if the accept filter doen't match this pkt */
-                       if (!can_pcbfilter(canp, m))
+                       if (!can_pcbfilter(canp, m)) {
+                               mutex_exit(&canp->canp_mtx);
                                continue;
+                       }
 
                        if (TAILQ_NEXT(canp, canp_queue) != NULL) {
                                /*
@@ -375,9 +389,13 @@
                                m_freem(mc);
                        } else
                                sorwakeup(canp->canp_socket);
+                       mutex_exit(&canp->canp_mtx);
                        if (m == NULL)
                                break;
                }
+               if (sender_canp) {
+                       canp_unref(sender_canp);
+               }
                /* If it didn't go anywhere just delete it */
                if (m) {
                        m_freem(m);
@@ -492,7 +510,6 @@
        /*soisdisconnected(so);*/
        so->so_state &= ~SS_ISCONNECTED;        /* XXX */
        can_pcbdisconnect(canp);
-       can_pcbstate(canp, CANP_BOUND);         /* XXX */
        return 0;
 }
 
@@ -877,7 +894,9 @@
                int nfilters = sopt->sopt_size / sizeof(struct can_filter);
                if (sopt->sopt_size % sizeof(struct can_filter) != 0)
                        return EINVAL;
+               mutex_enter(&canp->canp_mtx);
                error = can_pcbsetfilter(canp, sopt->sopt_data, nfilters);
+               mutex_exit(&canp->canp_mtx);
                break;
                }
        default:
diff -r d23335a39927 -r 22307e9d6b6e sys/netcan/can_pcb.c
--- a/sys/netcan/can_pcb.c      Sun Apr 23 11:17:25 2017 +0000
+++ b/sys/netcan/can_pcb.c      Sun Apr 23 21:05:09 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: can_pcb.c,v 1.1.2.3 2017/02/05 19:44:53 bouyer Exp $   */
+/*     $NetBSD: can_pcb.c,v 1.1.2.4 2017/04/23 21:05:09 bouyer Exp $   */
 
 /*-
  * Copyright (c) 2003, 2017 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: can_pcb.c,v 1.1.2.3 2017/02/05 19:44:53 bouyer Exp $");
+__KERNEL_RCSID(0, "$NetBSD: can_pcb.c,v 1.1.2.4 2017/04/23 21:05:09 bouyer Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -106,12 +106,14 @@
        canp->canp_socket = so;
        canp->canp_filters = can_init_filter;
        canp->canp_nfilters = 1;
+       mutex_init(&canp->canp_mtx, MUTEX_DEFAULT, IPL_NET);
+       canp->canp_refcount = 1;
 
        so->so_pcb = canp;
-       s = splnet();
+       mutex_enter(&canp->canp_mtx);
        TAILQ_INSERT_HEAD(&table->canpt_queue, canp, canp_queue);
        can_pcbstate(canp, CANP_ATTACHED);
-       splx(s);
+       mutex_exit(&canp->canp_mtx);
        return (0);
 }
 
@@ -122,6 +124,7 @@
 
        if (scan->can_family != AF_CAN)
                return (EAFNOSUPPORT);
+       mutex_enter(&canp->canp_mtx);
        if (scan->can_ifindex != 0) {
                canp->canp_ifp = if_byindex(scan->can_ifindex);
                if (canp->canp_ifp == NULL)
@@ -132,6 +135,7 @@
                canp->canp_socket->so_state &= ~SS_ISCONNECTED; /* XXX */
        }
        can_pcbstate(canp, CANP_BOUND);
+       mutex_exit(&canp->canp_mtx);
        return 0;
 }
 
@@ -150,8 +154,10 @@
        if (scan->can_family != AF_CAN)
                return (EAFNOSUPPORT);
 #if 0
+       mutex_enter(&canp->canp_mtx);
        memcpy(&canp->canp_dst, scan, sizeof(struct sockaddr_can));
        can_pcbstate(canp, CANP_CONNECTED);
+       mutex_exit(&canp->canp_mtx);
        return 0;
 #endif
        return EOPNOTSUPP;
@@ -162,7 +168,9 @@
 {
        struct canpcb *canp = v;
 
-       can_pcbstate(canp, CANP_BOUND);
+       mutex_enter(&canp->canp_mtx);
+       can_pcbstate(canp, CANP_DETACHED);
+       mutex_exit(&canp->canp_mtx);
        if (canp->canp_socket->so_state & SS_NOFDREF)
                can_pcbdetach(canp);
 }
@@ -172,28 +180,51 @@
 {
        struct canpcb *canp = v;
        struct socket *so = canp->canp_socket;
-       int s;
 
        KASSERT(mutex_owned(softnet_lock));
        so->so_pcb = NULL;
-       s =  splnet();
-       can_pcbstate(canp, CANP_ATTACHED);
+       mutex_enter(&canp->canp_mtx);
+       can_pcbstate(canp, CANP_DETACHED);
+       can_pcbsetfilter(canp, NULL, 0);
+       mutex_exit(&canp->canp_mtx);
        TAILQ_REMOVE(&canp->canp_table->canpt_queue, canp, canp_queue);
-       splx(s);
-       sofree(so); /* sofree drops the lock */
-       can_pcbsetfilter(canp, NULL, 0);
+       sofree(so); /* sofree drops the softnet_lock */
+       canp_unref(canp);
+       mutex_enter(softnet_lock);
+}
+
+void
+canp_ref(struct canpcb *canp)
+{
+       KASSERT(mutex_owned(&canp->canp_mtx));
+       canp->canp_refcount++;
+}
+
+void
+canp_unref(struct canpcb *canp)
+{
+       mutex_enter(&canp->canp_mtx);
+       canp->canp_refcount--;
+       KASSERT(canp->canp_refcount >= 0);
+       if (canp->canp_refcount > 0) {
+               mutex_exit(&canp->canp_mtx);
+               return;
+       }
+       mutex_exit(&canp->canp_mtx);
+       mutex_destroy(&canp->canp_mtx);
        pool_put(&canpcb_pool, canp);
-       mutex_enter(softnet_lock);
 }
 
 void
 can_setsockaddr(struct canpcb *canp, struct sockaddr_can *scan)
 {
 
+       mutex_enter(&canp->canp_mtx);
        memset(scan, 0, sizeof (*scan));
        scan->can_family = AF_CAN;
        scan->can_len = sizeof(*scan);
        scan->can_ifindex = canp->canp_ifp->if_index;
+       mutex_exit(&canp->canp_mtx);
 }
 
 int
@@ -201,6 +232,7 @@
 {
 
        struct can_filter *newf;
+       KASSERT(mutex_owned(&canp->canp_mtx));
 
        if (nfilters > 0) {
                newf =
@@ -303,6 +335,7 @@
 can_pcbstate(struct canpcb *canp, int state)
 {
        int ifindex = canp->canp_ifp ? canp->canp_ifp->if_index : 0;
+       KASSERT(mutex_owned(&canp->canp_mtx));
 
        if (canp->canp_state > CANP_ATTACHED)
                LIST_REMOVE(canp, canp_hash);
@@ -332,6 +365,7 @@
        struct can_frame *fmp;
        struct can_filter *fip;
 
+       KASSERT(mutex_owned(&canp->canp_mtx));
        KASSERT((m->m_flags & M_PKTHDR) != 0);
        KASSERT(m->m_len == m->m_pkthdr.len);
 
diff -r d23335a39927 -r 22307e9d6b6e sys/netcan/can_pcb.h
--- a/sys/netcan/can_pcb.h      Sun Apr 23 11:17:25 2017 +0000
+++ b/sys/netcan/can_pcb.h      Sun Apr 23 21:05:09 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: can_pcb.h,v 1.1.2.3 2017/02/05 10:56:12 bouyer Exp $   */
+/*     $NetBSD: can_pcb.h,v 1.1.2.4 2017/04/23 21:05:09 bouyer Exp $   */
 
 /*-
  * Copyright (c) 2003, 2017 The NetBSD Foundation, Inc.
@@ -48,6 +48,7 @@
        LIST_ENTRY(canpcb) canp_hash;
        LIST_ENTRY(canpcb) canp_lhash;
        TAILQ_ENTRY(canpcb) canp_queue;
+       kmutex_t        canp_mtx;       /* protect states and refcount */
        int             canp_state;
        int             canp_flags;
        struct          socket *canp_socket;    /* back pointer to socket */
@@ -56,6 +57,8 @@
        struct          canpcbtable *canp_table;
        struct          can_filter *canp_filters; /* filter array */
        int             canp_nfilters; /* size of canp_filters */
+