netdev - Re: [PATCH net 3/4] nexthop: Bounce NHA

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]

Date:   Thu, 7 Jan 2021 09:22:37 -0700
From:   David Ahern <dsahern@...il.com>
To:     Ido Schimmel <idosch@...sch.org>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org, petrm@...dia.com,
        roopa@...dia.com, nikolay@...dia.com, mlxsw@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH net 3/4] nexthop: Bounce NHA_GATEWAY in FDB nexthop groups

On 1/7/21 7:48 AM, Ido Schimmel wrote:
> From: Petr Machata <petrm@...dia.com>
> 
> The function nh_check_attr_group() is called to validate nexthop groups.
> The intention of that code seems to have been to bounce all attributes
> above NHA_GROUP_TYPE except for NHA_FDB. However instead it bounces all
> these attributes except when NHA_FDB attribute is present--then it accepts
> them.
> 
> NHA_FDB validation that takes place before, in rtm_to_nh_config(), already
> bounces NHA_OIF, NHA_BLACKHOLE, NHA_ENCAP and NHA_ENCAP_TYPE. Yet further
> back, NHA_GROUPS and NHA_MASTER are bounced unconditionally.
> 
> But that still leaves NHA_GATEWAY as an attribute that would be accepted in
> FDB nexthop groups (with no meaning), so long as it keeps the address
> family as unspecified:
> 
>  # ip nexthop add id 1 fdb via 127.0.0.1
>  # ip nexthop add id 10 fdb via default group 1
> 
> The nexthop code is still relatively new and likely not used very broadly,
> and the FDB bits are newer still. Even though there is a reproducer out
> there, it relies on an improbable gateway arguments "via default", "via
> all" or "via any". Given all this, I believe it is OK to reformulate the
> condition to do the right thing and bounce NHA_GATEWAY.
> 
> Fixes: 38428d68719c ("nexthop: support for fdb ecmp nexthops")
> Signed-off-by: Petr Machata <petrm@...dia.com>
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
> ---
>  net/ipv4/nexthop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@...nel.org>