Move solock() down to sosetopt(). A part of standalone sblock() work.

This movement required because buffers related SO_SND* and SO_RCV*
socket options should be protected with sblock(). However, standalone
sblock() has different lock order with solock() and `so_snd' and
`so_rcv' buffers. At least sblock() for `so_snd' buffer will always be
taken before solock() in the sosend() path.

The (*pr_ctloutput)() call was removed from the SOL_SOCKET level 'else'
branch. Except the SO_RTABLE case where it handled in the special way,
this is null op call.

For SO_SND* and SO_RCV* cases solock() will be replaced by sblock() in
the future.

Feedback from bluhm

Tested by bluhm naddy

ok bluhm

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 9824d16..af0f621 100644 (file)
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_socket.c,v 1.306 2023/07/22 14:30:39 mvs Exp $   */
+/*     $OpenBSD: uipc_socket.c,v 1.307 2023/08/03 09:49:08 mvs Exp $   */
 /*     $NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $        */
 
 /*
@@ -1789,12 +1789,12 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 {
        int error = 0;
 
-       soassertlocked(so);
-
        if (level != SOL_SOCKET) {
                if (so->so_proto->pr_ctloutput) {
+                       solock(so);
                        error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
                            level, optname, m);
+                       sounlock(so);
                        return (error);
                }
                error = ENOPROTOOPT;
@@ -1813,9 +1813,16 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                            mtod(m, struct linger *)->l_linger < 0 ||
                            mtod(m, struct linger *)->l_linger > SHRT_MAX)
                                return (EINVAL);
+
+                       solock(so);
                        so->so_linger = mtod(m, struct linger *)->l_linger;
-                       /* FALLTHROUGH */
+                       if (*mtod(m, int *))
+                               so->so_options |= optname;
+                       else
+                               so->so_options &= ~optname;
+                       sounlock(so);
 
+                       break;
                case SO_BINDANY:
                case SO_DEBUG:
                case SO_KEEPALIVE:
@@ -1828,12 +1835,15 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                case SO_ZEROIZE:
                        if (m == NULL || m->m_len < sizeof (int))
                                return (EINVAL);
+
+                       solock(so);
                        if (*mtod(m, int *))
                                so->so_options |= optname;
                        else
                                so->so_options &= ~optname;
-                       break;
+                       sounlock(so);
 
+                       break;
                case SO_DONTROUTE:
                        if (m == NULL || m->m_len < sizeof (int))
                                return (EINVAL);
@@ -1853,23 +1863,32 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                        cnt = *mtod(m, int *);
                        if ((long)cnt <= 0)
                                cnt = 1;
-                       switch (optname) {
 
+                       solock(so);
+                       switch (optname) {
                        case SO_SNDBUF:
-                               if (so->so_snd.sb_state & SS_CANTSENDMORE)
-                                       return (EINVAL);
+                               if (so->so_snd.sb_state & SS_CANTSENDMORE) {
+                                       error = EINVAL;
+                                       break;
+                               }
                                if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
-                                   sbreserve(so, &so->so_snd, cnt))
-                                       return (ENOBUFS);
+                                   sbreserve(so, &so->so_snd, cnt)) {
+                                       error = ENOBUFS;
+                                       break;
+                               }
                                so->so_snd.sb_wat = cnt;
                                break;
 
                        case SO_RCVBUF:
-                               if (so->so_rcv.sb_state & SS_CANTRCVMORE)
-                                       return (EINVAL);
+                               if (so->so_rcv.sb_state & SS_CANTRCVMORE) {
+                                       error = EINVAL;
+                                       break;
+                               }
                                if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
-                                   sbreserve(so, &so->so_rcv, cnt))
-                                       return (ENOBUFS);
+                                   sbreserve(so, &so->so_rcv, cnt)) {
+                                       error = ENOBUFS;
+                                       break;
+                               }
                                so->so_rcv.sb_wat = cnt;
                                break;
 
@@ -1884,6 +1903,7 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                                    so->so_rcv.sb_hiwat : cnt;
                                break;
                        }
+                       sounlock(so);
                        break;
                    }
 
@@ -1903,8 +1923,9 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                                return (EDOM);
                        if (nsecs == 0)
                                nsecs = INFSLP;
-                       switch (optname) {
 
+                       solock(so);
+                       switch (optname) {
                        case SO_SNDTIMEO:
                                so->so_snd.sb_timeo_nsecs = nsecs;
                                break;
@@ -1912,6 +1933,7 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                                so->so_rcv.sb_timeo_nsecs = nsecs;
                                break;
                        }
+                       sounlock(so);
                        break;
                    }
 
@@ -1923,19 +1945,20 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                                    so->so_proto->pr_domain;
 
                                level = dom->dom_protosw->pr_protocol;
