Make `unp_msgcount' and `unp_file' protection with `unp_gc_lock'

rwlock(9).

This save us from from races provided by unlocked access to the `f_count'
which cause false marking alive socket as dead. We always modify `f_count'
and `unp_msgcount' together so the `f_count' modification should also pass
the `unp_gc_rwlock' before `unp_msgcount' increment and after
`unp_msgcount' decrement. The locked `unp_file' assignment avoids us from
drain unp_gc() run.

This moves unp_gc() locking back when these wariables were protected with
the same lock which was taken for all garbage collector run but uses
another lock not `unp_lock'.

ok kettenis@ bluhm@

diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 1c0a445..7cb3cc3 100644 (file)
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_usrreq.c,v 1.158 2021/11/17 22:56:19 mvs Exp $   */
+/*     $OpenBSD: uipc_usrreq.c,v 1.159 2021/12/07 01:19:47 mvs Exp $   */
 /*     $NetBSD: uipc_usrreq.c,v 1.18 1996/02/09 19:00:50 christos Exp $        */
 
 /*
@@ -960,8 +960,11 @@ restart:
 
                fp = rp->fp;
                rp++;
-               if ((unp = fptounp(fp)) != NULL)
-                       atomic_dec_long(&unp->unp_msgcount);
+               if ((unp = fptounp(fp)) != NULL) {
+                       rw_enter_write(&unp_gc_lock);
+                       unp->unp_msgcount--;
+                       rw_exit_write(&unp_gc_lock);
+               }
        }
 
        mtx_enter(&unp_rights_mtx);
@@ -1085,8 +1088,10 @@ morespace:
                rp->flags = fdp->fd_ofileflags[fd] & UF_PLEDGED;
                rp--;
                if ((unp = fptounp(fp)) != NULL) {
-                       atomic_inc_long(&unp->unp_msgcount);
+                       rw_enter_write(&unp_gc_lock);
+                       unp->unp_msgcount++;
                        unp->unp_file = fp;
+                       rw_exit_write(&unp_gc_lock);
                }
        }
        fdpunlock(fdp);
@@ -1099,8 +1104,11 @@ fail:
        for ( ; i > 0; i--) {
                rp++;
                fp = rp->fp;
-               if ((unp = fptounp(fp)) != NULL)
-                       atomic_dec_long(&unp->unp_msgcount);
+               if ((unp = fptounp(fp)) != NULL) {
+                       rw_enter_write(&unp_gc_lock);
+                       unp->unp_msgcount--;
+                       rw_exit_write(&unp_gc_lock);
+               }
                FRELE(fp, p);
        }
 
@@ -1136,8 +1144,11 @@ unp_gc(void *arg __unused)
                        fp = defer->ud_fp[i].fp;
                        if (fp == NULL)
                                continue;
-                       if ((unp = fptounp(fp)) != NULL)
-                               atomic_dec_long(&unp->unp_msgcount);
+                       if ((unp = fptounp(fp)) != NULL) {
+                               rw_enter_write(&unp_gc_lock);
+                               unp->unp_msgcount--;
+                               rw_exit_write(&unp_gc_lock);
+                       }
                        mtx_enter(&unp_rights_mtx);
                        unp_rights--;
                        mtx_leave(&unp_rights_mtx);

diff --git a/sys/sys/unpcb.h b/sys/sys/unpcb.h
index 7111ea9..5def517 100644 (file)
--- a/sys/sys/unpcb.h
+++ b/sys/sys/unpcb.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: unpcb.h,v 1.20 2021/11/16 08:56:20 mvs Exp $  */
+/*     $OpenBSD: unpcb.h,v 1.21 2021/12/07 01:19:47 mvs Exp $  */
 /*     $NetBSD: unpcb.h,v 1.6 1994/06/29 06:46:08 cgd Exp $    */
 
 /*
@@ -61,20 +61,19 @@
  *      I       immutable after creation
  *      G       unp_gc_lock
  *      U       unp_lock
- *      a       atomic
  */
 
 
 struct unpcb {
        struct  socket *unp_socket;     /* [I] pointer back to socket */
        struct  vnode *unp_vnode;       /* [U] if associated with file */
-       struct  file *unp_file;         /* [a] backpointer for unp_gc() */
+       struct  file *unp_file;         /* [G] backpointer for unp_gc() */
        struct  unpcb *unp_conn;        /* [U] control block of connected socket */
        ino_t   unp_ino;                /* [U] fake inode number */
        SLIST_HEAD(,unpcb) unp_refs;    /* [U] referencing socket linked list */
        SLIST_ENTRY(unpcb) unp_nextref; /* [U] link in unp_refs list */
        struct  mbuf *unp_addr;         /* [U] bound address of socket */
-       long    unp_msgcount;           /* [a] references from socket rcv buf */
+       long    unp_msgcount;           /* [G] references from socket rcv buf */
        int     unp_flags;              /* [U] this unpcb contains peer eids */
        int     unp_gcflags;            /* [G] garbge collector flags */
        struct  sockpeercred unp_connid;/* [U] id of peer process */