Page MenuHomeFreeBSD

Fix panic when adding vtnet interfaces to a bridge
ClosedPublic

Authored by kp on Jun 13 2015, 4:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 15 2024, 8:18 AM
Unknown Object (File)
Oct 6 2024, 10:51 PM
Unknown Object (File)
Oct 6 2024, 10:51 PM
Unknown Object (File)
Oct 6 2024, 10:51 PM
Unknown Object (File)
Oct 6 2024, 10:09 PM
Unknown Object (File)
Oct 2 2024, 2:59 AM
Unknown Object (File)
Sep 25 2024, 2:20 AM
Unknown Object (File)
Sep 24 2024, 6:23 PM
Subscribers

Details

Summary

vtnet interfaces are always in promiscuous mode (at least if the
VIRTIO_NET_F_CTRL_RX feature is not negotiated with the host). if_promisc() on
a vtnet interface returned ENOTSUP although it has IFF_PROMISC set. This
confused the bridge code. Instead we now accept all enable/disable promiscuous
commands (and always keep IFF_PROMISC set).

There are also two issues with the if_bridge error handling.

If if_promisc() fails it uses bridge_delete_member() to clean up. This tries to
disable promiscuous mode on the interface. That runs into an assert, because
promiscuous mode was never set in the first place. (That's the panic reported in
PR 200210.)
We can only unset promiscuous mode if the interface actually is promiscuous.
This goes against the reference counting done by if_promisc(), but only the
first/last if_promic() calls can actually fail, so this is safe.

A second issue is a double free of bif. It's already freed by
bridge_delete_member().

PR: 200210

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kp retitled this revision from to Fix panic when adding vtnet interfaces to a bridge.
kp updated this object.
kp edited the test plan for this revision. (Show Details)
kp added reviewers: philip, gnn.
kp set the repository for this revision to rS FreeBSD src repository - subversion.
sys/net/if_bridge.c
1040

Is it safe to dereference ifs here? Check locking in ifpromisc().

sys/net/if_bridge.c
1040

ifpromisc() doesn't acquire any locks. It calls if_setflag() to do actual work, but that one doesn't acquire anything either.

I'm not entirely sure what lock protects the ifnet here, but I am confident this isn't making things any more broken.

philip edited edge metadata.
This revision is now accepted and ready to land.Jun 13 2015, 7:35 PM
This revision was automatically updated to reflect the committed changes.