Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink
@ 2026-08-13 17:47 Abdifatah Suruur
  2026-08-18 16:23 ` Jakub Kicinski
  2026-08-19  8:35 ` [PATCH net v2] " Abdifatah Suruur
  0 siblings, 2 replies; 4+ messages in thread
From: Abdifatah Suruur @ 2026-08-13 17:47 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, quic_subashab, quic_stranche, doruk,
	Abdifatah Suruur

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).  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).

Fixes: 2abb5792387e ("net: qualcomm: rmnet: Allow configuration updates to existing devices")
Reported-by: Abdifatah Suruur <suruurism@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>

---
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -312,6 +312,14 @@
 	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)))
+		return -EPERM;
+
 	port = rmnet_get_port_rtnl(real_dev);
 
 	if (data[IFLA_RMNET_MUX_ID]) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink
  2026-08-13 17:47 [PATCH net] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink Abdifatah Suruur
@ 2026-08-18 16:23 ` Jakub Kicinski
  2026-08-19  8:35 ` [PATCH net v2] " Abdifatah Suruur
  1 sibling, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-08-18 16:23 UTC (permalink / raw)
  To: Abdifatah Suruur
  Cc: netdev, linux-kernel, quic_subashab, quic_stranche, doruk

On Thu, 13 Aug 2026 20:47:27 +0300 Abdifatah Suruur wrote:
> Reported-by: Abdifatah Suruur <suruurism@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>

If you are the author the Reported-by is implied.
please fix this and repost.
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net v2] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink
  2026-08-13 17:47 [PATCH net] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink Abdifatah Suruur
  2026-08-18 16:23 ` Jakub Kicinski
@ 2026-08-19  8:35 ` Abdifatah Suruur
  2026-08-21 10:19   ` Simon Horman
  1 sibling, 1 reply; 4+ messages in thread
From: Abdifatah Suruur @ 2026-08-19  8:35 UTC (permalink / raw)
  To: netdev; +Cc: linux-kernel, quic_subashab, quic_stranche, doruk, kuba

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).  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).

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

--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -312,6 +312,14 @@
 	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)))
+		return -EPERM;
+
 	port = rmnet_get_port_rtnl(real_dev);

 	if (data[IFLA_RMNET_MUX_ID]) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v2] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink
  2026-08-19  8:35 ` [PATCH net v2] " Abdifatah Suruur
@ 2026-08-21 10:19   ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2026-08-21 10:19 UTC (permalink / raw)
  To: Abdifatah Suruur
  Cc: netdev, linux-kernel, quic_subashab, quic_stranche, doruk, kuba

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!

...

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-21 10:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 17:47 [PATCH net] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for changelink Abdifatah Suruur
2026-08-18 16:23 ` Jakub Kicinski
2026-08-19  8:35 ` [PATCH net v2] " Abdifatah Suruur
2026-08-21 10:19   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox