[PATCH net v6] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops

[PATCH net v6] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops

[Abdifatah Suruur]

An rmnet device may be created with its real device in a different
netns than the rmnet device itself (rmnet_newlink() resolves it in
link_net), and the config paths below only check CAP_NET_ADMIN against
dev_net(dev), while mutating rmnet port state attached to the real
device:

- rmnet_changelink() rewrites the endpoint mux table and
  port->data_format and, via rmnet_vnd_update_dev_mtu(), can shrink the
  MTU of the rmnet endpoint netdevs.
- rmnet_add_bridge() and rmnet_del_bridge(), reachable via
  ndo_add_slave/ndo_del_slave through RTM_SETLINK IFLA_MASTER, flip
  port->rmnet_mode and port->bridge_ep on the real device's port; with
  bridge_ep pointing at a caller-owned device, rmnet_rx_handler() then
  forwards real-device ingress frames to it.
- rmnet_set_coalesce() rewrites the port aggregation parameters via
  ETHTOOL_SCOALESCE (ioctl) or ETHTOOL_MSG_COALESCE_SET (netlink),
  whose capability checks likewise only cover dev's netns.

A caller privileged only in the rmnet device's netns can therefore
rewrite the shared cellular data-path state owned by another netns, and
steer its ingress traffic.

Gate the rtnl paths with rtnl_dev_link_net_capable(), matching the
"require CAP_NET_ADMIN in the device netns for changelink" series
(vxlan/geneve, CVE-2026-68432), and gate the ethtool setter with
ns_capable() in the real device netns, mirroring the check dev_ethtool()
already applies to dev's netns.  Report the new rejections through
extack where one is available.

Fixes: 2abb5792387e ("net: qualcomm: rmnet: Allow configuration updates to existing devices")
Fixes: 60d58f971c1077 ("net: qualcomm: rmnet: Implement bridge mode")
Fixes: db8a563a9d9024 ("net: qualcomm: rmnet: add ethtool support for configuring tx aggregation")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
---
v6:
- gate rmnet_add_bridge() and rmnet_del_bridge() on slave_dev, the
  RTM_SETLINK target the caller was actually authorized against,
  instead of rmnet_dev: a master moved into the real device's netns
  short-circuits rtnl_dev_link_net_capable() through the net_eq()
  check, so a caller only privileged in the slave's netns could
  otherwise attach or clear the bridge state of the real device's
  port
- add the Fixes tag for the commit that introduced rmnet_set_coalesce()
v5:
- also gate rmnet_set_coalesce(), the ethtool setter that rewrites the
  port aggregation parameters of the real device's port, per the
  Sashiko review
- report the new capability rejections through extack where available
v4:
- use the netdev comment style, per Subash Abhinov Kasiviswanathan
v3:
- cover rmnet_add_bridge() and rmnet_del_bridge() with the same gate;
  they mutate the same real-device port state via ndo_add_slave/
  ndo_del_slave and have no capability check of their own
- correct the impact wording: rmnet_vnd_update_dev_mtu() only reads
  real_dev->mtu; the MTU store lands on the rmnet endpoint netdevs via
  rmnet_vnd_change_mtu(), not on the real device
v2:
- drop Reported-by: (implied for the author), per Jakub Kicinski
---
 .../ethernet/qualcomm/rmnet/rmnet_config.c    | 40 ++++++++++++++++++-
 .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c   | 18 ++++++++-
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index bed6f63facf25..8df9e455110dd 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -312,6 +312,16 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (!rmnet_is_real_dev_registered(real_dev))
 		return -ENODEV;
 
