netdev - [PATCH net] vxlan: Fix regression when dropping packets due to invalid src addresses

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: 
 <PH0PR18MB4474EF3DC991F456E6D1C524DEFF2@PH0PR18MB4474.namprd18.prod.outlook.com>
Date: Mon, 3 Jun 2024 06:45:09 +0000
From: Hariprasad Kelam <hkelam@...vell.com>
To: Daniel Borkmann <daniel@...earbox.net>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
CC: "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        David Bauer
	<mail@...id-bauer.net>, Ido Schimmel <idosch@...dia.com>,
        Nikolay Aleksandrov
	<razor@...ckwall.org>,
        Martin KaFai Lau <martin.lau@...nel.org>
Subject: [PATCH net] vxlan: Fix regression when dropping packets due to
 invalid src addresses



> Commit f58f45c1e5b9 ("vxlan: drop packets from invalid src-address") has
> been recently added to vxlan mainly in the context of source address
> snooping/learning so that when it is enabled, an entry in the FDB is not being
> created for an invalid address for the tunnel endpoint.
> 
> Before commit f58f45c1e5b9 vxlan was similarly behaving as geneve in that it
> passed through whichever macs were set in the L2 header. It turns out that
> this change in behavior breaks setups, for example, Cilium with netkit in L3
> mode for Pods as well as tunnel mode has been passing before the change in
> f58f45c1e5b9 for both vxlan and geneve.
> After mentioned change it is only passing for geneve as in case of vxlan
> packets are dropped due to vxlan_set_mac() returning false as source and
> destination macs are zero which for E/W traffic via tunnel is totally fine.
> 
> Fix it by only opting into the is_valid_ether_addr() check in
> vxlan_set_mac() when in fact source address snooping/learning is actually
> enabled in vxlan. With this change, the Cilium connectivity test suite passes
> again for both tunnel flavors.
> 
> Fixes: f58f45c1e5b9 ("vxlan: drop packets from invalid src-address")
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> Cc: David Bauer <mail@...id-bauer.net>
> Cc: Ido Schimmel <idosch@...dia.com>
> Cc: Nikolay Aleksandrov <razor@...ckwall.org>
> Cc: Martin KaFai Lau <martin.lau@...nel.org>
> ---
>  drivers/net/vxlan/vxlan_core.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index f78dd0438843..7353f27b02dc 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1605,6 +1605,7 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan,
>  			  struct vxlan_sock *vs,
>  			  struct sk_buff *skb, __be32 vni)
>  {
> +	bool learning = vxlan->cfg.flags & VXLAN_F_LEARN;
>  	union vxlan_addr saddr;
>  	u32 ifindex = skb->dev->ifindex;
>   

  Not related to this change,  can you adjust existing declaration align to reverse X-mas tree?

Thanks,
Hariprasad k


> @@ -1616,8 +1617,11 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan,
>  	if (ether_addr_equal(eth_hdr(skb)->h_source, vxlan->dev-
> >dev_addr))
>  		return false;
> 
> -	/* Ignore packets from invalid src-address */
> -	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
> +	/* Ignore packets from invalid src-address when in learning mode,
> +	 * otherwise let them through e.g. when originating from NOARP
> +	 * devices with all-zero mac, etc.
> +	 */
> +	if (learning && !is_valid_ether_addr(eth_hdr(skb)->h_source))
>  		return false;
> 
>  	/* Get address from the outer IP header */ @@ -1631,7 +1635,7 @@
> static bool vxlan_set_mac(struct vxlan_dev *vxlan,  #endif
>  	}
> 
> -	if ((vxlan->cfg.flags & VXLAN_F_LEARN) &&
> +	if (learning &&
>  	    vxlan_snoop(skb->dev, &saddr, eth_hdr(skb)->h_source, ifindex,
> vni))
>  		return false;
> 
> --
> 2.34.1
>