From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: unp_detach(): unlock socket only if vput() required
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 25 Jun 2024 12:42:41 +0200

On Thu, Jun 20, 2024 at 09:22:37PM +0300, Vitaliy Makkoveev wrote:
> At this time `unp_gc_lock' rwlock(9) could be taken after solock() as
> it is done in uipc_attach(). The only reason to unlock dying `so' is
> lock order with vnode(9) lock `i_lock' required to release `unp_vnode'.
> So push re-lock to vnode(9) release path instead doing this every time.

passed regress, OK bluhm@

> Index: sys/kern/uipc_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> diff -u -p -r1.206 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c	3 May 2024 17:43:09 -0000	1.206
> +++ sys/kern/uipc_usrreq.c	20 Jun 2024 18:12:44 -0000
> @@ -761,25 +761,21 @@ unp_detach(struct unpcb *unp)
>  
>  	unp->unp_vnode = NULL;
>  
> -	/*
> -	 * Enforce `i_lock' -> `solock()' lock order.
> -	 */
> -	sounlock(so);
> -
>  	rw_enter_write(&unp_gc_lock);
>  	LIST_REMOVE(unp, unp_link);
>  	rw_exit_write(&unp_gc_lock);
>  
>  	if (vp != NULL) {
> +		/* Enforce `i_lock' -> solock() lock order. */
> +		sounlock(so);
>  		VOP_LOCK(vp, LK_EXCLUSIVE);
>  		vp->v_socket = NULL;
>  
>  		KERNEL_LOCK();
>  		vput(vp);
>  		KERNEL_UNLOCK();
> +		solock(so);
>  	}
> -
> -	solock(so);
>  
>  	if (unp->unp_conn != NULL) {
>  		/*