[PATCH net v2] fib_semantics: Don't match route with mismatching tclassid

[PATCH net v2] fib_semantics: Don't match route with mismatching tclassid

[Stefano Brivio]

In fib_nh_match(), if output interface or gateway are passed in
the FIB configuration, we don't have to check next hops of
multipath routes to conclude whether we have a match or not.

However, we might still have routes with different realms
matching the same output interface and gateway configuration,
and this needs to cause the match to fail. Otherwise the first
route inserted in the FIB will match, regardless of the realms:

 # ip route add 1.1.1.1 dev eth0 table 1234 realms 1/2
 # ip route append 1.1.1.1 dev eth0 table 1234 realms 3/4
 # ip route list table 1234
 1.1.1.1 dev eth0 scope link realms 1/2
 1.1.1.1 dev eth0 scope link realms 3/4
 # ip route del 1.1.1.1 dev ens3 table 1234 realms 3/4
 # ip route list table 1234
 1.1.1.1 dev ens3 scope link realms 3/4

whereas route with realms 3/4 should have been deleted instead.

Explicitly check for fc_flow passed in the FIB configuration
(this comes from RTA_FLOW extracted by rtm_to_fib_config()) and
fail matching if it differs from nh_tclassid.

The handling of RTA_FLOW for multipath routes later in
fib_nh_match() is still needed, as we can have multiple RTA_FLOW
attributes that need to be matched against the tclassid of each
next hop.

v2: Check that fc_flow is set before discarding the match, so
    that the user can still select the first matching rule by
    not specifying any realm, as suggested by David Ahern.

Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 net/ipv4/fib_semantics.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index c586597da20d..7d36a950d961 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -646,6 +646,11 @@ int fib_nh_match(struct fib_config *cfg, struct fib_info *fi,
 					    fi->fib_nh, cfg, extack))
 				return 1;
 		}
+#ifdef CONFIG_IP_ROUTE_CLASSID
+		if (cfg->fc_flow &&
+		    cfg->fc_flow != fi->fib_nh->nh_tclassid)
+			return 1;
+#endif
 		if ((!cfg->fc_oif || cfg->fc_oif == fi->fib_nh->nh_oif) &&
 		    (!cfg->fc_gw  || cfg->fc_gw == fi->fib_nh->nh_gw))
 			return 0;
-- 
2.15.1

Re: [PATCH net v2] fib_semantics: Don't match route with mismatching tclassid

[David Ahern]

On 2/15/18 1:46 AM, Stefano Brivio wrote:
> In fib_nh_match(), if output interface or gateway are passed in
> the FIB configuration, we don't have to check next hops of
> multipath routes to conclude whether we have a match or not.
> 
> However, we might still have routes with different realms
> matching the same output interface and gateway configuration,
> and this needs to cause the match to fail. Otherwise the first
> route inserted in the FIB will match, regardless of the realms:
> 
>  # ip route add 1.1.1.1 dev eth0 table 1234 realms 1/2
>  # ip route append 1.1.1.1 dev eth0 table 1234 realms 3/4
>  # ip route list table 1234
>  1.1.1.1 dev eth0 scope link realms 1/2
>  1.1.1.1 dev eth0 scope link realms 3/4
>  # ip route del 1.1.1.1 dev ens3 table 1234 realms 3/4
>  # ip route list table 1234
>  1.1.1.1 dev ens3 scope link realms 3/4
> 
> whereas route with realms 3/4 should have been deleted instead.
> 
> Explicitly check for fc_flow passed in the FIB configuration
> (this comes from RTA_FLOW extracted by rtm_to_fib_config()) and
> fail matching if it differs from nh_tclassid.
> 
> The handling of RTA_FLOW for multipath routes later in
> fib_nh_match() is still needed, as we can have multiple RTA_FLOW
> attributes that need to be matched against the tclassid of each
> next hop.
> 
> v2: Check that fc_flow is set before discarding the match, so
>     that the user can still select the first matching rule by
>     not specifying any realm, as suggested by David Ahern.
> 
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  net/ipv4/fib_semantics.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Acked-by: David Ahern <dsahern@gmail.com>

Re: [PATCH net v2] fib_semantics: Don't match route with mismatching tclassid

[David Miller]

From: Stefano Brivio <sbrivio@redhat.com>
Date: Thu, 15 Feb 2018 09:46:03 +0100

> In fib_nh_match(), if output interface or gateway are passed in
> the FIB configuration, we don't have to check next hops of
> multipath routes to conclude whether we have a match or not.
> 
> However, we might still have routes with different realms
> matching the same output interface and gateway configuration,
> and this needs to cause the match to fail. Otherwise the first
> route inserted in the FIB will match, regardless of the realms:
> 
>  # ip route add 1.1.1.1 dev eth0 table 1234 realms 1/2
>  # ip route append 1.1.1.1 dev eth0 table 1234 realms 3/4
>  # ip route list table 1234
>  1.1.1.1 dev eth0 scope link realms 1/2
>  1.1.1.1 dev eth0 scope link realms 3/4
>  # ip route del 1.1.1.1 dev ens3 table 1234 realms 3/4
>  # ip route list table 1234
>  1.1.1.1 dev ens3 scope link realms 3/4
> 
> whereas route with realms 3/4 should have been deleted instead.
> 
> Explicitly check for fc_flow passed in the FIB configuration
> (this comes from RTA_FLOW extracted by rtm_to_fib_config()) and
> fail matching if it differs from nh_tclassid.
> 
> The handling of RTA_FLOW for multipath routes later in
> fib_nh_match() is still needed, as we can have multiple RTA_FLOW
> attributes that need to be matched against the tclassid of each
> next hop.
> 
> v2: Check that fc_flow is set before discarding the match, so
>     that the user can still select the first matching rule by
>     not specifying any realm, as suggested by David Ahern.
> 
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Applied and queued up for -stable, thanks.