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

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'.

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 *);