Netdev List
 help / color / mirror / Atom feed
* [PATCH net v7] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops
@ 2026-09-10  8:26 Abdifatah Suruur
  2026-09-11 10:49 ` Simon Horman
  0 siblings, 1 reply; 2+ messages in thread
From: Abdifatah Suruur @ 2026-09-10  8:26 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev,
	subash.a.kasiviswanathan, sean.tranchetti, dnlplm, stable,
	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.

The gates cover the configuration paths only.  Teardown (RTM_DELLINK
reaching rmnet_dellink(), and rmnet_config_notify_cb() unregistering
the bridge when the slave device is deleted) is intentionally left
ungated: link deletion is normal netdev lifecycle behaviour, and the
changelink series this matches (vxlan/geneve, CVE-2026-68432) gated
only the configuration paths.

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>
---
v7:
- state explicitly in the commit message that the dellink and notifier
  teardown paths are out of scope, per the Sashiko review
- drop the master-moved-into-the-real-device-netns rationale from the
  rmnet_add_bridge() comment: on the add path do_set_master() resolves
  the master in the slave's netns, so the rationale only applies to
  rmnet_del_bridge(), where the master comes from
  netdev_master_upper_dev_get()
- use the netdev comment style in rmnet_set_coalesce(), per the
  Sashiko review
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    | 37 ++++++++++++++++++-
 .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c   | 16 +++++++-
 2 files changed, 51 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..ca6971150180d 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,16 @@ 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.
+	 */
+	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 +509,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..7549318f83985 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,21 @@ 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


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

* Re: [PATCH net v7] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops
  2026-09-10  8:26 [PATCH net v7] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops Abdifatah Suruur
@ 2026-09-11 10:49 ` Simon Horman
  0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2026-09-11 10:49 UTC (permalink / raw)
  To: Abdifatah Suruur
  Cc: netdev, linux-kernel, davem, edumazet, kuba, pabeni,
	andrew+netdev, subash.a.kasiviswanathan, sean.tranchetti, dnlplm,
	stable

On Thu, Sep 10, 2026 at 11:26:42AM +0300, Abdifatah Suruur wrote:
> 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.
> 
> The gates cover the configuration paths only.  Teardown (RTM_DELLINK
> reaching rmnet_dellink(), and rmnet_config_notify_cb() unregistering
> the bridge when the slave device is deleted) is intentionally left
> ungated: link deletion is normal netdev lifecycle behaviour, and the
> changelink series this matches (vxlan/geneve, CVE-2026-68432) gated
> only the configuration paths.
> 
> 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>
> ---
> v7:
> - state explicitly in the commit message that the dellink and notifier
>   teardown paths are out of scope, per the Sashiko review
> - drop the master-moved-into-the-real-device-netns rationale from the
>   rmnet_add_bridge() comment: on the add path do_set_master() resolves
>   the master in the slave's netns, so the rationale only applies to
>   rmnet_del_bridge(), where the master comes from
>   netdev_master_upper_dev_get()
> - use the netdev comment style in rmnet_set_coalesce(), per the
>   Sashiko review
> 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

Reviewed-by: Simon Horman <horms@kernel.org>


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

end of thread, other threads:[~2026-09-11 10:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10  8:26 [PATCH net v7] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops Abdifatah Suruur
2026-09-11 10:49 ` Simon Horman

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