From: Alexander Bluhm <alexander.bluhm@gmx.net>
Subject: Re: Use `sb_mtx' instead of `inp_mtx' in receive path for inet sockets
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 12 Feb 2024 21:49:19 +0100

On Mon, Feb 12, 2024 at 09:35:37PM +0300, Vitaliy Makkoveev wrote:
> On Mon, Feb 12, 2024 at 07:14:20PM +0300, Vitaliy Makkoveev wrote:
> > On Mon, Feb 12, 2024 at 03:57:20PM +0100, Alexander Bluhm wrote:
> > > On Mon, Feb 12, 2024 at 03:25:14AM +0300, Vitaliy Makkoveev wrote:
> > > > This time only sbappendaddr() requires to hold sb_mtx for some
> > > > cases, so it uses sbmtxassertlocked() instead of soassertlocked().
> > > 
> > > I looks like regress already found something.  This was a test with
> > > your diff applied.
> > > 
> > > [-- MARK -- Sun Feb 11 21:30:00 2024]
> > > splassert: soassertlocked: want 0 have 1
> > > Starting stack trace...
> > > soassertlocked(0) at soassertlocked+0xc7
> > > soassertlocked(d7e26a68) at soassertlocked+0xc7
> > > sbappendaddr(d7e26a68,d7e26adc,f58028c4,d8c73900,0) at sbappendaddr+0x21
> > > divert_packet(d8c73900,1,9a02) at divert_packet+0x239
> > > pf_test(2,1,d835c830,f5802a70) at pf_test+0x873
> > > ip_input_if(f5802a70,f5802a5c,f5802a88,0,d835c830) at ip_input_if+0xab
> > > ipv4_input(d835c830,d8c73900) at ipv4_input+0x3a
> > > ether_input(d835c830,d8c73900) at ether_input+0x2c4
> > > if_input_process(d835c830,f5802ac4) at if_input_process+0x4d
> > > ifiq_process(d835cb00) at ifiq_process+0x66
> > > taskq_thread(d82e7000) at taskq_thread+0x84
> > > End of stack trace.
> > 
> > That's interesting. Divert sockets are raw inet sockets, so `sb_flags'
> > should have SB_MTXLOCK bit set, but it seems to be missing. Can you
> > check this please?
> > 
> 
> dp->dom_protosw->pr_type of inet sockets always points to inetsw[0].
> Pass protosw instead domain to soalloc() to get real `pr_type'. The
> corresponding domain is refrenced as `pr_domain'.

OK bluhm@

I will throw it onto regress machine.

> Index: sys/kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.319
> diff -u -p -r1.319 uipc_socket.c
> --- sys/kern/uipc_socket.c	11 Feb 2024 21:36:49 -0000	1.319
> +++ sys/kern/uipc_socket.c	12 Feb 2024 18:30:57 -0000
> @@ -132,8 +132,9 @@ soinit(void)
>  }
>  
>  struct socket *
> -soalloc(const struct domain *dp, int wait)
> +soalloc(const struct protosw *prp, int wait)
>  {
> +	const struct domain *dp = prp->pr_domain;
>  	struct socket *so;
>  
>  	so = pool_get(&socket_pool, (wait == M_WAIT ? PR_WAITOK : PR_NOWAIT) |
> @@ -153,7 +154,7 @@ soalloc(const struct domain *dp, int wai
>  	switch (dp->dom_family) {
>  	case AF_INET:
>  	case AF_INET6:
> -		switch (dp->dom_protosw->pr_type) {
> +		switch (prp->pr_type) {
>  		case SOCK_DGRAM:
>  		case SOCK_RAW:
>  			so->so_rcv.sb_flags |= SB_MTXLOCK;
> @@ -188,7 +189,7 @@ socreate(int dom, struct socket **aso, i
>  		return (EPROTONOSUPPORT);
>  	if (prp->pr_type != type)
>  		return (EPROTOTYPE);
> -	so = soalloc(pffinddomain(dom), M_WAIT);
> +	so = soalloc(prp, M_WAIT);
>  	so->so_type = type;
>  	if (suser(p) == 0)
>  		so->so_state = SS_PRIV;
> Index: sys/kern/uipc_socket2.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.143
> diff -u -p -r1.143 uipc_socket2.c
> --- sys/kern/uipc_socket2.c	11 Feb 2024 18:14:26 -0000	1.143
> +++ sys/kern/uipc_socket2.c	12 Feb 2024 18:30:57 -0000
> @@ -188,7 +188,7 @@ sonewconn(struct socket *head, int conns
>  		return (NULL);
>  	if (head->so_qlen + head->so_q0len > head->so_qlimit * 3)
>  		return (NULL);
> -	so = soalloc(head->so_proto->pr_domain, wait);
> +	so = soalloc(head->so_proto, wait);
>  	if (so == NULL)
>  		return (NULL);
>  	so->so_type = head->so_type;
> Index: sys/sys/socketvar.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.123
> diff -u -p -r1.123 socketvar.h
> --- sys/sys/socketvar.h	11 Feb 2024 18:14:27 -0000	1.123
> +++ sys/sys/socketvar.h	12 Feb 2024 18:30:57 -0000
> @@ -375,7 +375,7 @@ int	soconnect(struct socket *, struct mb
>  int	soconnect2(struct socket *, struct socket *);
>  int	socreate(int, struct socket **, int, int);
>  int	sodisconnect(struct socket *);
> -struct socket *soalloc(const struct domain *, int);
> +struct socket *soalloc(const struct protosw *, int);
>  void	sofree(struct socket *, int);
>  int	sogetopt(struct socket *, int, int, struct mbuf *);
>  void	sohasoutofband(struct socket *);