Page MenuHomeFreeBSD

if: avoid interface destroy race
ClosedPublic

Authored by kp on Mar 29 2022, 8:54 AM.

Details

Summary

When we destroy an interface while the jail containing it is being
destroyed we risk seeing a race between if_vmove() and the destruction
code, which results in us trying to move a destroyed interface.

Protect against this by using the ifnet_detach_sxlock to also covert
if_vmove() (and not just detach).

PR: 262829
MFC after: 3 weeks

Diff Detail

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

Event Timeline

kp requested review of this revision.Mar 29 2022, 8:54 AM
zec requested changes to this revision.Mar 30 2022, 7:08 PM
zec added a subscriber: zec.

This change builds on top if_index globalization (91f44749c6feb50f39af8805dd803e860f0418f1) which I strongly objected to, and which glebius agreed to back out as outlined in https://github.com/glebius/FreeBSD/commits/backout-ifindex, but that never happened. Hence, pls. don't proceed with this until if_index is reverted back to per-VNET state.

This revision now requires changes to proceed.Mar 30 2022, 7:08 PM
In D34704#786704, @zec wrote:

This change builds on top if_index globalization (91f44749c6feb50f39af8805dd803e860f0418f1) which I strongly objected to, and which glebius agreed to back out as outlined in https://github.com/glebius/FreeBSD/commits/backout-ifindex, but that never happened. Hence, pls. don't proceed with this until if_index is reverted back to per-VNET state.

Given the updated plan in addressing the concerns with if_index globalization I'm going to proceed with this commit.

It fixes a panic (as demonstrated by the attached test case) and the fix doesn't rely on the if_index globalization for anything other than assertions, so it won't block any evolution of the code in that regard.

This revision was not accepted when it landed; it landed in state Needs Revision.May 6 2022, 11:56 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.