RE: [PATCH net-next v8] net: qualcomm: rmnet: require CAP_NET_ADMIN in the real device netns for config ops

[<subash.a.kasiviswanathan@oss.qualcomm.com>]

> -----Original Message-----
> From: Abdifatah Suruur <suruurism@gmail.com>
> Sent: Tuesday, September 15, 2026 1:57 PM
> To: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: subash.a.kasiviswanathan@oss.qualcomm.com;
> sean.tranchetti@oss.qualcomm.com; horms@kernel.org; kuba@kernel.org;
> Abdifatah Suruur <suruurism@gmail.com>
> Subject: [PATCH net-next v8] net: qualcomm: rmnet: require
> CAP_NET_ADMIN in the real device netns for config ops
> 
> 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.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> 
> ---
> v8:
> - drop the Fixes and stable tags, per Jakub Kicinski
> - drop the explanatory comments added next to the gates, per Jakub
>   Kicinski
> - shorten the extack messages to "request modifies device in another
>   netns", per Jakub Kicinski
> 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    | 21
> ++++++++++++++++++-
>  .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c   | 11 +++++++++-
>  2 files changed, 30 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..8bf385ab44a90 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -312,6 +312,12 @@ static int rmnet_changelink(struct net_device *dev,
> struct nlattr *tb[],
>  	if (!rmnet_is_real_dev_registered(real_dev))
>  		return -ENODEV;
> 
> +	if (!rtnl_dev_link_net_capable(dev, dev_net(real_dev))) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "request modifies device in another
netns");
> +		return -EPERM;
> +	}
> +
>  	port = rmnet_get_port_rtnl(real_dev);
> 
>  	if (data[IFLA_RMNET_MUX_ID]) {
> @@ -441,6 +447,12 @@ int rmnet_add_bridge(struct net_device
> *rmnet_dev,
>  	struct rmnet_port *port, *slave_port;
>  	int err;
> 
> +	if (!rtnl_dev_link_net_capable(slave_dev, dev_net(real_dev))) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "request modifies device in another
netns");
> +		return -EPERM;
> +	}
> +
>  	port = rmnet_get_port_rtnl(real_dev);
> 
>  	/* If there is more than one rmnet dev attached, its probably being
> @@ -489,7 +501,14 @@ 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;
> +
> +	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..53802b485a813 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,16 @@ 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);
> +	if (!ns_capable(dev_net(real_dev)->user_ns, CAP_NET_ADMIN)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "request modifies device in another
netns");
> +		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

Reviewed-by: Subash Abhinov Kasiviswanathan
<subash.a.kasiviswanathan@oss.qualcomm.com>