Page MenuHomeFreeBSD

if: avoid interface destroy race
ClosedPublic

Authored by kp on Mar 29 2022, 8:54 AM.
Tags
None
Referenced Files
F55703259: D34704.diff
Wed, Feb 1, 6:36 PM
Unknown Object (File)
Dec 14 2022, 6:58 PM
Unknown Object (File)
Dec 10 2022, 7:59 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
rS FreeBSD src repository - subversion
Lint
Lint Warnings
SeverityLocationCodeMessage
Warningtests/sys/net/if_clone_test.sh:CHMOD1Invalid Executable
Unit
No Test Coverage
Build Status
Buildable 44914
Build 41802: arc lint + arc unit

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.