Page MenuHomeFreeBSD

Wrap a vlan's parent's if_output in a separate function.
ClosedPublic

Authored by mjoras on Aug 10 2019, 7:44 PM.

Details

Summary

When a vlan interface is created, its if_output is set directly to the
parent interface's if_output. This is fine in the normal case but has an
unfortunate consequence if you end up with a certain combination of vlan
and lagg interfaces.

Consider you have a lagg interface with a single laggport member. When
an interface is added to a lagg its if_output is set to
lagg_port_output, which blackholes traffic from the normal networking
stack but not certain frames from BPF (pseudo_AF_HDRCMPLT). If you now
create a vlan with the laggport member (not the lagg interface) as its
parent, its if_output is set to lagg_port_output as well. While this is
confusing conceptually and likely represents a misconfigured system, it
is not itself a problem. The problem arises when you then remove the
lagg interface. Doing this resets the if_output of the laggport member
back to its original state, but the vlan's if_output is left pointing to
lagg_port_output. This gives rise to the possibility that the system
will panic when e.g. bpf is used to send any frames on the vlan
interface.

Fix this by creating a new function, vlan_output, which simply wraps the
parent's current if_output. That way when the parent's if_output is
restored there is no stale usage of lagg_port_output.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mjoras created this revision.Aug 10 2019, 7:44 PM
ae added a comment.Aug 11 2019, 9:45 AM

If you know that the problem occurs only with BPF, bpfwrite() invokes if_output() already in epoch section.

In D21209#461161, @ae wrote:

If you know that the problem occurs only with BPF, bpfwrite() invokes if_output() already in epoch section.

That's a good point. It's the only place I know of a reported panic, but I think the panic can happen with any direct call to if_output. Probably not every such call is currently in an epoch section. Do we care about pointlessly refreshing the epoch section?

rstone accepted this revision.Aug 27 2019, 8:54 PM
This revision is now accepted and ready to land.Aug 27 2019, 8:54 PM
This revision was automatically updated to reflect the committed changes.