summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVitaliy Makkoveev <mvs@cvs.openbsd.org>2021-12-07 01:19:48 +0000
committerVitaliy Makkoveev <mvs@cvs.openbsd.org>2021-12-07 01:19:48 +0000
commit8555e51e3db8c3e12459dddbfa6e478b93e55bc3 (patch)
tree9cfefaf5cff034a9d52bec54f25f5962905b9d87
parentfeaea13e9b89c8f232384c72e9d8368e2486d087 (diff)
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@
-rw-r--r--sys/kern/uipc_usrreq.c27
-rw-r--r--sys/sys/unpcb.h7
2 files changed, 22 insertions, 12 deletions
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 1c0a4454458..7cb3cc39494 100644
--- 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 7111ea97f54..5def517fa08 100644
--- 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 */