Page MenuHomeFreeBSD

lagg: wrap lagg_port2req() into LAGG_SLOCK()
ClosedPublic

Authored by glebius on Jan 18 2024, 7:20 PM.
Tags
None
Referenced Files
F92958437: D43501.id134804.diff
Thu, Sep 5, 4:35 PM
Unknown Object (File)
Wed, Sep 4, 11:44 PM
Unknown Object (File)
Wed, Sep 4, 8:09 AM
Unknown Object (File)
Tue, Sep 3, 1:50 AM
Unknown Object (File)
Mon, Sep 2, 12:51 AM
Unknown Object (File)
Aug 6 2024, 2:32 AM
Unknown Object (File)
Jul 19 2024, 5:25 PM
Unknown Object (File)
Jul 6 2024, 10:17 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.