From 48698ead6ff0640098e6aecdd5cbf6ea8f5ac177 Mon Sep 17 00:00:00 2001 From: Gleb Smirnoff Date: Fri, 23 Feb 2024 17:56:46 -0800 Subject: [PATCH] lagg: wrap lagg_port2req() into LAGG_SLOCK() Although a port addition is coded in a sequence where first all softc information is fulfilled and only then it is attached to the lagg, we still need a locking primitive to guarantee cache invalidation. Panic observed in the wild shows that lacp_portreq() called via lagg_port_ioctl(SIOCGLAGGPORT) immediately after port creation may see lp->lp_psc as NULL and panic. In the core file we will see valid data all around. A race via lagg_ioctl() wasn't observed but potentially is possible. Differential Revision: https://reviews.freebsd.org/D43501 --- sys/net/if_lagg.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sys/net/if_lagg.c b/sys/net/if_lagg.c index 9746d4115bc6..554d8c2c1bcb 100644 --- a/sys/net/if_lagg.c +++ b/sys/net/if_lagg.c @@ -83,8 +83,10 @@ MODULE_DEPEND(if_lagg, netmap, 1, 1, 1); #define LAGG_SX_DESTROY(_sc) sx_destroy(&(_sc)->sc_sx) #define LAGG_XLOCK(_sc) sx_xlock(&(_sc)->sc_sx) #define LAGG_XUNLOCK(_sc) sx_xunlock(&(_sc)->sc_sx) -#define LAGG_SXLOCK_ASSERT(_sc) sx_assert(&(_sc)->sc_sx, SA_LOCKED) #define LAGG_XLOCK_ASSERT(_sc) sx_assert(&(_sc)->sc_sx, SA_XLOCKED) +#define LAGG_SLOCK(_sc) sx_slock(&(_sc)->sc_sx) +#define LAGG_SUNLOCK(_sc) sx_sunlock(&(_sc)->sc_sx) +#define LAGG_SXLOCK_ASSERT(_sc) sx_assert(&(_sc)->sc_sx, SA_LOCKED) /* Special flags we should propagate to the lagg ports. */ static struct { @@ -1044,7 +1046,9 @@ lagg_port_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) break; } + LAGG_SLOCK(sc); lagg_port2req(lp, rp); + LAGG_SUNLOCK(sc); NET_EPOCH_EXIT(et); break; @@ -1581,7 +1585,9 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) break; } + LAGG_SLOCK(sc); lagg_port2req(lp, rp); + LAGG_SUNLOCK(sc); NET_EPOCH_EXIT(et); if_rele(tpif); break;