Page MenuHomeFreeBSD

ifnet: if_detach(): Fix races with if_vmove()
Needs ReviewPublic

Authored by zlei on Mon, Apr 13, 10:00 AM.

Details

Reviewers
kp
pouria
Group Reviewers
network
Summary

The rationality is that the driver private data holds a strong reference
to the interface, and the detach operation shall never fail. Given the
vmove operation is not atomic and spans multiple steps, acquire
ifnet_detach_sxlock only for if_detach_internal() and if_vmove() is not
sufficient. It is possible that the thread running if_detach() sees
stale vnet, or the vmoving is in progress, hence if_unlink_ifnet() can
fail.

Fix that by extending coverage of ifnet_detach_sxlock a bit to cover
the entire detach and vmove operations.

Since it is an error to fail to detach, and if_detach() is a public KPI,
prefer panic() instead of assertion, to indicate the error explicitly.
That shall prevent potential races with vmove operations ( concurrent
if_detach_internal() and if_attach_internal() ) and the status of the
interface get corrupted, which is harder to diagnose.

PR: 292993
MFC after: 2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zlei requested review of this revision.Mon, Apr 13, 10:00 AM

This is still not sufficient to fix 292993. The thread running ether_ifdetach() / bpfdetach() is still racing with those running if_vmove() / bpf_vmove() . Ideally bpfdetach() shall run after if_detach(), but that is a behavior change, so I will later post a workaround for that.

I have WIP to overhaul the if_attach() and if_detach(). But that is a longer task and I think a workaround shall fit right now.

This is still not sufficient to fix 292993. The thread running ether_ifdetach() / bpfdetach() is still racing with those running if_vmove() / bpf_vmove() . Ideally bpfdetach() shall run after if_detach(), but that is a behavior change, so I will later post a workaround for that.

See D56375.