Differential D34704

if: avoid interface destroy race
ClosedPublic
Actions

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

Details

Reviewers

zec

Group Reviewers

network

Commits

rG4dfd3ffc4488: if: avoid interface destroy race
rG868bf82153e8: if: avoid interface destroy race

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

Severity	Location	Code	Message
Warning	tests/sys/net/if_clone_test.sh:	CHMOD1	Invalid Executable

Unit

No Test Coverage

Build Status

Buildable 44914
Build 41802: arc lint + arc unit

Event Timeline

kp created this revision.Mar 29 2022, 8:54 AM

Herald added subscribers: glebius, melifaro, ae, imp. · View Herald TranscriptMar 29 2022, 8:54 AM

kp requested review of this revision.Mar 29 2022, 8:54 AM

Harbormaster completed remote builds in B44914: Diff 104316.Mar 29 2022, 8:54 AM

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.

Closed by commit rG868bf82153e8: if: avoid interface destroy race (authored by kp). · Explain Why

This revision was automatically updated to reflect the committed changes.

kp added a commit: rG868bf82153e8: if: avoid interface destroy race.

kp added a commit: rG4dfd3ffc4488: if: avoid interface destroy race.May 27 2022, 4:37 PM