Date: Fri, 24 Sep 2021 13:32:12 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 5a9ecb0b1505 - stable/13 - socket: Add assertions around naked refcount decrements Message-ID: <202109241332.18ODWCJF087053@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=5a9ecb0b1505b4830c67b586164be7593ba32bf4 commit 5a9ecb0b1505b4830c67b586164be7593ba32bf4 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-09-17 16:26:56 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2021-09-24 13:03:04 +0000 socket: Add assertions around naked refcount decrements Sockets in a listen queue hold a reference to the parent listening socket. Several code paths release this reference manually when moving a child socket out of the queue. Replace comments about the expected post-decrement refcount value with assertions. Use refcount_load() instead of a plain load. No functional change intended. Sponsored by: The FreeBSD Foundation (cherry picked from commit 6b288408ca32e68c74f6ab12324448ab4862a045) --- sys/kern/uipc_socket.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index 13482fce5980..77c23859cf33 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -1073,11 +1073,12 @@ void sofree(struct socket *so) { struct protosw *pr = so->so_proto; + bool last __diagused; SOCK_LOCK_ASSERT(so); - if ((so->so_state & SS_NOFDREF) == 0 || so->so_count != 0 || - (so->so_state & SS_PROTOREF) || (so->so_qstate == SQ_COMP)) { + if ((so->so_state & (SS_NOFDREF | SS_PROTOREF)) != SS_NOFDREF || + refcount_load(&so->so_count) != 0 || so->so_qstate == SQ_COMP) { SOCK_UNLOCK(so); return; } @@ -1113,8 +1114,9 @@ sofree(struct socket *so) __func__, so, sol)); TAILQ_REMOVE(&sol->sol_incomp, so, so_list); sol->sol_incqlen--; - /* This is guarenteed not to be the last. */ - refcount_release(&sol->so_count); + last = refcount_release(&sol->so_count); + KASSERT(!last, ("%s: released last reference for %p", + __func__, sol)); so->so_qstate = SQ_NONE; so->so_listen = NULL; } else @@ -1122,7 +1124,7 @@ sofree(struct socket *so) ("%s: so %p not on (in)comp with so_listen", __func__, so)); sorele(sol); - KASSERT(so->so_count == 1, + KASSERT(refcount_load(&so->so_count) == 1, ("%s: so %p count %u", __func__, so, so->so_count)); so->so_count = 0; } @@ -1178,6 +1180,7 @@ soclose(struct socket *so) struct accept_queue lqueue; struct socket *sp, *tsp; int error = 0; + bool last __diagused; KASSERT(!(so->so_state & SS_NOFDREF), ("soclose: SS_NOFDREF on enter")); @@ -1224,8 +1227,9 @@ drop: sp->so_qstate = SQ_NONE; sp->so_listen = NULL; SOCK_UNLOCK(sp); - /* Guaranteed not to be the last. */ - refcount_release(&so->so_count); + last = refcount_release(&so->so_count); + KASSERT(!last, ("%s: released last reference for %p", + __func__, so)); } } KASSERT((so->so_state & SS_NOFDREF) == 0, ("soclose: NOFDREF")); @@ -1237,7 +1241,7 @@ drop: SOCK_UNLOCK(sp); soabort(sp); } else { - /* sp is now in sofree() */ + /* See the handling of queued sockets in sofree(). */ SOCK_UNLOCK(sp); } } @@ -3971,6 +3975,7 @@ soisconnecting(struct socket *so) void soisconnected(struct socket *so) { + bool last __diagused; SOCK_LOCK(so); so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING|SS_ISCONFIRMING); @@ -4003,8 +4008,9 @@ soisconnected(struct socket *so) sorele(head); return; } - /* Not the last one, as so holds a ref. */ - refcount_release(&head->so_count); + last = refcount_release(&head->so_count); + KASSERT(!last, ("%s: released last reference for %p", + __func__, head)); } again: if ((so->so_options & SO_ACCEPTFILTER) == 0) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202109241332.18ODWCJF087053>