Page MenuHomeFreeBSD

lagg: wrap lagg_port2req() into LAGG_SLOCK()
ClosedPublic

Authored by glebius on Jan 18 2024, 7:20 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 12, 9:17 PM
Unknown Object (File)
Thu, Sep 25, 9:00 AM
Unknown Object (File)
Sat, Sep 20, 1:22 AM
Unknown Object (File)
Wed, Sep 17, 9:56 AM
Unknown Object (File)
Sep 15 2025, 4:14 PM
Unknown Object (File)
Aug 28 2025, 6:19 AM
Unknown Object (File)
Jul 29 2025, 11:19 AM
Unknown Object (File)
Jul 28 2025, 4:48 PM
Subscribers

Details

Summary

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/net/if_lagg.c
1049

ifunit() calls CK_STAILQ_FOREACH() so I guess there should not be cache invalidation issues.

I'm getting confused.

sys/net/if_lagg.c
1049

This isn't related to ifunit() at all. The whole code path of SIOCGLAGGPORT goes lockless. The syscall enters lacp_portreq() without taking any locks. There it may see a NULL lp->lp_psc and panic. The CPU the syscall executes on did not take any measures to synchronize the the CPU that did lacp_port_create() and assigned lp_psc to a value.

glebius retitled this revision from lagg: wrap lagg_port2req() into LAGG_XLOCK() to lagg: wrap lagg_port2req() into LAGG_SLOCK().Feb 21 2024, 5:56 PM

Use a shared lock. That should be sufficient.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 24 2024, 1:57 AM
This revision was automatically updated to reflect the committed changes.