Jail/bridge fixes and a bridge-port procd data type for external bridge membership - #89
Open
dangowrt wants to merge 4 commits into
Open
Jail/bridge fixes and a bridge-port procd data type for external bridge membership#89dangowrt wants to merge 4 commits into
dangowrt wants to merge 4 commits into
Conversation
The vlan_aliases kvlist is initialised lazily in device_vlan_update(), which only runs during configuration load. A bridge that is created and freed without going through a config load (e.g. one created at runtime over ubus) reaches bridge_free() with the kvlist still zero-filled, and kvlist_free() then walks an uninitialised avl_tree whose list head contains NULL pointers, crashing netifd. Guard the call on get_len, the same marker device_vlan_update() uses to tell an initialised kvlist apart. Fixes: 4544f02 ("bridge-vlan: add support for defining aliases for vlan ids") Signed-off-by: Daniel Golle <daniel@makrotopia.org>
system_link_netns_move() built its RTM_NEWLINK message with whatever system_if_resolve() returned for the source device. When the device does not exist (e.g. a jailed interface whose veth has not been created yet), system_if_resolve() returns 0, and the message goes out with ifi_index = 0 while IFLA_IFNAME is set to target_ifname (the jail_device name). The kernel's __rtnl_newlink() then selects the target device by name when ifi_index is 0 (net/core/rtnetlink.c: rtnl_dev_get -> __dev_get_by_name), so the move operates on whatever host device happens to be named like the jail_device. With a jail_device of "eth0" this moves the host's real eth0 into the container's network namespace, knocking the host off the network. Bail out when the source device cannot be resolved to a real ifindex so the request is never sent with a zero index. Fixes: d93126d ("interface: allow renaming interface when moving to jail netns") Signed-off-by: Daniel Golle <daniel@makrotopia.org>
interface_start_jail() moved each jailed interface's main device into the container netns at the moment the jail's netns was registered. If the device did not exist yet (e.g. a veth whose creation had not been triggered), the move was a no-op and the container came up with no network device; the in-jail netifd then failed DHCP with "udhcpc: SIOCGIFINDEX: No such device" until the operator manually brought the host interface up. Keep a duplicated reference to the jail netns on the interface when the move cannot be performed yet, and complete it from interface_main_dev_cb once the device appears (DEV_EVENT_ADD). Any stale pending reference from an earlier jail incarnation is dropped on the next interface_start_jail() invocation, so a successful immediate move can never leave a dangling netns reference behind that would later divert the device into a dead namespace. interface_stop_jail and interface_free drop any still-pending reference as well. Fixes: 1321c1b ("add basic support for jail network namespaces") Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Bridge membership is declared from the bridge side only: a bridge's
"ports" list in UCI or in a "network-device" procd data entry, and the
"ports" lists of its bridge-vlan sections. A separate procd publisher
such as a container manager that wants to attach a port to a bridge it
does not own therefore has no declarative way to do so. Publishing a
same-named "network-device" entry replaces the bridge's ports, and a
"bridge-vlan" entry replaces the VLAN and forces vlan_filtering on.
Add a "bridge-port" procd data type keyed by the port device name:
"bridge-port": {
"<port>": { "device": "<bridge>", "vlans": [ "<vid>[:t|:*]", ... ] }
}
The entries are read at the start of each config load and merged into
the loaded UCI network package in memory, following the precedent of
config_fixup_bridge_ports(): the port is appended to the "ports" list of
the UCI device section whose name matches "device", and each "vlans"
element appends "<port>[:flags]" to the "ports" list of the bridge-vlan
section with the matching device and vlan id. A "ports" option written
as a whitespace separated string is split into a list first, since
uci_add_list() would otherwise keep the whole string as a single list
element. Legacy bridges declared with "option type bridge" on an
interface section are merged in the same way. Nothing is committed, and
config_init_package() reloads the package on every config_init_all(), so
the merge only lasts as long as the publisher's service data exists. The
port becomes an ordinary versioned member and is dropped by the bridge's
vlist flush on the first reload after the data is withdrawn.
Only UCI-defined bridges and bridge-vlan sections are merged; bridges
defined through "network-device" data carry their own "ports" from their
single owner. "vlans" elements naming a vlan id without a matching
bridge-vlan section are ignored, as are bridge-vlan sections whose vlan
id is not a number between 1 and 4095.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
This was referenced Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent, self-contained bugfixes on the jail and bridge paths, plus one new opt-in procd data type. The three fixes are individually cherry-pickable and useful on their own even if the feature needs discussion.
Fixes: 4544f026bb09). A bridge created and freed over ubus without ever going through a config load never initialises itsvlan_aliaseskvlist, sobridge_free()dereferences it. Guard the free.Fixes: d93126d3b91e).system_link_netns_move()sentRTM_NEWLINKwithifi_index = 0when the source device could not be resolved, so the kernel selected the target by name and could move the host's real interface (for exampleeth0) into a container. Bail out when the device has no ifindex.Fixes: 1321c1bd8fe9).interface_start_jail()moved each jailed interface's device at netns registration time; if the device did not exist yet (a veth whose creation had not been triggered), the move was a no-op and the container came up with no network device. Keep a reference to the jail netns and complete the move once the device appears.portslist and theportslists of its bridge-vlan sections), so a separate procd publisher such as a container manager that wants to attach a port to a bridge it does not own has no declarative way to do so: a same-namednetwork-deviceentry replaces the bridge's ports, and abridge-vlanentry replaces the VLAN and forcesvlan_filteringon. The newbridge-porttype, keyed by the port device name and carrying{ "device": "<bridge>", "vlans": [ "<vid>[:t|:*]", ... ] }, is merged into the loaded UCI network package in memory at the start of each config load, following the precedent ofconfig_fixup_bridge_ports(). Nothing is committed and the package is reloaded on everyconfig_init_all(), so the port lives exactly as long as the publisher's service data and is dropped by the bridge's vlist flush on the first reload after the data is withdrawn. The type is standalone: it introduces no build or runtime dependency on procd and is simply ignored when nothing publishes it.The feature is consumed by procd's
ujail/uxc-net, which publishes container veths asbridge-portentries so a container can join an existing user-owned bridge (including a VLAN-filtering bridge) without disturbing the owner's configuration, but netifd does not depend on that.