Re: [PATCH 6.1.y] net: add missing ns_capable check for peer netns

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Approximating `netlink_ns_capable()` with `ns_capable()` introduces a confused deputy vulnerability, allowing privilege escalation via socket passing.
--

commit fbf821e5c53ccbfb6a6615992ef20fa8b86e79bd
Author: Maximilian Heyne <mheyne@amazon.de>

net: add missing ns_capable check for peer netns

This patch attempts to backport a capability check for peer netns
creation in vxcan and veth interfaces. It replaces the upstream
netlink_ns_capable() check with an ns_capable() check to accommodate
older stable kernels.

> diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
> index 98c669ad51414..da4affff65476 100644
> --- a/drivers/net/can/vxcan.c
> +++ b/drivers/net/can/vxcan.c
[ ... ]
> @@ -211,6 +211,11 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
>  	if (IS_ERR(peer_net))
>  		return PTR_ERR(peer_net);
>  
> +	if (!ns_capable(peer_net->user_ns, CAP_NET_ADMIN)) {
> +		put_net(peer_net);
> +		return -EPERM;
> +	}
> +

[Severity: High]
Could approximating netlink_ns_capable() with ns_capable() introduce a
confused deputy vulnerability?

Because ns_capable() checks the credentials of the currently executing
thread rather than the credentials of the process that originally opened
the netlink socket, this might be bypassed via socket passing.

If an unprivileged attacker opens a netlink socket, crafts a malicious
RTM_NEWLINK message to create a vxcan peer in a privileged namespace,
and passes the socket file descriptor to a privileged setuid binary, the
message could be processed synchronously in the context of the privileged
thread. The ns_capable() check would then incorrectly succeed based on the
privileged process executing it.

Should this validate the socket's opener credentials instead to prevent
unauthorized interface creation?

[ ... ]
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index e1e8c825483aa..dac8cc5a79f5a 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
[ ... ]
> @@ -1707,6 +1707,11 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
>  	if (IS_ERR(net))
>  		return PTR_ERR(net);
>  
> +	if (!ns_capable(net->user_ns, CAP_NET_ADMIN)) {
> +		put_net(net);
> +		return -EPERM;
> +	}
> +

[Severity: High]
Does this have the same confused deputy issue as mentioned above in
vxcan_newlink() since it relies on the current thread's credentials?

If a privileged process is tricked into writing to an attacker-controlled
netlink socket, ns_capable() would evaluate the privileged current thread
instead of the unprivileged socket creator.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-keyed-dude-3493dbdb@mheyne-amazon?part=1