+	/* The rtnl path only checks CAP_NET_ADMIN against dev_net(dev),
+	 * but the port state mutated below is attached to real_dev, which
+	 * may live in a different netns.
+	 */
+	if (!rtnl_dev_link_net_capable(dev, dev_net(real_dev))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Changing the port settings requires CAP_NET_ADMIN in the real device network namespace");
+		return -EPERM;
+	}
+
 	port = rmnet_get_port_rtnl(real_dev);
 
 	if (data[IFLA_RMNET_MUX_ID]) {
@@ -441,6 +451,19 @@ int rmnet_add_bridge(struct net_device *rmnet_dev,
 	struct rmnet_port *port, *slave_port;
 	int err;
 
+	/* The rtnl path authorizes the caller against the RTM_SETLINK
+	 * target, slave_dev, but the port state mutated below is attached
+	 * to real_dev, which may live in a different netns.  Check the
+	 * capability against slave_dev so the master's netns, which the
+	 * caller was never checked against, cannot short-circuit the gate
+	 * after being moved into the real device's netns.
+	 */
+	if (!rtnl_dev_link_net_capable(slave_dev, dev_net(real_dev))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Attaching a bridge device requires CAP_NET_ADMIN in the real device network namespace");
+		return -EPERM;
+	}
+
 	port = rmnet_get_port_rtnl(real_dev);
 
 	/* If there is more than one rmnet dev attached, its probably being
@@ -489,7 +512,22 @@ int rmnet_add_bridge(struct net_device *rmnet_dev,
 int rmnet_del_bridge(struct net_device *rmnet_dev,
 		     struct net_device *slave_dev)
 {
-	struct rmnet_port *port = rmnet_get_port_rtnl(slave_dev);
+	struct rmnet_priv *priv = netdev_priv(rmnet_dev);
+	struct net_device *real_dev = priv->real_dev;
+	struct rmnet_port *port;
+
+	/* The rtnl path authorizes the caller against the RTM_SETLINK
+	 * target, slave_dev, but rmnet_unregister_bridge() below clears
+	 * the bridge state of the real device's port, which may live in a
+	 * different netns.  Check the capability against slave_dev so the
+	 * master's netns, which the caller was never checked against,
+	 * cannot short-circuit the gate after being moved into the real
+	 * device's netns.
+	 */
+	if (!rtnl_dev_link_net_capable(slave_dev, dev_net(real_dev)))
+		return -EPERM;
+
+	port = rmnet_get_port_rtnl(slave_dev);
 
 	rmnet_unregister_bridge(port);
 
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 4f0ddcedfa979..1f4a3246f254a 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -4,9 +4,11 @@
  * RMNET Data virtual network driver
  */
 
+#include <linux/capability.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
 #include <linux/if_arp.h>
+#include <linux/netlink.h>
 #include <net/pkt_sched.h>
 #include "rmnet_config.h"
 #include "rmnet_handlers.h"
@@ -240,9 +242,23 @@ static int rmnet_set_coalesce(struct net_device *dev,
 			      struct netlink_ext_ack *extack)
 {
 	struct rmnet_priv *priv = netdev_priv(dev);
+	struct net_device *real_dev = priv->real_dev;
 	struct rmnet_port *port;
 
-	port = rmnet_get_port_rtnl(priv->real_dev);
+	/*
+	 * The aggregation parameters live in the port attached to
+	 * real_dev, which may reside in a different netns.  The ethtool
+	 * paths only require CAP_NET_ADMIN in dev's netns, so require it
+	 * in real_dev's netns as well before mutating the shared port
+	 * state.
+	 */
+	if (!ns_capable(dev_net(real_dev)->user_ns, CAP_NET_ADMIN)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Changing aggregation parameters requires CAP_NET_ADMIN in the real device network namespace");
+		return -EPERM;
+	}
+
+	port = rmnet_get_port_rtnl(real_dev);
 
 	if (kernel_coal->tx_aggr_max_frames < 1 || kernel_coal->tx_aggr_max_frames > 64)
 		return -EINVAL;
-- 
2.53.0

Re: [PATCH net v6] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 2

- [High] The new rtnl_dev_link_net_capable() gate in rmnet_del_bridge()
  (rmnet_config.c:519-529) does not close the bug class the commit…
- [Low] The comment added above the new capability gate in
  rmnet_add_bridge()…
- [Low] The v4 changelog states 'use the netdev comment style, per Subash
  Abhinov Kasiviswanathan', but the block comment newly added in…

--

net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops

This adds capability gates so that rmnet config operations which mutate the
rmnet_port attached to the real device also require CAP_NET_ADMIN in the real
device's network namespace.  rmnet_changelink(), rmnet_add_bridge() and
rmnet_del_bridge() are gated with rtnl_dev_link_net_capable(), and
rmnet_set_coalesce() with ns_capable() on the real device's user namespace.

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index bed6f63facf2..8df9e455110d 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c

[ ... ]

> @@ -441,6 +451,19 @@ int rmnet_add_bridge(struct net_device *rmnet_dev,
>  	struct rmnet_port *port, *slave_port;
>  	int err;
>  
> +	/* The rtnl path authorizes the caller against the RTM_SETLINK
> +	 * target, slave_dev, but the port state mutated below is attached
> +	 * to real_dev, which may live in a different netns.  Check the
> +	 * capability against slave_dev so the master's netns, which the
> +	 * caller was never checked against, cannot short-circuit the gate
> +	 * after being moved into the real device's netns.
> +	 */
> +	if (!rtnl_dev_link_net_capable(slave_dev, dev_net(real_dev))) {

[Severity: Low]

Is the second half of this comment accurate for the add path?  The scenario
it describes is the master having been moved into the real device's netns so
that the net_eq() test inside rtnl_dev_link_net_capable() short-circuits.

On the ndo_add_slave path the master is resolved in the slave's netns by
do_set_master() in net/core/rtnetlink.c:

	upper_dev = __dev_get_by_index(dev_net(dev), ifindex);
	if (!upper_dev)
		return -EINVAL;
	ops = upper_dev->netdev_ops;
	if (ops->ndo_add_slave) {
		err = ops->ndo_add_slave(upper_dev, dev, extack);

Here dev is the slave, so dev_net(rmnet_dev) == dev_net(slave_dev) always
holds by the time rmnet_add_bridge() runs, which makes
rtnl_dev_link_net_capable(slave_dev, dev_net(real_dev)) and
rtnl_dev_link_net_capable(rmnet_dev, dev_net(real_dev)) equivalent at this
call site.

The same wording is accurate in rmnet_del_bridge(), where the master comes
from netdev_master_upper_dev_get(dev) with no per-netns lookup.  Would it be
clearer to keep the "moved into the real device's netns" rationale only in
rmnet_del_bridge() and leave a shorter note here?

>  	port = rmnet_get_port_rtnl(real_dev);
>  
>  	/* If there is more than one rmnet dev attached, its probably being
> @@ -489,7 +512,22 @@ int rmnet_add_bridge(struct net_device *rmnet_dev,
>  int rmnet_del_bridge(struct net_device *rmnet_dev,
>  		     struct net_device *slave_dev)
>  {
> -	struct rmnet_port *port = rmnet_get_port_rtnl(slave_dev);
> +	struct rmnet_priv *priv = netdev_priv(rmnet_dev);
> +	struct net_device *real_dev = priv->real_dev;
> +	struct rmnet_port *port;
> +
> +	/* The rtnl path authorizes the caller against the RTM_SETLINK
> +	 * target, slave_dev, but rmnet_unregister_bridge() below clears
> +	 * the bridge state of the real device's port, which may live in a
> +	 * different netns.  Check the capability against slave_dev so the
> +	 * master's netns, which the caller was never checked against,
> +	 * cannot short-circuit the gate after being moved into the real
> +	 * device's netns.
> +	 */
> +	if (!rtnl_dev_link_net_capable(slave_dev, dev_net(real_dev)))
> +		return -EPERM;
> +
> +	port = rmnet_get_port_rtnl(slave_dev);
>  
>  	rmnet_unregister_bridge(port);

[Severity: High]

Does this -EPERM actually close the teardown direction described in the
commit message?  The same mutation of the real device's port looks reachable
via RTM_DELLINK with CAP_NET_ADMIN only in the rmnet device's netns.

rtnl_dellink() resolves the victim in tgt_net and checks nothing against
dev_net(real_dev):

net/core/rtnetlink.c:rtnl_dellink() {
	...
	if (ifm->ifi_index > 0)
		dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
	...
	if (dev)
		err = rtnl_delete_link(dev, portid, nlh);
}

rmnet_dellink() then reaches into the real device's port unconditionally:

drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c:rmnet_dellink() {
	real_port = rmnet_get_port_rtnl(real_dev);
	bridge_dev = real_port->bridge_ep;
	if (bridge_dev) {
		bridge_port = rmnet_get_port_rtnl(bridge_dev);
		rmnet_unregister_bridge(bridge_port);
	}
	...
	netdev_upper_dev_unlink(real_dev, dev);
	rmnet_unregister_real_device(real_dev);
}

so real_port->bridge_ep and real_port->rmnet_mode are cleared, the mux
endpoint is removed, real_port->nr_rmnet_devs is decremented, and
rmnet_unregister_real_device() can call netdev_rx_handler_unregister() on
real_dev and free the port.

The precondition is the same one the patch assumes for changelink: the rmnet
device is created by an admin privileged in the real device's netns and then
moved into another netns, after which "ip link del rmnet0" from that netns
reaches the sequence above.

Deleting the bridge slave device looks similar:
rmnet_config_notify_cb() -> rmnet_force_unassociate_device() also calls
rmnet_unregister_bridge(port), which follows port->bridge_ep into the real
device's netns with no check against dev_net(real_dev).

If those paths stay ungated, is the net effect of the new check in
rmnet_del_bridge() limited to rejecting the clean IFLA_MASTER=0 unenslave,
while the commit message's statement that

  "A caller privileged only in the rmnet device's netns can therefore
   rewrite the shared cellular data-path state owned by another netns"

remains true for the teardown direction after the patch?

Teardown-on-delete is normal netdev lifecycle behaviour, and the vxlan/geneve
series cited in the commit message gated only changelink, so gating dellink
may not be the right fix.  Could the commit message state explicitly that the
dellink and notifier teardown paths are out of scope, so stable backporters
can judge the coverage?  The add direction, where bridge_ep is pointed at a
caller-owned device and rmnet_rx_handler() steers the real device's ingress
frames, does look closed by the rmnet_add_bridge() gate.

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> index 4f0ddcedfa97..1f4a3246f254 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c

[ ... ]

> @@ -240,9 +242,23 @@ static int rmnet_set_coalesce(struct net_device *dev,
>  			      struct netlink_ext_ack *extack)
>  {
>  	struct rmnet_priv *priv = netdev_priv(dev);
> +	struct net_device *real_dev = priv->real_dev;
>  	struct rmnet_port *port;
>  
> -	port = rmnet_get_port_rtnl(priv->real_dev);
> +	/*
> +	 * The aggregation parameters live in the port attached to
> +	 * real_dev, which may reside in a different netns.  The ethtool

[Severity: Low]

This isn't a bug, but this block comment opens with a bare "/*" on its own
line, while the v4 changelog says "use the netdev comment style, per Subash
Abhinov Kasiviswanathan".

The three comments this patch adds to rmnet_config.c use the netdev form,
for example in rmnet_changelink():

	/* The rtnl path only checks CAP_NET_ADMIN against dev_net(dev),

and the existing comments in rmnet_vnd.c do too, for example above
rmnet_vnd_setup():

	/* Called by kernel whenever a new rmnet<n> device is created. Sets MTU,
	 * flags, ARP type, needed headroom, etc...
	 */

Note that checkpatch.pl in this tree has no networking block comment check,
so this is only an internal inconsistency with the rest of the patch and the
rest of the file.

> +	 * paths only require CAP_NET_ADMIN in dev's netns, so require it
> +	 * in real_dev's netns as well before mutating the shared port
> +	 * state.
> +	 */
> +	if (!ns_capable(dev_net(real_dev)->user_ns, CAP_NET_ADMIN)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Changing aggregation parameters requires CAP_NET_ADMIN in the real device network namespace");
> +		return -EPERM;
> +	}
> +
> +	port = rmnet_get_port_rtnl(real_dev);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909080209.6348-1-suruurism%40gmail.com