From: mvs <mvs@openbsd.org>
Date: Thu, 14 Jul 2022 11:03:15 +0000 (+0000)
Subject: Turn pppoe(4) back to kernel lock. We can't predict netlock state within
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ff335594b74dd061d0e7cd1abfb0c5c256a082f7;p=openbsd

Turn pppoe(4) back to kernel lock. We can't predict netlock state within
pppoe_start(), so we can't use it for pppoe(4) data protection. Except
input path, pppoe(4) always accessed with kernel lock held, so grab it
around pppoeintr() too.

Interfaces should not use netlock for their data protection. They should
rely on kernel lock or implement their own.

ok bluhm@ bket@
---

diff --git a/sys/net/if.c b/sys/net/if.c
index d54c7e6041b..17f2265aab5 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.658 2022/07/10 21:26:55 mvs Exp $	*/
+/*	$OpenBSD: if.c,v 1.659 2022/07/14 11:03:15 mvs Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -920,8 +920,11 @@ if_netisr(void *unused)
 			pipexintr();
 #endif
 #if NPPPOE > 0
-		if (n & (1 << NETISR_PPPOE))
+		if (n & (1 << NETISR_PPPOE)) {
+			KERNEL_LOCK();
 			pppoeintr();
+			KERNEL_UNLOCK();
+		}
 #endif
 		t |= n;
 	}
diff --git a/sys/net/if_pppoe.c b/sys/net/if_pppoe.c
index c8c9d49e9c0..7b5d260ea95 100644
--- a/sys/net/if_pppoe.c
+++ b/sys/net/if_pppoe.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_pppoe.c,v 1.82 2022/07/09 20:57:01 mvs Exp $ */
+/* $OpenBSD: if_pppoe.c,v 1.83 2022/07/14 11:03:15 mvs Exp $ */
 /* $NetBSD: if_pppoe.c,v 1.51 2003/11/28 08:56:48 keihan Exp $ */
 
 /*
@@ -117,30 +117,30 @@ struct pppoetag {
 /*
  * Locks used to protect struct members and global data
  *       I       immutable after creation
- *       N       net lock
+ *       K       kernel lock
  */
 
 struct pppoe_softc {
 	struct sppp sc_sppp;		/* contains a struct ifnet as first element */
-	LIST_ENTRY(pppoe_softc) sc_list;/* [N] */
-	unsigned int sc_eth_ifidx;	/* [N] */
-
-	int sc_state;			/* [N] discovery phase or session connected */
-	struct ether_addr sc_dest;	/* [N] hardware address of concentrator */
-	u_int16_t sc_session;		/* [N] PPPoE session id */
-
-	char *sc_service_name;		/* [N] if != NULL: requested name of service */
-	char *sc_concentrator_name;	/* [N] if != NULL: requested concentrator id */
-	u_int8_t *sc_ac_cookie;		/* [N] content of AC cookie we must echo back */
-	size_t sc_ac_cookie_len;	/* [N] length of cookie data */
-	u_int8_t *sc_relay_sid;		/* [N] content of relay SID we must echo back */
-	size_t sc_relay_sid_len;	/* [N] length of relay SID data */
+	LIST_ENTRY(pppoe_softc) sc_list;/* [K] */
+	unsigned int sc_eth_ifidx;	/* [K] */
+
+	int sc_state;			/* [K] discovery phase or session connected */
+	struct ether_addr sc_dest;	/* [K] hardware address of concentrator */
+	u_int16_t sc_session;		/* [K] PPPoE session id */
+
+	char *sc_service_name;		/* [K] if != NULL: requested name of service */
+	char *sc_concentrator_name;	/* [K] if != NULL: requested concentrator id */
+	u_int8_t *sc_ac_cookie;		/* [K] content of AC cookie we must echo back */
+	size_t sc_ac_cookie_len;	/* [K] length of cookie data */
+	u_int8_t *sc_relay_sid;		/* [K] content of relay SID we must echo back */
+	size_t sc_relay_sid_len;	/* [K] length of relay SID data */
 	u_int32_t sc_unique;		/* [I] our unique id */
-	struct timeout sc_timeout;	/* [N] timeout while not in session state */
-	int sc_padi_retried;		/* [N] number of PADI retries already done */
-	int sc_padr_retried;		/* [N] number of PADR retries already done */
+	struct timeout sc_timeout;	/* [K] timeout while not in session state */
+	int sc_padi_retried;		/* [K] number of PADI retries already done */
+	int sc_padr_retried;		/* [K] number of PADR retries already done */
 
-	struct timeval sc_session_time;	/* [N] time the session was established */
+	struct timeval sc_session_time;	/* [K] time the session was established */
 };
 
 /* input routines */