nexthop: Bounce NHA_GATEWAY in FDB nexthop groups
authorPetr Machata <petrm@nvidia.com>
Thu, 7 Jan 2021 14:48:23 +0000 (16:48 +0200)
committerJakub Kicinski <kuba@kernel.org>
Fri, 8 Jan 2021 02:47:18 +0000 (18:47 -0800)
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@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/ipv4/nexthop.c

index 712cdc061cde930018fa38aa7a9beb7e5b984798..e53e43aef78540a6f48b3355d90c71a064f4f926 100644 (file)
@@ -627,7 +627,7 @@ static int nh_check_attr_group(struct net *net, struct nlattr *tb[],
        for (i = NHA_GROUP_TYPE + 1; i < __NHA_MAX; ++i) {
                if (!tb[i])
                        continue;
-               if (tb[NHA_FDB])
+               if (i == NHA_FDB)
                        continue;
                NL_SET_ERR_MSG(extack,
                               "No other attributes can be set in nexthop groups");