From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: fix race in in_pcbsolock_ref()
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 5 Feb 2025 01:29:42 +0300

> On 5 Feb 2025, at 01:05, Alexander Bluhm <bluhm@openbsd.org> wrote:
> 
> Hi,
> 
> Further testing of parallel TCP input revealed this race in
> in_pcbsolock_ref().
> 
> ok?
> 

Do we still need in_pcbsounlock_rele()? We have tcp_sogetpcb() to
get inp and tp from socket while performing tcp_usrreqs handlers.
Is it reasonable to transform in_pcbsolock_ref() to something like
it? I’m asking because this diff implements similar logic.

> bluhm
> 
> Index: netinet/in_pcb.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
> diff -u -p -r1.310 in_pcb.c
> --- netinet/in_pcb.c	9 Jan 2025 16:47:24 -0000	1.310
> +++ netinet/in_pcb.c	4 Feb 2025 20:55:32 -0000
> @@ -635,6 +635,13 @@ in_pcbsolock_ref(struct inpcb *inp)
> 	if (so == NULL)
> 		return NULL;
> 	rw_enter_write(&so->so_lock);
> +	/* between mutex and rwlock inpcb could be detached */
> +	if (so->so_pcb == NULL) {
> +		rw_exit_write(&so->so_lock);
> +		sorele(so);
> +		return NULL;
> +	}
> +	KASSERT(inp->inp_socket == so && sotoinpcb(so) == inp);
> 	return so;
> }
> 
> @@ -643,7 +650,6 @@ in_pcbsounlock_rele(struct inpcb *inp, s
> {
> 	if (so == NULL)
> 		return;
> -	KASSERT(inp->inp_socket == NULL || inp->inp_socket == so);
> 	rw_exit_write(&so->so_lock);
> 	sorele(so);
> }
>