Diff below changes the type of `so_state' and `so_error' to int.  It
also introduces two new wrappers to set & clear state bits atomically.

My goal is to prevent a second CPU from reading garbage when looking
at these two fields in filt_soread() and filt_sowrite().  The values
might be outdated though.

This should be the last requirement before unlocking the receiving path.

ok?

Index: kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.213
diff -u -p -r1.213 uipc_socket.c
--- kern/uipc_socket.c  2 Jan 2018 12:54:07 -0000       1.213
+++ kern/uipc_socket.c  8 Jan 2018 14:32:33 -0000
@@ -139,7 +139,7 @@ socreate(int dom, struct socket **aso, i
        s = solock(so);
        error = (*prp->pr_attach)(so, proto);
        if (error) {
-               so->so_state |= SS_NOFDREF;
+               sosetstate(so, SS_NOFDREF);
                sofree(so);
                sounlock(s);
                return (error);
@@ -275,7 +275,7 @@ drop:
 discard:
        if (so->so_state & SS_NOFDREF)
                panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
-       so->so_state |= SS_NOFDREF;
+       sosetstate(so, SS_NOFDREF);
        sofree(so);
        sounlock(s);
        return (error);
@@ -299,7 +299,7 @@ soaccept(struct socket *so, struct mbuf 
 
        if ((so->so_state & SS_NOFDREF) == 0)
                panic("soaccept !NOFDREF: so %p, so_type %d", so, so->so_type);
-       so->so_state &= ~SS_NOFDREF;
+       soclrstate(so, SS_NOFDREF);
        if ((so->so_state & SS_ISDISCONNECTED) == 0 ||
            (so->so_proto->pr_flags & PR_ABRTACPTDIS) == 0)
                error = (*so->so_proto->pr_usrreq)(so, PRU_ACCEPT, NULL,
@@ -425,7 +425,7 @@ sosend(struct socket *so, struct mbuf *a
 restart:
        if ((error = sblock(so, &so->so_snd, SBLOCKWAIT(flags))) != 0)
                goto out;
-       so->so_state |= SS_ISSENDING;
+       sosetstate(so, SS_ISSENDING);
        do {
                if (so->so_state & SS_CANTSENDMORE)
                        snderr(EPIPE);
@@ -455,7 +455,7 @@ restart:
                                snderr(EWOULDBLOCK);
                        sbunlock(so, &so->so_snd);
                        error = sbwait(so, &so->so_snd);
-                       so->so_state &= ~SS_ISSENDING;
+                       soclrstate(so, SS_ISSENDING);
                        if (error)
                                goto out;
                        goto restart;
@@ -481,7 +481,7 @@ restart:
                                        top->m_flags |= M_EOR;
                        }
                        if (resid == 0)
-                               so->so_state &= ~SS_ISSENDING;
+                               soclrstate(so, SS_ISSENDING);
                        if (top && so->so_options & SO_ZEROIZE)
                                top->m_flags |= M_ZEROIZE;
                        error = (*so->so_proto->pr_usrreq)(so,
@@ -496,7 +496,7 @@ restart:
        } while (resid);
 
 release:
-       so->so_state &= ~SS_ISSENDING;
+       soclrstate(so, SS_ISSENDING);
        sbunlock(so, &so->so_snd);
 out:
        sounlock(s);
@@ -857,7 +857,7 @@ dontblock:
                        panic("receive 3: so %p, so_type %d, m %p, m_type %d",
                            so, so->so_type, m, m->m_type);
 #endif
-               so->so_state &= ~SS_RCVATMARK;
+               soclrstate(so, SS_RCVATMARK);
                len = uio->uio_resid;
                if (so->so_oobmark && len > so->so_oobmark - offset)
                        len = so->so_oobmark - offset;
@@ -932,7 +932,7 @@ dontblock:
                        if ((flags & MSG_PEEK) == 0) {
                                so->so_oobmark -= len;
                                if (so->so_oobmark == 0) {
-                                       so->so_state |= SS_RCVATMARK;
+                                       sosetstate(so, SS_RCVATMARK);
                                        break;
                                }
                        } else {
@@ -1292,7 +1292,7 @@ somove(struct socket *so, int wait)
                        goto release;
                len = space;
        }
-       sosp->so_state |= SS_ISSENDING;
+       sosetstate(sosp, SS_ISSENDING);
 
        SBLASTRECORDCHK(&so->so_rcv, "somove 1");
        SBLASTMBUFCHK(&so->so_rcv, "somove 1");
@@ -1422,12 +1422,12 @@ somove(struct socket *so, int wait)
 
        /* Receive buffer did shrink by len bytes, adjust oob. */
        state = so->so_state;
-       so->so_state &= ~SS_RCVATMARK;
+       soclrstate(so, SS_RCVATMARK);
        oobmark = so->so_oobmark;
        so->so_oobmark = oobmark > len ? oobmark - len : 0;
        if (oobmark) {
                if (oobmark == len)
-                       so->so_state |= SS_RCVATMARK;
+                       sosetstate(so, SS_RCVATMARK);
                if (oobmark >= len)
                        oobmark = 0;
        }
@@ -1485,7 +1485,7 @@ somove(struct socket *so, int wait)
 
        /* Append all remaining data to drain socket. */
        if (so->so_rcv.sb_cc == 0 || maxreached)
-               sosp->so_state &= ~SS_ISSENDING;
+               soclrstate(sosp, SS_ISSENDING);
        error = (*sosp->so_proto->pr_usrreq)(sosp, PRU_SEND, m, NULL, NULL,
            NULL);
        if (error) {
@@ -1500,7 +1500,7 @@ somove(struct socket *so, int wait)
                goto nextpkt;
 
  release:
-       sosp->so_state &= ~SS_ISSENDING;
+       soclrstate(sosp, SS_ISSENDING);
        if (!error && maxreached && so->so_splicemax == so->so_splicelen)
                error = EFBIG;
        if (error)
Index: kern/sys_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.35
diff -u -p -r1.35 sys_socket.c
--- kern/sys_socket.c   10 Dec 2017 11:31:54 -0000      1.35
+++ kern/sys_socket.c   8 Jan 2018 14:25:21 -0000
@@ -80,20 +80,20 @@ soo_ioctl(struct file *fp, u_long cmd, c
        case FIONBIO:
                s = solock(so);
                if (*(int *)data)
-                       so->so_state |= SS_NBIO;
+                       sosetstate(so, SS_NBIO);
                else
-                       so->so_state &= ~SS_NBIO;
+                       soclrstate(so, SS_NBIO);
                sounlock(s);
                break;
 
        case FIOASYNC:
                s = solock(so);
                if (*(int *)data) {
-                       so->so_state |= SS_ASYNC;
+                       sosetstate(so, SS_ASYNC);
                        so->so_rcv.sb_flags |= SB_ASYNC;
                        so->so_snd.sb_flags |= SB_ASYNC;
                } else {
-                       so->so_state &= ~SS_ASYNC;
+                       soclrstate(so, SS_ASYNC);
                        so->so_rcv.sb_flags &= ~SB_ASYNC;
                        so->so_snd.sb_flags &= ~SB_ASYNC;
                }
Index: kern/uipc_usrreq.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.123
diff -u -p -r1.123 uipc_usrreq.c
--- kern/uipc_usrreq.c  4 Jan 2018 10:45:30 -0000       1.123
+++ kern/uipc_usrreq.c  8 Jan 2018 14:30:45 -0000
@@ -575,7 +575,7 @@ unp_disconnect(struct unpcb *unp)
 
        case SOCK_DGRAM:
                SLIST_REMOVE(&unp2->unp_refs, unp, unpcb, unp_nextref);
-               unp->unp_socket->so_state &= ~SS_ISCONNECTED;
+               soclrstate(unp->unp_socket, SS_ISCONNECTED);
                break;
 
        case SOCK_STREAM:
Index: kern/uipc_socket2.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.89
diff -u -p -r1.89 uipc_socket2.c
--- kern/uipc_socket2.c 30 Dec 2017 20:47:00 -0000      1.89
+++ kern/uipc_socket2.c 8 Jan 2018 14:30:01 -0000
@@ -87,8 +87,8 @@ void
 soisconnecting(struct socket *so)
 {
        soassertlocked(so);
-       so->so_state &= ~(SS_ISCONNECTED|SS_ISDISCONNECTING);
-       so->so_state |= SS_ISCONNECTING;
+       soclrstate(so, SS_ISCONNECTED|SS_ISDISCONNECTING);
+       sosetstate(so, SS_ISCONNECTING);
 }
 
 void
@@ -97,8 +97,8 @@ soisconnected(struct socket *so)
        struct socket *head = so->so_head;
 
        soassertlocked(so);
-       so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING);
-       so->so_state |= SS_ISCONNECTED;
+       soclrstate(so, SS_ISCONNECTING|SS_ISDISCONNECTING);
+       sosetstate(so, SS_ISCONNECTED);
        if (head && soqremque(so, 0)) {
                soqinsque(head, so, 1);
                sorwakeup(head);
@@ -114,8 +114,8 @@ void
 soisdisconnecting(struct socket *so)
 {
        soassertlocked(so);
-       so->so_state &= ~SS_ISCONNECTING;
-       so->so_state |= (SS_ISDISCONNECTING|SS_CANTRCVMORE|SS_CANTSENDMORE);
+       soclrstate(so, SS_ISCONNECTING);
+       sosetstate(so, SS_ISDISCONNECTING|SS_CANTRCVMORE|SS_CANTSENDMORE);
        wakeup(&so->so_timeo);
        sowwakeup(so);
        sorwakeup(so);
@@ -125,8 +125,8 @@ void
 soisdisconnected(struct socket *so)
 {
        soassertlocked(so);
-       so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
-       so->so_state |= (SS_CANTRCVMORE|SS_CANTSENDMORE|SS_ISDISCONNECTED);
+       soclrstate(so, SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
+       sosetstate(so, SS_CANTRCVMORE|SS_CANTSENDMORE|SS_ISDISCONNECTED);
        wakeup(&so->so_timeo);
        sowwakeup(so);
        sorwakeup(so);
@@ -198,7 +198,7 @@ sonewconn(struct socket *head, int conns
        if (connstatus) {
                sorwakeup(head);
                wakeup(&head->so_timeo);
-               so->so_state |= connstatus;
+               sosetstate(so, connstatus);
        }
        return (so);
 }
@@ -260,7 +260,7 @@ void
 socantsendmore(struct socket *so)
 {
        soassertlocked(so);
-       so->so_state |= SS_CANTSENDMORE;
+       sosetstate(so, SS_CANTSENDMORE);
        sowwakeup(so);
 }
 
@@ -268,7 +268,7 @@ void
 socantrcvmore(struct socket *so)
 {
        soassertlocked(so);
-       so->so_state |= SS_CANTRCVMORE;
+       sosetstate(so, SS_CANTRCVMORE);
        sorwakeup(so);
 }
 
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.161
diff -u -p -r1.161 uipc_syscalls.c
--- kern/uipc_syscalls.c        2 Jan 2018 06:38:45 -0000       1.161
+++ kern/uipc_syscalls.c        8 Jan 2018 14:29:48 -0000
@@ -112,8 +112,8 @@ sys_socket(struct proc *p, void *v, regi
                fdpunlock(fdp);
        } else {
                if (type & SOCK_NONBLOCK)
-                       so->so_state |= SS_NBIO;
-               so->so_state |= ss;
+                       sosetstate(so, SS_NBIO);
+               sosetstate(so, ss);
                fp->f_data = so;
                FILE_SET_MATURE(fp, p);
                *retval = fd;
@@ -340,9 +340,9 @@ doaccept(struct proc *p, int sock, struc
                error = copyaddrout(p, nam, name, namelen, anamelen);
        if (!error) {
                if (nflag & FNONBLOCK)
-                       so->so_state |= SS_NBIO;
+                       sosetstate(so, SS_NBIO);
                else
-                       so->so_state &= ~SS_NBIO;
+                       soclrstate(so, SS_NBIO);
                fp->f_data = so;
                FILE_SET_MATURE(fp, p);
                *retval = tmpfd;
@@ -424,7 +424,7 @@ sys_connect(struct proc *p, void *v, reg
        }
 bad:
        if (!interrupted)
-               so->so_state &= ~SS_ISCONNECTING;
+               soclrstate(so, SS_ISCONNECTING);
 out:
        sounlock(s);
        FRELE(fp, p);
Index: miscfs/fifofs/fifo_vnops.c
===================================================================
RCS file: /cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.62
diff -u -p -r1.62 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c  2 Jan 2018 06:38:45 -0000       1.62
+++ miscfs/fifofs/fifo_vnops.c  8 Jan 2018 14:31:49 -0000
@@ -158,7 +158,7 @@ fifo_open(void *v)
                        return (error);
                }
                fip->fi_readers = fip->fi_writers = 0;
-               wso->so_state |= SS_CANTSENDMORE;
+               sosetstate(wso, SS_CANTSENDMORE);
                wso->so_snd.sb_lowat = PIPE_BUF;
        } else {
                rso = fip->fi_readsock;
@@ -168,7 +168,7 @@ fifo_open(void *v)
        if (ap->a_mode & FREAD) {
                fip->fi_readers++;
                if (fip->fi_readers == 1) {
-                       wso->so_state &= ~SS_CANTSENDMORE;
+                       soclrstate(wso, SS_CANTSENDMORE);
                        if (fip->fi_writers > 0)
                                wakeup(&fip->fi_writers);
                }
@@ -181,7 +181,7 @@ fifo_open(void *v)
                        goto bad;
                }
                if (fip->fi_writers == 1) {
-                       rso->so_state &= ~(SS_CANTRCVMORE|SS_ISDISCONNECTED);
+                       soclrstate(rso, (SS_CANTRCVMORE|SS_ISDISCONNECTED));
                        if (fip->fi_readers > 0)
                                wakeup(&fip->fi_readers);
                }
@@ -231,12 +231,12 @@ fifo_read(void *v)
        if (uio->uio_resid == 0)
                return (0);
        if (ap->a_ioflag & IO_NDELAY)
-               rso->so_state |= SS_NBIO;
+               sosetstate(rso, SS_NBIO);
        VOP_UNLOCK(ap->a_vp, p);
        error = soreceive(rso, NULL, uio, NULL, NULL, NULL, 0);
        vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY, p);
        if (ap->a_ioflag & IO_NDELAY) {
-               rso->so_state &= ~SS_NBIO;
+               soclrstate(rso, SS_NBIO);
                if (error == EWOULDBLOCK &&
                    ap->a_vp->v_fifoinfo->fi_writers == 0)
                        error = 0;
@@ -260,14 +260,13 @@ fifo_write(void *v)
        if (ap->a_uio->uio_rw != UIO_WRITE)
                panic("fifo_write mode");
 #endif
-       /* XXXSMP changing state w/o lock isn't safe. */
        if (ap->a_ioflag & IO_NDELAY)
-               wso->so_state |= SS_NBIO;
+               sosetstate(wso, SS_NBIO);
        VOP_UNLOCK(ap->a_vp, p);
        error = sosend(wso, NULL, ap->a_uio, NULL, NULL, 0);
        vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY, p);
        if (ap->a_ioflag & IO_NDELAY)
-               wso->so_state &= ~SS_NBIO;
+               soclrstate(wso, SS_NBIO);
        return (error);
 }
 
@@ -387,7 +386,7 @@ fifo_close(void *v)
 
                        s = solock(rso);
                        /* SS_ISDISCONNECTED will result in POLLHUP */
-                       rso->so_state |= SS_ISDISCONNECTED;
+                       sosetstate(rso, SS_ISDISCONNECTED);
                        socantrcvmore(rso);
                        sounlock(s);
                }
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.162
diff -u -p -r1.162 tcp_usrreq.c
--- netinet/tcp_usrreq.c        1 Dec 2017 10:33:33 -0000       1.162
+++ netinet/tcp_usrreq.c        8 Jan 2018 14:32:00 -0000
@@ -246,7 +246,7 @@ tcp_usrreq(struct socket *so, int req, s
                        break;
                }
 
-               so->so_state |= SS_CONNECTOUT;
+               sosetstate(so, SS_CONNECTOUT);
 
                /* Compute window scaling to request.  */
                tcp_rscale(tp, sb_max);
@@ -585,9 +585,9 @@ tcp_attach(struct socket *so, int proto)
        if (tp == NULL) {
                int nofd = so->so_state & SS_NOFDREF;   /* XXX */
 
-               so->so_state &= ~SS_NOFDREF;    /* don't free the socket yet */
+               soclrstate(so, SS_NOFDREF);     /* don't free the socket yet */
                in_pcbdetach(inp);
-               so->so_state |= nofd;
+               sosetstate(so, nofd);
                return (ENOBUFS);
        }
        tp->t_state = TCPS_CLOSED;
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.245
diff -u -p -r1.245 udp_usrreq.c
--- netinet/udp_usrreq.c        1 Dec 2017 10:33:33 -0000       1.245
+++ netinet/udp_usrreq.c        8 Jan 2018 14:28:40 -0000
@@ -1144,7 +1144,7 @@ udp_usrreq(struct socket *so, int req, s
                        inp->inp_laddr.s_addr = INADDR_ANY;
                in_pcbdisconnect(inp);
 
-               so->so_state &= ~SS_ISCONNECTED;                /* XXX */
+               soclrstate(so, SS_ISCONNECTED);         /* XXX */
                break;
 
        case PRU_SHUTDOWN:
Index: netinet/tcp_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.354
diff -u -p -r1.354 tcp_input.c
--- netinet/tcp_input.c 4 Dec 2017 13:40:34 -0000       1.354
+++ netinet/tcp_input.c 8 Jan 2018 14:28:45 -0000
@@ -1872,7 +1872,7 @@ step6:
                        so->so_oobmark = so->so_rcv.sb_cc +
                            (tp->rcv_up - tp->rcv_nxt) - 1;
                        if (so->so_oobmark == 0)
-                               so->so_state |= SS_RCVATMARK;
+                               sosetstate(so, SS_RCVATMARK);
                        sohasoutofband(so);
                        tp->t_oobflags &= ~(TCPOOB_HAVEDATA | TCPOOB_HADDATA);
                }
Index: netinet6/raw_ip6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/raw_ip6.c,v
retrieving revision 1.125
diff -u -p -r1.125 raw_ip6.c
--- netinet6/raw_ip6.c  4 Dec 2017 13:40:35 -0000       1.125
+++ netinet6/raw_ip6.c  8 Jan 2018 14:28:50 -0000
@@ -560,7 +560,7 @@ rip6_usrreq(struct socket *so, int req, 
                        break;
                }
                in6p->inp_faddr6 = in6addr_any;
-               so->so_state &= ~SS_ISCONNECTED;        /* XXX */
+               soclrstate(so, SS_ISCONNECTED); /* XXX */
                break;
 
        case PRU_ABORT:
Index: nfs/nfs_socket.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_socket.c,v
retrieving revision 1.128
diff -u -p -r1.128 nfs_socket.c
--- nfs/nfs_socket.c    7 Sep 2017 11:35:34 -0000       1.128
+++ nfs/nfs_socket.c    8 Jan 2018 14:28:55 -0000
@@ -319,7 +319,7 @@ nfs_connect(struct nfsmount *nmp, struct
                        if ((so->so_state & SS_ISCONNECTING) &&
                            so->so_error == 0 && rep &&
                            (error = nfs_sigintr(nmp, rep, rep->r_procp)) != 0){
-                               so->so_state &= ~SS_ISCONNECTING;
+                               soclrstate(so, SS_ISCONNECTING);
                                goto bad;
                        }
                }
Index: sys/socketvar.h
===================================================================
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.81
diff -u -p -r1.81 socketvar.h
--- sys/socketvar.h     2 Jan 2018 12:54:07 -0000       1.81
+++ sys/socketvar.h     8 Jan 2018 14:20:40 -0000
@@ -51,12 +51,12 @@ TAILQ_HEAD(soqhead, socket);
  * private data and error information.
  */
 struct socket {
+       const struct protosw *so_proto; /* protocol handle */
+       void    *so_pcb;                /* protocol control block */
+       u_int   so_state;               /* internal state flags SS_*, below */
        short   so_type;                /* generic type, see socket.h */
        short   so_options;             /* from socket call, see socket.h */
        short   so_linger;              /* time to linger while closing */
-       short   so_state;               /* internal state flags SS_*, below */
-       void    *so_pcb;                /* protocol control block */
-       const struct protosw *so_proto; /* protocol handle */
 /*
  * Variables for connection queueing.
  * Socket where accepts occur is so_head in all subsidiary sockets.
@@ -77,7 +77,7 @@ struct socket {
        short   so_qlen;                /* number of connections on so_q */
        short   so_qlimit;              /* max number queued connections */
        short   so_timeo;               /* connection timeout */
-       u_short so_error;               /* error affecting connection */
+       u_int   so_error;               /* error affecting connection */
        pid_t   so_pgid;                /* pgid for signals */
        uid_t   so_siguid;              /* uid of process who set so_pgid */
        uid_t   so_sigeuid;             /* euid of process who set so_pgid */
@@ -156,6 +156,7 @@ struct socket {
 
 #ifdef _KERNEL
 
+#include <sys/atomic.h>
 #include <lib/libkern/libkern.h>
 
 void   soassertlocked(struct socket *);
@@ -167,6 +168,18 @@ void       soassertlocked(struct socket *);
 #define isspliced(so)          ((so)->so_sp && (so)->so_sp->ssp_socket)
 #define issplicedback(so)      ((so)->so_sp && (so)->so_sp->ssp_soback)
 
+static inline void
+sosetstate(struct socket *so, unsigned int state)
+{
+       atomic_setbits_int(&so->so_state, state);
+}
+
+static inline void
+soclrstate(struct socket *so, unsigned int state)
+{
+       atomic_clearbits_int(&so->so_state, state);
+}
+
 /*
  * Do we need to notify the other side when I/O is possible?
  */
@@ -190,10 +203,6 @@ static inline long
 sbspace(struct socket *so, struct sockbuf *sb)
 {
        KASSERT(sb == &so->so_rcv || sb == &so->so_snd);
-#if 0
-       /* XXXSMP kqueue_scan() calling filt_sowrite() cannot sleep. */
-       soassertlocked(so);
-#endif
        return lmin(sb->sb_hiwat - sb->sb_cc, sb->sb_mbmax - sb->sb_mbcnt);
 }
 

Reply via email to