From: bluhm <bluhm@openbsd.org>
Date: Mon, 9 May 2022 19:33:46 +0000 (+0000)
Subject: Protect sbappendaddr() in divert_packet() with kernel lock.  With
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ddfdd69b7d10ca2e51fb57f397dd2e190b205350;p=openbsd

Protect sbappendaddr() in divert_packet() with kernel lock.  With
divert-packet rules pf calls directly from IP layer to protocol
layer.  As the former has only shared net lock, additional protection
against parallel access is needed.  Kernel lock is a temporary
workaround until the socket layer is MP safe.
discussed with kettenis@ mvs@
---

diff --git a/sys/netinet/ip_divert.c b/sys/netinet/ip_divert.c
index 187bbe8c96c..aae4ed62bea 100644
--- a/sys/netinet/ip_divert.c
+++ b/sys/netinet/ip_divert.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: ip_divert.c,v 1.67 2022/05/05 16:44:22 bluhm Exp $ */
+/*      $OpenBSD: ip_divert.c,v 1.68 2022/05/09 19:33:46 bluhm Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -222,11 +222,19 @@ divert_packet(struct mbuf *m, int dir, u_int16_t divert_port)
 	}
 
 	so = inp->inp_socket;
+	/*
+	 * XXXSMP sbappendaddr() is not MP safe and this function is called
+	 * from pf with shared netlock.  To call only one sbappendaddr() from
+	 * divert_packet(), protect it with kernel lock.  All other places
+	 * call sbappendaddr() with exclusive net lock.  This blocks
+	 * divert_packet() as we have the shared lock.
+	 */
+	KERNEL_LOCK();
 	if (sbappendaddr(so, &so->so_rcv, sintosa(&sin), m, NULL) == 0) {
+		KERNEL_UNLOCK();
 		divstat_inc(divs_fullsock);
 		goto bad;
 	}
-	KERNEL_LOCK();
 	sorwakeup(inp->inp_socket);
 	KERNEL_UNLOCK();
 
diff --git a/sys/netinet6/ip6_divert.c b/sys/netinet6/ip6_divert.c
index 35eb40945c1..eb88105ea9a 100644
--- a/sys/netinet6/ip6_divert.c
+++ b/sys/netinet6/ip6_divert.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: ip6_divert.c,v 1.66 2022/05/05 16:44:22 bluhm Exp $ */
+/*      $OpenBSD: ip6_divert.c,v 1.67 2022/05/09 19:33:46 bluhm Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -228,11 +228,19 @@ divert6_packet(struct mbuf *m, int dir, u_int16_t divert_port)
 	}
 
 	so = inp->inp_socket;
+	/*
+	 * XXXSMP sbappendaddr() is not MP safe and this function is called
+	 * from pf with shared netlock.  To call only one sbappendaddr() from
+	 * divert_packet(), protect it with kernel lock.  All other places
+	 * call sbappendaddr() with exclusive net lock.  This blocks
+	 * divert_packet() as we have the shared lock.
+	 */
+	KERNEL_LOCK();
 	if (sbappendaddr(so, &so->so_rcv, sin6tosa(&sin6), m, NULL) == 0) {
+		KERNEL_UNLOCK();
 		div6stat_inc(div6s_fullsock);
 		goto bad;
 	}
-	KERNEL_LOCK();
 	sorwakeup(inp->inp_socket);
 	KERNEL_UNLOCK();