netdev - [PATCH net-next] net: ipv4: replace the right route in case prefsrc is used

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231117114837.36100-1-atenart@kernel.org>
Date: Fri, 17 Nov 2023 12:48:36 +0100
From: Antoine Tenart <atenart@...nel.org>
To: davem@...emloft.net,
	kuba@...nel.org,
	pabeni@...hat.com,
	edumazet@...gle.com
Cc: Antoine Tenart <atenart@...nel.org>,
	netdev@...r.kernel.org,
	liuhangbin@...il.com,
	ja@....bg
Subject: [PATCH net-next] net: ipv4: replace the right route in case prefsrc is used

In case similar routes with different prefsrc are installed, any
modification of one of those routes will always modify the first one
found as the prefsrc is not matched. Fix this by updating the entry we
found in case prefsrc was set in the request.

Before the patch:

  $ ip route show
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.2 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.3 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.4 metric 100
  $ ip route change 172.16.42.0/24 dev eth0 proto kernel scope link \
        src 172.16.42.4 metric 100 mtu 1280
  $ ip route show
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.4 metric 100 mtu 1280
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.3 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.4 metric 100

After the patch:

  $ ip route show
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.2 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.3 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.4 metric 100
  $ ip route change 172.16.42.0/24 dev eth0 proto kernel scope link \
        src 172.16.42.4 metric 100 mtu 1280
  $ ip route show
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.2 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.3 metric 100
  172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.4 metric 100 mtu 1280

All fib selftest ran and no failure was seen.

Note: a selftest wasn't added as `ip route` use NLM_F_EXCL which
prevents us from constructing the above routes. But this is a valid
example of what NetworkManager can construct for example.

Signed-off-by: Antoine Tenart <atenart@...nel.org>
---

Hi, comment/question below,

I'm wondering if we want to fix the above case. I made this patch
because we already filter on prefsrc when deleting a route[1] to deal
with the same configurations as above, and that would make the route
replacement consistent with that.

However even with this (same for [1]) things are not 100% failsafe
(and we can argue on the use case and feasibility). For example
consider,

$ ip route show
172.16.42.0/24 dev eth0 proto kernel scope link src 172.16.42.2 metric 100
172.16.42.0/24 dev eth0 proto kernel scope link metric 100
$ ip route del 172.16.42.0/24 dev eth0 proto kernel scope link metric 100
$ ip route show
172.16.42.0/24 dev eth0 proto kernel scope link metric 100

Also the differing part could be something else that the prefsrc (not
that it would necessarily make sense).

Thoughts?

Thanks!
Antoine

[1] 74cb3c108bc0 ("ipv4: match prefsrc when deleting routes").

---
 net/ipv4/fib_trie.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 9bdfdab906fe..6cf775d4574e 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1263,10 +1263,11 @@ int fib_table_insert(struct net *net, struct fib_table *tb,
 
 		nlflags &= ~NLM_F_EXCL;
 
-		/* We have 2 goals:
+		/* We have 3 goals:
 		 * 1. Find exact match for type, scope, fib_info to avoid
 		 * duplicate routes
 		 * 2. Find next 'fa' (or head), NLM_F_APPEND inserts before it
+		 * 3. Find the right 'fa' in case a prefsrc is used
 		 */
 		fa_match = NULL;
 		fa_first = fa;
@@ -1282,6 +1283,9 @@ int fib_table_insert(struct net *net, struct fib_table *tb,
 				fa_match = fa;
 				break;
 			}
+			if (cfg->fc_prefsrc &&
+			    cfg->fc_prefsrc == fa->fa_info->fib_prefsrc)
+				fa_first = fa;
 		}
 
 		if (cfg->fc_nlflags & NLM_F_REPLACE) {
-- 
2.41.0