Revert per-socket `so_lock' rwlock(9) and use it to protect routing
authorclaudio <claudio@openbsd.org>
Mon, 26 Apr 2021 08:21:35 +0000 (08:21 +0000)
committerclaudio <claudio@openbsd.org>
Mon, 26 Apr 2021 08:21:35 +0000 (08:21 +0000)
(PF_ROUTE) sockets. There is a locking issue with timeouts that needs
to be fixed.
Requested by deraadt@

sys/kern/uipc_socket.c
sys/kern/uipc_socket2.c
sys/net/rtsock.c
sys/sys/socketvar.h

index d457f36..52b6c47 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_socket.c,v 1.257 2021/04/25 00:00:34 mvs Exp $   */
+/*     $OpenBSD: uipc_socket.c,v 1.258 2021/04/26 08:21:35 claudio Exp $       */
 /*     $NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $        */
 
 /*
@@ -151,7 +151,6 @@ socreate(int dom, struct socket **aso, int type, int proto)
        if (prp->pr_type != type)
                return (EPROTOTYPE);
        so = pool_get(&socket_pool, PR_WAITOK | PR_ZERO);
-       rw_init(&so->so_lock, "solock");
        sigio_init(&so->so_sigio);
        TAILQ_INIT(&so->so_q0);
        TAILQ_INIT(&so->so_q);
index 068e3fb..940da14 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_socket2.c,v 1.107 2021/04/25 00:00:34 mvs Exp $  */
+/*     $OpenBSD: uipc_socket2.c,v 1.108 2021/04/26 08:21:35 claudio Exp $      */
 /*     $NetBSD: uipc_socket2.c,v 1.11 1996/02/04 02:17:55 christos Exp $       */
 
 /*
@@ -162,7 +162,6 @@ sonewconn(struct socket *head, int connstatus)
        so = pool_get(&socket_pool, PR_NOWAIT|PR_ZERO);
        if (so == NULL)
                return (NULL);
-       rw_init(&so->so_lock, "solock");
        so->so_type = head->so_type;
        so->so_options = head->so_options &~ SO_ACCEPTCONN;
        so->so_linger = head->so_linger;
@@ -288,8 +287,6 @@ solock(struct socket *so)
                rw_enter_write(&unp_lock);
                break;
        case PF_ROUTE:
-               rw_enter_write(&so->so_lock);
-               break;
        case PF_KEY:
        default:
                KERNEL_LOCK();
@@ -316,8 +313,6 @@ sounlock(struct socket *so, int s)
                rw_exit_write(&unp_lock);
                break;
        case PF_ROUTE:
-               rw_exit_write(&so->so_lock);
-               break;
        case PF_KEY:
        default:
                KERNEL_UNLOCK();
@@ -337,8 +332,6 @@ soassertlocked(struct socket *so)
                rw_assert_wrlock(&unp_lock);
                break;
        case PF_ROUTE:
-               rw_assert_wrlock(&so->so_lock);
-               break;
        case PF_KEY:
        default:
                KERNEL_ASSERT_LOCKED();
@@ -361,8 +354,6 @@ sosleep_nsec(struct socket *so, void *ident, int prio, const char *wmesg,
                ret = rwsleep_nsec(ident, &unp_lock, prio, wmesg, nsecs);
                break;
        case PF_ROUTE:
-               ret = rwsleep_nsec(ident, &so->so_lock, prio, wmesg, nsecs);
-               break;
        case PF_KEY:
        default:
                ret = tsleep_nsec(ident, prio, wmesg, nsecs);
index ad662dd..ac7c163 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rtsock.c,v 1.310 2021/04/25 00:00:35 mvs Exp $        */
+/*     $OpenBSD: rtsock.c,v 1.311 2021/04/26 08:21:36 claudio Exp $    */
 /*     $NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $  */
 
 /*
@@ -143,7 +143,7 @@ int          rt_setsource(unsigned int, struct sockaddr *);
 /*
  * Locks used to protect struct members
  *       I       immutable after creation
- *       s       solock
+ *       sK      solock (kernel lock)
  */
 struct rtpcb {
        struct socket           *rop_socket;            /* [I] */
@@ -151,12 +151,12 @@ struct rtpcb {
        SRPL_ENTRY(rtpcb)       rop_list;
        struct refcnt           rop_refcnt;
        struct timeout          rop_timeout;
-       unsigned int            rop_msgfilter;          /* [s] */
-       unsigned int            rop_flagfilter;         /* [s] */
-       unsigned int            rop_flags;              /* [s] */
-       u_int                   rop_rtableid;           /* [s] */
+       unsigned int            rop_msgfilter;          /* [sK] */
+       unsigned int            rop_flagfilter;         /* [sK] */
+       unsigned int            rop_flags;              /* [sK] */
+       u_int                   rop_rtableid;           /* [sK] */
        unsigned short          rop_proto;              /* [I] */
-       u_char                  rop_priority;           /* [s] */
+       u_char                  rop_priority;           /* [sK] */
 };
 #define        sotortpcb(so)   ((struct rtpcb *)(so)->so_pcb)
 
@@ -188,7 +188,7 @@ route_prinit(void)
        rw_init(&rtptable.rtp_lk, "rtsock");
        SRPL_INIT(&rtptable.rtp_list);
        pool_init(&rtpcb_pool, sizeof(struct rtpcb), 0,
-           IPL_SOFTNET, PR_WAITOK, "rtpcb", NULL);
+           IPL_NONE, PR_WAITOK, "rtpcb", NULL);
 }
 
 void
@@ -352,13 +352,9 @@ route_detach(struct socket *so)
            rop_list);
        rw_exit(&rtptable.rtp_lk);
 
-       sounlock(so, SL_LOCKED);
-
        /* wait for all references to drop */
        refcnt_finalize(&rop->rop_refcnt, "rtsockrefs");
 
-       solock(so);
-
        so->so_pcb = NULL;
        KASSERT((so->so_state & SS_NOFDREF) == 0);
        pool_put(&rtpcb_pool, rop);
@@ -680,7 +676,7 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
        struct rtentry          *rt = NULL;
        struct rt_addrinfo       info;
        struct ifnet            *ifp;
-       int                      len, seq, useloopback, error = 0;
+       int                      len, seq, error = 0;
        u_int                    tableid;
        u_int8_t                 prio;
        u_char                   vers, type;
@@ -690,16 +686,6 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
                return (ENOBUFS);
        if ((m->m_flags & M_PKTHDR) == 0)
                panic("route_output");
-
-       useloopback = so->so_options & SO_USELOOPBACK;
-
-       /*
-        * The socket can't be closed concurrently because the file
-        * descriptor reference is still held.
-        */
-
-       sounlock(so, SL_LOCKED);
-
        len = m->m_pkthdr.len;
        if (len < offsetof(struct rt_msghdr, rtm_hdrlen) + 1 ||
            len != mtod(m, struct rt_msghdr *)->rtm_msglen) {
@@ -873,10 +859,13 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
        /*
         * Check to see if we don't want our own messages.
         */
-       if (!useloopback) {
-               if (rtptable.rtp_count == 0) {
+       if (!(so->so_options & SO_USELOOPBACK)) {
+               if (rtptable.rtp_count <= 1) {
                        /* no other listener and no loopback of messages */
-                       goto fail;
+fail:
+                       free(rtm, M_RTABLE, len);
+                       m_freem(m);
+                       return (error);
                }
        }
        if (m_copyback(m, 0, len, rtm, M_NOWAIT)) {
@@ -888,13 +877,6 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
        if (m)
                route_input(m, so, info.rti_info[RTAX_DST] ?
                    info.rti_info[RTAX_DST]->sa_family : AF_UNSPEC);
-       solock(so);
-
-       return (error);
-fail:
-       free(rtm, M_RTABLE, len);
-       m_freem(m);
-       solock(so);
 
        return (error);
 }
index 6a7dfaa..1884573 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: socketvar.h,v 1.94 2021/04/25 00:00:35 mvs Exp $      */
+/*     $OpenBSD: socketvar.h,v 1.95 2021/04/26 08:21:36 claudio Exp $  */
 /*     $NetBSD: socketvar.h,v 1.18 1996/02/09 18:25:38 christos Exp $  */
 
 /*-
@@ -37,7 +37,6 @@
 #include <sys/sigio.h>                         /* for struct sigio_ref */
 #include <sys/task.h>
 #include <sys/timeout.h>
-#include <sys/rwlock.h>
 
 #ifndef        _SOCKLEN_T_DEFINED_
 #define        _SOCKLEN_T_DEFINED_
@@ -54,7 +53,6 @@ TAILQ_HEAD(soqhead, socket);
  */
 struct socket {
        const struct protosw *so_proto; /* protocol handle */
-       struct rwlock so_lock;          /* this socket lock */
        void    *so_pcb;                /* protocol control block */
        u_int   so_state;               /* internal state flags SS_*, below */
        short   so_type;                /* generic type, see socket.h */