From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: socreate() shared net lock
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Thu, 6 Feb 2025 13:18:46 +0100

On Wed, Feb 05, 2025 at 09:48:29PM +0300, Vitaliy Makkoveev wrote:
> On Wed, Feb 05, 2025 at 06:58:07PM +0100, Alexander Bluhm wrote:
> > Hi,
> > 
> > socreate() and its internet attach functions look MP safe.  So
> > replace exclusive net lock with shared net lock plus socket lock.
> > 
> > As sofree() unlocks exclusive net lock, swap the locks in the error
> > path.  As neither userland not protocol stack know about the newly
> > created socket, this is safe.
> > 
> > ok?
> > 
> > bluhm
> > 
> 
> I have the same, but without relocking. It relies on soref()/sorele().

Either way works.  We should revert it, after soclose() has been
converted to shared net lock.

OK bluhm@

> Index: sys/kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.371
> diff -u -p -r1.371 uipc_socket.c
> --- sys/kern/uipc_socket.c	5 Feb 2025 08:28:25 -0000	1.371
> +++ sys/kern/uipc_socket.c	5 Feb 2025 18:38:08 -0000
> @@ -208,15 +208,18 @@ socreate(int dom, struct socket **aso, i
>  	so->so_snd.sb_timeo_nsecs = INFSLP;
>  	so->so_rcv.sb_timeo_nsecs = INFSLP;
>  
> -	solock(so);
> +	solock_shared(so);
>  	error = pru_attach(so, proto, M_WAIT);
>  	if (error) {
>  		so->so_state |= SS_NOFDREF;
>  		/* sofree() calls sounlock(). */
> -		sofree(so, 0);
> +		soref(so);
> +		sofree(so, 1);
> +		sounlock_shared(so);
> +		sorele(so);
>  		return (error);
>  	}
> -	sounlock(so);
> +	sounlock_shared(so);
>  	*aso = so;
>  	return (0);
>  }
> Index: sys/netinet/raw_ip.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.163
> diff -u -p -r1.163 raw_ip.c
> --- sys/netinet/raw_ip.c	1 Jan 2025 13:44:22 -0000	1.163
> +++ sys/netinet/raw_ip.c	5 Feb 2025 18:38:08 -0000
> @@ -488,7 +488,6 @@ rip_attach(struct socket *so, int proto,
>  
>  	if ((error = soreserve(so, rip_sendspace, rip_recvspace)))
>  		return error;
> -	NET_ASSERT_LOCKED();
>  	if ((error = in_pcballoc(so, &rawcbtable, wait)))
>  		return error;
>  	inp = sotoinpcb(so);
> Index: sys/netinet/tcp_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.241
> diff -u -p -r1.241 tcp_usrreq.c
> --- sys/netinet/tcp_usrreq.c	30 Jan 2025 14:40:50 -0000	1.241
> +++ sys/netinet/tcp_usrreq.c	5 Feb 2025 18:38:08 -0000
> @@ -487,7 +487,6 @@ tcp_attach(struct socket *so, int proto,
>  			return (error);
>  	}
>  
> -	NET_ASSERT_LOCKED();
>  #ifdef INET6
>  	if (so->so_proto->pr_domain->dom_family == PF_INET6)
>  		table = &tcb6table;
> Index: sys/netinet/udp_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.330
> diff -u -p -r1.330 udp_usrreq.c
> --- sys/netinet/udp_usrreq.c	25 Jan 2025 02:06:40 -0000	1.330
> +++ sys/netinet/udp_usrreq.c	5 Feb 2025 18:38:08 -0000
> @@ -1107,7 +1107,6 @@ udp_attach(struct socket *so, int proto,
>  	    atomic_load_int(&udp_recvspace))))
>  		return error;
>  
> -	NET_ASSERT_LOCKED();
>  #ifdef INET6
>  	if (so->so_proto->pr_domain->dom_family == PF_INET6)
>  		table = &udb6table;
> Index: sys/netinet6/raw_ip6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/raw_ip6.c,v
> retrieving revision 1.186
> diff -u -p -r1.186 raw_ip6.c
> --- sys/netinet6/raw_ip6.c	8 Nov 2024 10:24:13 -0000	1.186
> +++ sys/netinet6/raw_ip6.c	5 Feb 2025 18:38:08 -0000
> @@ -606,7 +606,6 @@ rip6_attach(struct socket *so, int proto
>  
>  	if ((error = soreserve(so, rip6_sendspace, rip6_recvspace)))
>  		return error;
> -	NET_ASSERT_LOCKED();
>  	if ((error = in_pcballoc(so, &rawin6pcbtable, wait)))
>  		return error;
>