From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: TCP and UNIX soabort() locking
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 20 Jan 2025 17:29:01 +0300

On Mon, Jan 20, 2025 at 01:37:24PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> I want to make UNIX and TCP socket locking more similar.
> 
> One difference is that UNIX sockets unlock in soabort() while TCP
> does not do that.  in_pcbdetach() does keep the lock, so let's
> change uipc_abort().  This also gives symetric lock and unlock in
> the caller.  Refcount is needed to call unlock on an aborted socket.
> 
> The queue 0 in soclose() is only used by UNIX sockets, so replace
> the "if" with "kassert".
> 
> ok?
> 

The diff is ok mvs@, but do we really need this assertion?

> +		if (!TAILQ_EMPTY(&so->so_q0))
> +			KASSERT(persocket);

> bluhm
> 
> Index: kern/uipc_socket.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
> diff -u -p -r1.360 uipc_socket.c
> --- kern/uipc_socket.c	13 Jan 2025 18:10:20 -0000	1.360
> +++ kern/uipc_socket.c	18 Jan 2025 16:40:02 -0000
> @@ -399,23 +399,27 @@ drop:
>  	if (so->so_options & SO_ACCEPTCONN) {
>  		int persocket = solock_persocket(so);
>  
> +		if (!TAILQ_EMPTY(&so->so_q0))
> +			KASSERT(persocket);
>  		while ((so2 = TAILQ_FIRST(&so->so_q0)) != NULL) {
> -			if (persocket)
> -				solock(so2);
> +			soref(so2);
> +			solock(so2);
>  			(void) soqremque(so2, 0);
> -			if (persocket)
> -				sounlock(so);
> +			sounlock(so);
>  			soabort(so2);
> -			if (persocket)
> -				solock(so);
> +			sounlock(so2);
> +			sorele(so2);
> +			solock(so);
>  		}
>  		while ((so2 = TAILQ_FIRST(&so->so_q)) != NULL) {
> -			if (persocket)
> -				solock(so2);
> +			soref(so2);
> +			solock_nonet(so2);
>  			(void) soqremque(so2, 1);
>  			if (persocket)
>  				sounlock(so);
>  			soabort(so2);
> +			sounlock_nonet(so2);
> +			sorele(so2);
>  			if (persocket)
>  				solock(so);
>  		}
> Index: kern/uipc_usrreq.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_usrreq.c,v
> diff -u -p -r1.212 uipc_usrreq.c
> --- kern/uipc_usrreq.c	1 Jan 2025 13:44:22 -0000	1.212
> +++ kern/uipc_usrreq.c	18 Jan 2025 16:24:10 -0000
> @@ -656,7 +656,7 @@ uipc_abort(struct socket *so)
>  	struct unpcb *unp = sotounpcb(so);
>  
>  	unp_detach(unp);
> -	sofree(so, 0);
> +	sofree(so, 1);
>  }
>  
>  int
>