+                               solock(so);
                                error = (*so->so_proto->pr_ctloutput)
                                    (PRCO_SETOPT, so, level, optname, m);
-                               return (error);
-                       }
-                       error = ENOPROTOOPT;
+                               sounlock(so);
+                       } else
+                               error = ENOPROTOOPT;
                        break;
-
 #ifdef SOCKET_SPLICE
                case SO_SPLICE:
+                       solock(so);
                        if (m == NULL) {
                                error = sosplice(so, -1, 0, NULL);
                        } else if (m->m_len < sizeof(int)) {
-                               return (EINVAL);
+                               error = EINVAL;
                        } else if (m->m_len < sizeof(struct splice)) {
                                error = sosplice(so, *mtod(m, int *), 0, NULL);
                        } else {
@@ -1944,6 +1967,7 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                                    mtod(m, struct splice *)->sp_max,
                                   &mtod(m, struct splice *)->sp_idle);
                        }
+                       sounlock(so);
                        break;
 #endif /* SOCKET_SPLICE */
 
@@ -1951,10 +1975,6 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
                        error = ENOPROTOOPT;
                        break;
                }
-               if (error == 0 && so->so_proto->pr_ctloutput) {
-                       (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
-                           level, optname, m);
-               }
        }
 
        return (error);

diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index 6b8bfc8..7e6cb9b 100644 (file)
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_syscalls.c,v 1.212 2023/02/10 14:34:17 visa Exp $        */
+/*     $OpenBSD: uipc_syscalls.c,v 1.213 2023/08/03 09:49:08 mvs Exp $ */
 /*     $NetBSD: uipc_syscalls.c,v 1.19 1996/02/09 19:00:48 christos Exp $      */
 
 /*
@@ -1232,9 +1232,7 @@ sys_setsockopt(struct proc *p, void *v, register_t *retval)
                m->m_len = SCARG(uap, valsize);
        }
        so = fp->f_data;
-       solock(so);
        error = sosetopt(so, SCARG(uap, level), SCARG(uap, name), m);
-       sounlock(so);
 bad:
        m_freem(m);
        FRELE(fp, p);

diff --git a/sys/net/bfd.c b/sys/net/bfd.c
index 4b10558..9592c1b 100644 (file)
--- a/sys/net/bfd.c
+++ b/sys/net/bfd.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: bfd.c,v 1.79 2023/07/12 16:10:45 mvs Exp $    */
+/*     $OpenBSD: bfd.c,v 1.80 2023/08/03 09:49:08 mvs Exp $    */
 
 /*
  * Copyright (c) 2016-2018 Peter Hessler <phessler@openbsd.org>
@@ -452,9 +452,7 @@ bfd_listener(struct bfd_config *bfd, unsigned int port)
        mopt->m_len = sizeof(int);
        ip = mtod(mopt, int *);
        *ip = MAXTTL;
-       solock(so);
        error = sosetopt(so, IPPROTO_IP, IP_MINTTL, mopt);
-       sounlock(so);
        m_freem(mopt);
        if (error) {
                printf("%s: sosetopt error %d\n",
@@ -531,9 +529,7 @@ bfd_sender(struct bfd_config *bfd, unsigned int port)
        mopt->m_len = sizeof(int);
        ip = mtod(mopt, int *);
        *ip = IP_PORTRANGE_HIGH;
-       solock(so);
        error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
-       sounlock(so);
        m_freem(mopt);
        if (error) {
                printf("%s: sosetopt error %d\n",
@@ -545,9 +541,7 @@ bfd_sender(struct bfd_config *bfd, unsigned int port)
        mopt->m_len = sizeof(int);
        ip = mtod(mopt, int *);
        *ip = MAXTTL;
-       solock(so);
        error = sosetopt(so, IPPROTO_IP, IP_TTL, mopt);
-       sounlock(so);
        m_freem(mopt);
        if (error) {
                printf("%s: sosetopt error %d\n",
@@ -559,9 +553,7 @@ bfd_sender(struct bfd_config *bfd, unsigned int port)
        mopt->m_len = sizeof(int);
        ip = mtod(mopt, int *);
        *ip = IPTOS_PREC_INTERNETCONTROL;
-       solock(so);
        error = sosetopt(so, IPPROTO_IP, IP_TOS, mopt);
-       sounlock(so);
        m_freem(mopt);
        if (error) {
                printf("%s: sosetopt error %d\n",

diff --git a/sys/net/if_vxlan.c b/sys/net/if_vxlan.c
index 937f36b..37311eb 100644 (file)
--- a/sys/net/if_vxlan.c
+++ b/sys/net/if_vxlan.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_vxlan.c,v 1.92 2023/04/13 02:19:05 jsg Exp $ */
+/*     $OpenBSD: if_vxlan.c,v 1.93 2023/08/03 09:49:08 mvs Exp $ */
 
 /*
  * Copyright (c) 2021 David Gwynne <dlg@openbsd.org>
@@ -934,9 +934,9 @@ vxlan_tep_add_addr(struct vxlan_softc *sc, const union vxlan_addr *addr,
                goto free;
 
        solock(so);
-
        sotoinpcb(so)->inp_upcall = vxlan_input;
        sotoinpcb(so)->inp_upcall_arg = vt;
+       sounlock(so);
 
        m_inithdr(&m);
        m.m_len = sizeof(vt->vt_rdomain);
@@ -973,12 +973,12 @@ vxlan_tep_add_addr(struct vxlan_softc *sc, const union vxlan_addr *addr,
                unhandled_af(vt->vt_af);
        }
 
+       solock(so);
        error = sobind(so, &m, curproc);
+       sounlock(so);
        if (error != 0)
                goto close;
 
-       sounlock(so);
-
        rw_assert_wrlock(&vxlan_lock);
        TAILQ_INSERT_TAIL(&vxlan_teps, vt, vt_entry);
 
@@ -987,7 +987,6 @@ vxlan_tep_add_addr(struct vxlan_softc *sc, const union vxlan_addr *addr,
        return (0);
 
 close:
-       sounlock(so);
        soclose(so, MSG_DONTWAIT);
 free:
        free(vt, M_DEVBUF, sizeof(*vt));

diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
index 951cb6f..593bfc6 100644 (file)
--- a/sys/net/if_wg.c
+++ b/sys/net/if_wg.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_wg.c,v 1.28 2023/06/01 18:57:53 kn Exp $ */
+/*     $OpenBSD: if_wg.c,v 1.29 2023/08/03 09:49:08 mvs Exp $ */
 
 /*
  * Copyright (C) 2015-2020 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
@@ -720,14 +720,16 @@ wg_socket_open(struct socket **so, int af, in_port_t *port,
        solock(*so);
        sotoinpcb(*so)->inp_upcall = wg_input;
        sotoinpcb(*so)->inp_upcall_arg = upcall_arg;
+       sounlock(*so);
 
        if ((ret = sosetopt(*so, SOL_SOCKET, SO_RTABLE, &mrtable)) == 0) {
+               solock(*so);
                if ((ret = sobind(*so, &mhostnam, curproc)) == 0) {
                        *port = sotoinpcb(*so)->inp_lport;
                        *rtable = sotoinpcb(*so)->inp_rtableid;
                }
+               sounlock(*so);
        }
-       sounlock(*so);
 
        if (ret != 0)
                wg_socket_close(so);

diff --git a/sys/nfs/krpc_subr.c b/sys/nfs/krpc_subr.c
index eb35b5f..fec8ae7 100644 (file)
--- a/sys/nfs/krpc_subr.c
+++ b/sys/nfs/krpc_subr.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: krpc_subr.c,v 1.37 2022/06/06 14:45:41 claudio Exp $  */
+/*     $OpenBSD: krpc_subr.c,v 1.38 2023/08/03 09:49:09 mvs Exp $      */
 /*     $NetBSD: krpc_subr.c,v 1.12.4.1 1996/06/07 00:52:26 cgd Exp $   */
 
 /*
@@ -239,9 +239,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
        tv.tv_usec = 0;
        memcpy(mtod(m, struct timeval *), &tv, sizeof tv);
        m->m_len = sizeof(tv);
-       solock(so);
        error = sosetopt(so, SOL_SOCKET, SO_RCVTIMEO, m);
-       sounlock(so);
        m_freem(m);
        if (error)
                goto out;
@@ -255,9 +253,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
                on = mtod(m, int32_t *);
                m->m_len = sizeof(*on);
                *on = 1;
-               solock(so);
                error = sosetopt(so, SOL_SOCKET, SO_BROADCAST, m);
-               sounlock(so);
                m_freem(m);
                if (error)
                        goto out;
@@ -272,9 +268,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
        mopt->m_len = sizeof(int);
        ip = mtod(mopt, int *);
        *ip = IP_PORTRANGE_LOW;
-       solock(so);
        error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
-       sounlock(so);
        m_freem(mopt);
        if (error)
                goto out;
@@ -299,9 +293,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
        mopt->m_len = sizeof(int);
        ip = mtod(mopt, int *);
        *ip = IP_PORTRANGE_DEFAULT;
-       solock(so);
        error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
-       sounlock(so);
        m_freem(mopt);
        if (error)
                goto out;

diff --git a/sys/nfs/nfs_socket.c b/sys/nfs/nfs_socket.c
index 5488a50..80dfaa6 100644 (file)
--- a/sys/nfs/nfs_socket.c
+++ b/sys/nfs/nfs_socket.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: nfs_socket.c,v 1.143 2022/08/13 21:01:46 mvs Exp $    */
+/*     $OpenBSD: nfs_socket.c,v 1.144 2023/08/03 09:49:09 mvs Exp $    */
 /*     $NetBSD: nfs_socket.c,v 1.27 1996/04/15 20:20:00 thorpej Exp $  */
 
 /*
@@ -258,7 +258,6 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
                MGET(nam, M_WAIT, MT_SONAME);
 
        so = nmp->nm_so;
-       solock(so);
        nmp->nm_soflags = so->so_proto->pr_flags;
 
        /*
@@ -282,7 +281,9 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
                sin->sin_family = AF_INET;
                sin->sin_addr.s_addr = INADDR_ANY;
                sin->sin_port = htons(0);
+               solock(so);
                error = sobind(so, nam, &proc0);
+               sounlock(so);
                if (error)
                        goto bad;
 
@@ -294,6 +295,7 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
                        goto bad;
        }
 
+       solock(so);
        /*
         * Protocols that do not require connections may be optionally left
         * unconnected for servers that reply from a port other than NFS_PORT.
@@ -301,12 +303,12 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
        if (nmp->nm_flag & NFSMNT_NOCONN) {
                if (nmp->nm_soflags & PR_CONNREQUIRED) {
                        error = ENOTCONN;
-                       goto bad;
+                       goto bad_locked;
                }
        } else {
                error = soconnect(so, nmp->nm_nam);
                if (error)
-                       goto bad;
+                       goto bad_locked;
 
                /*
                 * Wait for the connection to complete. Cribbed from the
@@ -320,13 +322,13 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
                            so->so_error == 0 && rep &&
                            (error = nfs_sigintr(nmp, rep, rep->r_procp)) != 0){
                                so->so_state &= ~SS_ISCONNECTING;
-                               goto bad;
+                               goto bad_locked;
                        }
                }
                if (so->so_error) {
                        error = so->so_error;
                        so->so_error = 0;
-                       goto bad;
+                       goto bad_locked;
                }
        }
        /*
@@ -338,6 +340,7 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
                so->so_snd.sb_timeo_nsecs = SEC_TO_NSEC(5);
        else
                so->so_snd.sb_timeo_nsecs = INFSLP;
+       sounlock(so);
        if (nmp->nm_sotype == SOCK_DGRAM) {
                sndreserve = nmp->nm_wsize + NFS_MAXPKTHDR;
                rcvreserve = (max(nmp->nm_rsize, nmp->nm_readdirsize) +
@@ -360,9 +363,10 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
        } else {
                panic("%s: nm_sotype %d", __func__, nmp->nm_sotype);
        }
+       solock(so);
        error = soreserve(so, sndreserve, rcvreserve);
        if (error)
-               goto bad;
+               goto bad_locked;
        so->so_rcv.sb_flags |= SB_NOINTR;
        so->so_snd.sb_flags |= SB_NOINTR;
        sounlock(so);
@@ -377,8 +381,9 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
        nmp->nm_timeouts = 0;
        return (0);
 
-bad:
+bad_locked:
        sounlock(so);
+bad:
 
        m_freem(mopt);
        m_freem(nam);

diff --git a/sys/nfs/nfs_syscalls.c b/sys/nfs/nfs_syscalls.c
index b9eecb0..45ff763 100644 (file)
--- a/sys/nfs/nfs_syscalls.c
+++ b/sys/nfs/nfs_syscalls.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: nfs_syscalls.c,v 1.118 2022/06/06 14:45:41 claudio Exp $      */
+/*     $OpenBSD: nfs_syscalls.c,v 1.119 2023/08/03 09:49:09 mvs Exp $  */
 /*     $NetBSD: nfs_syscalls.c,v 1.19 1996/02/18 11:53:52 fvdl Exp $   */
 
 /*
@@ -249,8 +249,8 @@ nfssvc_addsock(struct file *fp, struct mbuf *mynam)
                siz = NFS_MAXPACKET;
        solock(so);
        error = soreserve(so, siz, siz); 
+       sounlock(so);
        if (error) {
-               sounlock(so);
                m_freem(mynam);
                return (error);
        }
@@ -275,6 +275,7 @@ nfssvc_addsock(struct file *fp, struct mbuf *mynam)
                sosetopt(so, IPPROTO_TCP, TCP_NODELAY, m);
                m_freem(m);
        }
+       solock(so);
        so->so_rcv.sb_flags &= ~SB_NOINTR;
        so->so_rcv.sb_timeo_nsecs = INFSLP;
        so->so_snd.sb_flags &= ~SB_NOINTR;