netdev - Re: [PATCH net 4/4] mlx5: Don't accidentally set RTO_ONLINK before mlx5e_route_lookup_ipv4

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: <20220106035723.o4emdnfvhipoiz5r@sx1>
Date:   Wed, 5 Jan 2022 19:57:23 -0800
From:   Saeed Mahameed <saeed@...nel.org>
To:     Guillaume Nault <gnault@...hat.com>, roid@...dia.com
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Saeed Mahameed <saeedm@...dia.com>,
        Leon Romanovsky <leon@...nel.org>,
        Vlad Buslov <vladbu@...dia.com>,
        Or Gerlitz <ogerlitz@...lanox.com>
Subject: Re: [PATCH net 4/4] mlx5: Don't accidentally set RTO_ONLINK before
 mlx5e_route_lookup_ipv4_get()

On Wed, Jan 05, 2022 at 08:56:28PM +0100, Guillaume Nault wrote:
>Mask the ECN bits before calling mlx5e_route_lookup_ipv4_get(). The
>tunnel key might have the last ECN bit set. This interferes with the
>route lookup process as ip_route_output_key_hash() interpretes this bit
>specially (to restrict the route scope).
>
>Found by code inspection, compile tested only.
>
>Fixes: c7b9038d8af6 ("net/mlx5e: TC preparation refactoring for routing update event")
>Fixes: 9a941117fb76 ("net/mlx5e: Maximize ip tunnel key usage on the TC offloading path")
>Signed-off-by: Guillaume Nault <gnault@...hat.com>
>---
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
>index a5e450973225..bc5f1dcb75e1 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
>@@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
> /* Copyright (c) 2018 Mellanox Technologies. */
>
>+#include <net/inet_ecn.h>
> #include <net/vxlan.h>
> #include <net/gre.h>
> #include <net/geneve.h>
>@@ -235,7 +236,7 @@ int mlx5e_tc_tun_create_header_ipv4(struct mlx5e_priv *priv,
> 	int err;
>
> 	/* add the IP fields */
>-	attr.fl.fl4.flowi4_tos = tun_key->tos;
>+	attr.fl.fl4.flowi4_tos = tun_key->tos & ~INET_ECN_MASK;

This is TC control path, why would ecn bits be ON in TC act->tunnel info ?
I don't believe these bits are on and if they were, TC tunnels layer should
clear them before calling the driver's offload callback.