From: Vitaliy Makkoveev <otto@bsdbox.dev>
Subject: Re: socket locking in soisconnected()
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Sat, 18 Jan 2025 01:34:52 +0300

> On 18 Jan 2025, at 01:25, Alexander Bluhm <bluhm@openbsd.org> wrote:
> 
> Hi,
> 
> Queue 0 is only used for UNIX socket and socket pair.  So the
> following code is never reached with TCP.  Replacing "if" with
> "assert" makes arguing about TCP input socket locking easier.
> 
> ok?
> 

sure

> bluhm
> 
> Index: kern/uipc_socket2.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket2.c,v
> diff -u -p -r1.165 uipc_socket2.c
> --- kern/uipc_socket2.c	16 Jan 2025 16:35:01 -0000	1.165
> +++ kern/uipc_socket2.c	16 Jan 2025 21:01:22 -0000
> @@ -100,21 +100,17 @@ soisconnected(struct socket *so)
> 	so->so_state |= SS_ISCONNECTED;
> 
> 	if (head != NULL && so->so_onq == &head->so_q0) {
> -		int persocket = solock_persocket(so);
> +		KASSERT(solock_persocket(so));
> 
> -		if (persocket) {
> -			soref(head);
> -
> -			sounlock(so);
> -			solock(head);
> -			solock(so);
> -
> -			if (so->so_onq != &head->so_q0) {
> -				sounlock(head);
> -				sorele(head);
> -				return;
> -			}
> +		soref(head);
> +		sounlock(so);
> +		solock(head);
> +		solock(so);
> 
> +		if (so->so_onq != &head->so_q0) {
> +			sounlock(head);
> +			sorele(head);
> +			return;
> 		}
> 
> 		soqremque(so, 0);
> @@ -122,10 +118,8 @@ soisconnected(struct socket *so)
> 		sorwakeup(head);
> 		wakeup_one(&head->so_timeo);
> 
> -		if (persocket) {
> -			sounlock(head);
> -			sorele(head);
> -		}
> +		sounlock(head);
> +		sorele(head);
> 	} else {
> 		wakeup(&so->so_timeo);
> 		sorwakeup(so);
>