[PATCH net v2] macvlan: require lower-netns admin for shared port settings

[PATCH net v2] macvlan: require lower-netns admin for shared port settings

[Doruk Tan Ozturk]

struct macvlan_port is per lower device and is shared by every macvlan
upper on it, including uppers that live in other network namespaces.
Two of its fields are settable over rtnetlink by any upper on the port:
port->bc_cutoff, written by IFLA_MACVLAN_BC_CUTOFF, and
port->bc_queue_len_used, recomputed from IFLA_MACVLAN_BC_QUEUE_LEN.
(port->flags and port->perm_addr are also rtnetlink-settable, but only
in passthru mode, which requires port->count == 0 and so cannot be
reached from a second upper.)

rtnetlink checks CAP_NET_ADMIN against the network namespace the
configured device lives in and nothing else, so once a macvlan has been
moved into a child network namespace, an administrator of that namespace
alone reaches macvlan_changelink(), which applies both attributes
without considering who owns the lower device.

The create path has the same gap. macvlan_common_newlink() resolves a
lower device that is itself a macvlan to the real lower device:

	if (netif_is_macvlan(lowerdev))
		lowerdev = macvlan_dev_real_dev(lowerdev);

That real device may sit in a network namespace that was never
capability-checked. The new upper then joins its macvlan_port and runs
update_port_bc_queue_len() on it, and, when IFLA_MACVLAN_BC_CUTOFF is
present, update_port_bc_cutoff().

port->bc_cutoff is not a local tuning knob. update_port_bc_cutoff()
recomputes port->bc_filter, which macvlan_handle_frame() tests to decide
whether a multicast frame is deferred to the port broadcast work queue
or flooded inline from the RX softirq, and a negative cutoff clears
bc_filter outright. A namespace that administers none of the other
uppers can therefore change how all of them receive multicast.

Reproduced on 6.8 with a dummy lower device and two macvlan uppers, one
left in the initial namespace and one moved into a child user and
network namespace. From the child, both a changelink and a nested
newlink carrying IFLA_MACVLAN_BC_CUTOFF were accepted, and the value
read back on the initial-namespace sibling followed them, changing from
1 to -7 and then to -42.

Require CAP_NET_ADMIN in the lower device network namespace before
applying a shared port setting or creating a macvlan on a flattened
lower device. rtnl_dev_link_net_capable() short-circuits when the lower
device shares the macvlan network namespace, so an ordinary
single-namespace configuration is unaffected, and per-upper settings
such as mode and flags stay available to an administrator of the
macvlan's own namespace. This is the model ipvlan has used since
commit 7cc9f7003a96 ("ipvlan: disallow userns cap_net_admin to change
global mode/flags").

Found by 0sec automated security-research tooling (https://0sec.ai).

The newlink gate is unconditional rather than keyed on a BC attribute
being present, because joining another namespace's macvlan_port is
itself a mutation of shared state; ipvlan gates ipvlan_link_new() the
same way.

IFLA_MACVLAN_BC_QUEUE_LEN is gated here as well as by any magnitude
check, because the two address different things: a magnitude check
bounds how large a value any caller may request, while this bounds who
may write the shared port at all. update_port_bc_queue_len() takes the
maximum across uppers, so a cross-namespace lowering has no security
effect and this over-rejects it; that is accepted in exchange for one
rule covering every writer of the shared struct.

Fixes: d4bff72c8401 ("macvlan: Support for high multicast packet rate")
Fixes: 954d1fa1ac93 ("macvlan: Add netlink attribute for broadcast cutoff")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:multi-model
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v2:
 - Drop the Reported-by: and Closes: naming Xiang Mei. Xiang confirmed
   this is a real issue but a different bug from the broadcast-backlog
   OOM reported in
   https://lore.kernel.org/r/20260706212556.3199234-1-xmei5@asu.edu ,
   and demonstrated it by reproducing that OOM on a kernel carrying v1.
   The two differ in who the caller is: there the caller administers
   everything it uses, here it does not administer the lower device.
   Thanks to Xiang for separating them. Bounding the backlog is a
   separate fix, and this patch neither replaces nor competes with it.
 - Rewrite the commit message around what this patch alone covers,
   cross-namespace mutation of shared macvlan_port state, with
   IFLA_MACVLAN_BC_CUTOFF and the nested-newlink path as the parts
   nothing else addresses.
 - Add a second Fixes: tag for the commit that introduced
   IFLA_MACVLAN_BC_CUTOFF.
 - Add extack messages on both rejections.
 - Add the cross-namespace bc_cutoff reproduction described above.

The permission check itself is unchanged from v1.

v1: https://lore.kernel.org/netdev/20260726125447.32244-1-doruk@0sec.ai/

 drivers/net/macvlan.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index c40fa331836bb..19c599b676076 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1479,8 +1479,14 @@ int macvlan_common_newlink(struct net_device *dev,
 	/* When creating macvlans or macvtaps on top of other macvlans - use
 	 * the real device as the lowerdev.
 	 */
-	if (netif_is_macvlan(lowerdev))
+	if (netif_is_macvlan(lowerdev)) {
 		lowerdev = macvlan_dev_real_dev(lowerdev);
+		if (!rtnl_dev_link_net_capable(dev, dev_net(lowerdev))) {
+			NL_SET_ERR_MSG(extack,
+				       "Creating a macvlan on a lower device in another network namespace requires CAP_NET_ADMIN in that namespace");
+			return -EPERM;
+		}
+	}
 
 	if (!tb[IFLA_MTU])
 		dev->mtu = lowerdev->mtu;
@@ -1619,6 +1625,14 @@ static int macvlan_changelink(struct net_device *dev,
 	enum macvlan_macaddr_mode macmode;
 	int ret;
 
+	if (data &&
+	    (data[IFLA_MACVLAN_BC_QUEUE_LEN] || data[IFLA_MACVLAN_BC_CUTOFF]) &&
+	    !rtnl_dev_link_net_capable(dev, dev_net(vlan->lowerdev))) {
+		NL_SET_ERR_MSG(extack,
+			       "Changing shared macvlan port settings requires CAP_NET_ADMIN in the lower device network namespace");
+		return -EPERM;
+	}
+
 	/* Validate mode, but don't set yet: setting flags may fail. */
 	if (data && data[IFLA_MACVLAN_MODE]) {
 		set_mode = true;
-- 
2.43.0