netdev - Re: [PATCH] net: wireguard: Allow binding to specific ifindex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241203090927.GA9361@kernel.org>
Date: Tue, 3 Dec 2024 09:09:27 +0000
From: Simon Horman <horms@...nel.org>
To: greearb@...delatech.com
Cc: netdev@...r.kernel.org, Jason@...c4.com, wireguard@...ts.zx2c4.com,
	dsahern@...nel.org
Subject: Re: [PATCH] net: wireguard: Allow binding to specific ifindex

On Mon, Nov 25, 2024 at 01:21:11PM -0800, greearb@...delatech.com wrote:
> From: Ben Greear <greearb@...delatech.com>
> 
> Which allows us to bind to VRF.
> 
> Signed-off-by: Ben Greear <greearb@...delatech.com>
> ---
> 
> NOTE:  Modified user-space to utilize this may be found here:
> https://github.com/greearb/wireguard-tools-ct
> Only the 'wg' part has been tested with this new feature as of today.

...

> diff --git a/drivers/net/wireguard/socket.c b/drivers/net/wireguard/socket.c
> index 0414d7a6ce74..a7cb1c7c3112 100644
> --- a/drivers/net/wireguard/socket.c
> +++ b/drivers/net/wireguard/socket.c
> @@ -25,7 +25,8 @@ static int send4(struct wg_device *wg, struct sk_buff *skb,
>  		.daddr = endpoint->addr4.sin_addr.s_addr,
>  		.fl4_dport = endpoint->addr4.sin_port,
>  		.flowi4_mark = wg->fwmark,
> -		.flowi4_proto = IPPROTO_UDP
> +		.flowi4_proto = IPPROTO_UDP,
> +		.flowi4_oif = wg->lowerdev,
>  	};
>  	struct rtable *rt = NULL;
>  	struct sock *sock;
> @@ -111,6 +112,9 @@ static int send6(struct wg_device *wg, struct sk_buff *skb,
>  	struct sock *sock;
>  	int ret = 0;
>  
> +	if (wg->lowerdev)
> +		fl.flowi6_oif = wg->lowerdev,

Hi Ben,

I think that the trailing ',' on the line above should be a ';'.
As written, with a ',', the call to skb_mark_not_on_list() 
below will be included in the conditional block above.
And this doesn't seem to be the intention of the code based on indentation.

Flagged by clang-19 with -Wcomma

> +
>  	skb_mark_not_on_list(skb);
>  	skb->dev = wg->dev;
>  	skb->mark = wg->fwmark;

...