CVS commit: src/sys/net/lagg

[Shoichi YAMAGUCHI]

Module Name:    src
Committed By:   yamaguchi
Date:           Thu Sep 30 04:20:14 UTC 2021

Modified Files:
        src/sys/net/lagg: if_lagg.c

Log Message:
Fix to acquire LAGG_LOCK without psref
to remove possibility of deadlock

the deadlock maybe happened between lagg_ifdetach()
and lagg_delport()

1. lagg_ifdetach calls psref_target_acquire()
2. lagg_delport calls LAGG_LOCK()
3. lagg_ifdetach calls LAGG_LOCK()
   - wait for lagg_delport
4. lagg_delport calls psref_target_destroy()
   - wait for lagg_ifdetach


To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/sys/net/lagg/if_lagg.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/net/lagg/if_lagg.c
diff -u src/sys/net/lagg/if_lagg.c:1.7 src/sys/net/lagg/if_lagg.c:1.8
--- src/sys/net/lagg/if_lagg.c:1.7	Thu Sep 30 03:39:39 2021
+++ src/sys/net/lagg/if_lagg.c	Thu Sep 30 04:20:14 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_lagg.c,v 1.7 2021/09/30 03:39:39 yamaguchi Exp $	*/
+/*	$NetBSD: if_lagg.c,v 1.8 2021/09/30 04:20:14 yamaguchi Exp $	*/
 
 /*
  * Copyright (c) 2005, 2006 Reyk Floeter <r...@openbsd.org>
@@ -20,7 +20,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_lagg.c,v 1.7 2021/09/30 03:39:39 yamaguchi Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_lagg.c,v 1.8 2021/09/30 04:20:14 yamaguchi Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -2051,15 +2051,19 @@ lagg_addport(struct lagg_softc *sc, stru
 }
 
 static void
-lagg_delport_locked(struct lagg_softc *sc, struct lagg_port *lp)
+lagg_delport_locked(struct lagg_softc *sc, struct lagg_port *lp,
+    bool is_ifdetach)
 {
 	struct ifnet *ifp_port;
 	struct ifreq ifr;
 	u_long if_type;
-	bool stopped, detaching;
+	bool stopped;
 
 	KASSERT(LAGG_LOCKED(sc));
 
+	if (lp == NULL)
+		return;
+
 	ifp_port = lp->lp_ifp;
 	stopped = false;
 
@@ -2093,7 +2097,6 @@ lagg_delport_locked(struct lagg_softc *s
 	lagg_port_purgemulti(sc, lp);
 	lagg_port_purgevlan(sc, lp);
 
-	detaching = lp->lp_detaching;
 	IFNET_LOCK(ifp_port);
 	if_type = ifp_port->if_type;
 	ifp_port->if_type = lp->lp_iftype;
@@ -2111,7 +2114,7 @@ lagg_delport_locked(struct lagg_softc *s
 		ifp_port->if_init(ifp_port);
 	}
 
-	if (!detaching) {
+	if (is_ifdetach == false) {
 		IFNET_LOCK(ifp_port);
 		lagg_in6_ifattach(ifp_port);
 		IFNET_UNLOCK(ifp_port);
@@ -2125,7 +2128,7 @@ lagg_delport_all(struct lagg_softc *sc)
 
 	LAGG_LOCK(sc);
 	LAGG_PORTS_FOREACH_SAFE(sc, lp, lp0) {
-		lagg_delport_locked(sc, lp);
+		lagg_delport_locked(sc, lp, false);
 	}
 	LAGG_UNLOCK(sc);
 }
@@ -2142,7 +2145,7 @@ lagg_delport(struct lagg_softc *sc, stru
 		return ENOENT;
 
 	LAGG_LOCK(sc);
-	lagg_delport_locked(sc, lp);
+	lagg_delport_locked(sc, lp, false);
 	LAGG_UNLOCK(sc);
 
 	return 0;
@@ -2315,7 +2318,6 @@ lagg_ifdetach(struct ifnet *ifp_port)
 {
 	struct lagg_port *lp;
 	struct lagg_softc *sc;
-	struct psref psref;
 	int s;
 
 	IFNET_ASSERT_UNLOCKED(ifp_port);
@@ -2326,21 +2328,22 @@ lagg_ifdetach(struct ifnet *ifp_port)
 		pserialize_read_exit(s);
 		return;
 	}
-	lagg_port_getref(lp, &psref);
-	pserialize_read_exit(s);
 
 	sc = lp->lp_softc;
-	if (sc != NULL) {
-		IFNET_LOCK(&sc->sc_if);
-		LAGG_LOCK(sc);
-		lagg_port_putref(lp, &psref);
-	} else {
-		lagg_port_putref(lp, &psref);
+	if (sc == NULL) {
+		pserialize_read_exit(s);
 		return;
 	}
+	pserialize_read_exit(s);
 
-	lp->lp_detaching = true;
-	lagg_delport_locked(sc, lp);
+	IFNET_LOCK(&sc->sc_if);
+	LAGG_LOCK(sc);
+	/*
+	 * reload if_lagg because it may write
+	 * after pserialize_read_exit.
+	 */
+	lp = ifp_port->if_lagg;
+	lagg_delport_locked(sc, lp, true);
 	LAGG_UNLOCK(sc);
 	IFNET_UNLOCK(&sc->sc_if);
 }