Re: [PATCH net v2] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink

[Simon Horman <horms@kernel.org>]

On Wed, Aug 19, 2026 at 11:35:53AM +0300, Abdifatah Suruur wrote:
> A changelink() on an rmnet device mutates the rmnet port state attached
> to the real device (endpoint mux table, port->data_format and, via
> rmnet_vnd_update_dev_mtu(), the real device MTU).

There is an AI-generated review of this patch available at
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260819083553.1134-1-suruurism%40gmail.com

It asks if the statement above is strictly accurate:

  Is the "the real device MTU" part accurate?  rmnet_vnd_update_dev_mtu()
  only reads real_dev->mtu:

  drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c:rmnet_vnd_update_dev_mtu() {
	hash_for_each_safe(port->muxed_ep, bkt_ep, tmp_ep, ep, hlnode) {
		if (ep->egress_dev->mtu <= (real_dev->mtu - headroom))
			continue;
		if (rmnet_vnd_change_mtu(ep->egress_dev,
					 real_dev->mtu - headroom))
			return -1;
	}
  }

  The only MTU store is WRITE_ONCE(rmnet_dev->mtu, new_mtu) inside
  rmnet_vnd_change_mtu(), i.e. on the rmnet endpoint devices, not on
  real_dev.  Would it be more precise to say that changelink can shrink the
  MTU of the rmnet endpoint netdevs plus rewrite the shared
  port->data_format and mux table?

> The real device may
> live in a different netns than the rmnet device (rmnet_newlink()
> resolves it in link_net), but the rtnl path only checks CAP_NET_ADMIN
> against dev_net(dev).  A caller privileged only in the rmnet device's
> netns can therefore rewrite the shared cellular data-path state owned by
> another netns.
> 
> Gate rmnet_changelink() with rtnl_dev_link_net_capable(), matching the
> "require CAP_NET_ADMIN in the device netns for changelink" series
> (vxlan/geneve, CVE-2026-68432).

The AI-generated review at the link above also asks if
this patch is complete:

  This isn't a bug introduced by this patch, but do the bridge ops need the
  same gate?  rmnet_add_bridge() and rmnet_del_bridge() mutate the very same
  real_dev-owned struct rmnet_port and have no capability check:

  drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c:rmnet_add_bridge() {
	port = rmnet_get_port_rtnl(real_dev);
	...
	slave_port->rmnet_mode = RMNET_EPMODE_BRIDGE;
	slave_port->bridge_ep = real_dev;
	slave_port->rmnet_dev = rmnet_dev;
	port->rmnet_mode = RMNET_EPMODE_BRIDGE;
	port->bridge_ep = slave_dev;
  }

  and on teardown, rmnet_del_bridge() -> rmnet_unregister_bridge() clears
  real_port->bridge_ep and real_port->rmnet_mode.

  Both are live rtnetlink entry points via rmnet_vnd_ops in rmnet_vnd.c:

	.ndo_add_slave  = rmnet_add_bridge,
	.ndo_del_slave  = rmnet_del_bridge,

  and do_set_master() in net/core/rtnetlink.c only resolves and authorizes
  within dev_net(dev):

	upper_dev = __dev_get_by_index(dev_net(dev), ifindex);
	...
	err = ops->ndo_add_slave(upper_dev, dev, extack);

  So with the same preconditions the commit message describes (rmnet device
  in netns A, real_dev in netns B, CAP_NET_ADMIN in A only), can a caller
  still run "ip link set <slave> master rmnetX" to set the foreign real
  device's port into RMNET_EPMODE_BRIDGE with port->bridge_ep pointing at a
  device it owns?  rmnet_rx_handler() would then call
  rmnet_bridge_handler(skb, port->bridge_ep) and forward frames received on
  the real device into the caller's netns.

  The runtime gates in rmnet_add_bridge() (port->nr_rmnet_devs <= 1,
  port->rmnet_mode == RMNET_EPMODE_VND) look like the normal
  single-rmnet-device state rather than a restriction.

  Would it make sense to apply rtnl_dev_link_net_capable() to those two ops
  as well, so the driver enforces one consistent rule about who may
  reconfigure the port?

There are also other issues raised on the same AI-generated review.
But in my opinion they they do not seem relevant to the progress of this patch.

> 
> Fixes: 2abb5792387e ("net: qualcomm: rmnet: Allow configuration updates to existing devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> 
> ---
> v2:
> - drop Reported-by: (implied for the author), per Jakub Kicinski

When you post a new revision of a patch, please do so in a new email thread,
rather than as a response to the previous version.

Thanks!

...