[PATCH] net: ipv4: fix TOCTOU race in __ip_do_redirect

[PATCH] net: ipv4: fix TOCTOU race in __ip_do_redirect

[Lei Huang]

From: Lei Huang <huanglei@kylinos.cn>

fib_lookup() internally acquires and releases rcu_read_lock and always uses
FIB_LOOKUP_NOREF (no refcount on fib_info). After it returns, res (a local
struct fib_result on the stack) has its nhc field pointing into the
fib_info internal nexthop array, but RCU protection is already dropped.
A concurrent route deletion can free the fib_info via kfree_rcu, making
res.nhc a stale pointer. Subsequent FIB_RES_NHC(res) reads this stale value
and update_or_create_fnhe() dereferences it, causing UAF.

Fix by wrap the entire fib_lookup + FIB_RES_NHC + update_or_create_fnhe
region in an explicit rcu_read_lock/unlock to keep the fib_info alive
throughout the critical section.

Signed-off-by: Lei Huang <huanglei@kylinos.cn>
---
 net/ipv4/route.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 3f3de5164d6e..86f4b6325050 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -793,6 +793,7 @@ static void __ip_do_redirect(struct rtable *rt, struct sk_buff *skb, struct flow
 		if (!(READ_ONCE(n->nud_state) & NUD_VALID)) {
 			neigh_event_send(n, NULL);
 		} else {
+			rcu_read_lock();
 			if (fib_lookup(net, fl4, &res, 0) == 0) {
 				struct fib_nh_common *nhc;
 
@@ -802,6 +803,7 @@ static void __ip_do_redirect(struct rtable *rt, struct sk_buff *skb, struct flow
 						0, false,
 						jiffies + ip_rt_gc_timeout);
 			}
+			rcu_read_unlock();
 			if (kill_route)
 				WRITE_ONCE(rt->dst.obsolete, DST_OBSOLETE_KILL);
 			call_netevent_notifiers(NETEVENT_NEIGH_UPDATE, n);
-- 
2.25.1

Re: [PATCH] net: ipv4: fix TOCTOU race in __ip_do_redirect

[Eric Dumazet]

On Tue, Jun 30, 2026 at 5:24 AM Lei Huang <huanglei814@163.com> wrote:
>
> From: Lei Huang <huanglei@kylinos.cn>
>
> fib_lookup() internally acquires and releases rcu_read_lock and always uses
> FIB_LOOKUP_NOREF (no refcount on fib_info). After it returns, res (a local
> struct fib_result on the stack) has its nhc field pointing into the
> fib_info internal nexthop array, but RCU protection is already dropped.
> A concurrent route deletion can free the fib_info via kfree_rcu, making
> res.nhc a stale pointer. Subsequent FIB_RES_NHC(res) reads this stale value
> and update_or_create_fnhe() dereferences it, causing UAF.
>
> Fix by wrap the entire fib_lookup + FIB_RES_NHC + update_or_create_fnhe
> region in an explicit rcu_read_lock/unlock to keep the fib_info alive
> throughout the critical section.
>
> Signed-off-by: Lei Huang <huanglei@kylinos.cn>

You forgot to include a Fixes: tag.

Please read Documentation/process/maintainer-netdev.rst

Anyway, this patch isn't needed; all callers of this helper already
use rcu_read_lock().

I am guessing all of them are called from ip_protocol_deliver_rcu()

If you think about this, LOCKDEP would have fired a warning years ago
at line 769:

in_dev = __in_dev_get_rcu(dev);


pw-bot: cr

Re:Re: [PATCH] net: ipv4: fix TOCTOU race in __ip_do_redirect

[huanglei]

Thank you for the review. You are absolutely right.

I traced the callers and confirmed that fib_compute_spec_dst() and __ip_do_redirect() are both invoked from protocol handlers called via ip_protocol_deliver_rcu() in ip_local_deliver_finish(), which holds rcu_read_lock() across the entire dispatch. The fib_lookup() internal rcu_read_lock/unlock is only nesting, so res.fi and res.nhc remain protected by the outer lock after fib_lookup() returns.

Thanks again for the correction.


At 2026-06-30 20:31:09, "Eric Dumazet" <edumazet@google.com> wrote:
>On Tue, Jun 30, 2026 at 5:24 AM Lei Huang <huanglei814@163.com> wrote:
>>
>> From: Lei Huang <huanglei@kylinos.cn>
>>
>> fib_lookup() internally acquires and releases rcu_read_lock and always uses
>> FIB_LOOKUP_NOREF (no refcount on fib_info). After it returns, res (a local
>> struct fib_result on the stack) has its nhc field pointing into the
>> fib_info internal nexthop array, but RCU protection is already dropped.
>> A concurrent route deletion can free the fib_info via kfree_rcu, making
>> res.nhc a stale pointer. Subsequent FIB_RES_NHC(res) reads this stale value
>> and update_or_create_fnhe() dereferences it, causing UAF.
>>
>> Fix by wrap the entire fib_lookup + FIB_RES_NHC + update_or_create_fnhe
>> region in an explicit rcu_read_lock/unlock to keep the fib_info alive
>> throughout the critical section.
>>
>> Signed-off-by: Lei Huang <huanglei@kylinos.cn>
>
>You forgot to include a Fixes: tag.
>
>Please read Documentation/process/maintainer-netdev.rst
>
>Anyway, this patch isn't needed; all callers of this helper already
>use rcu_read_lock().
>
>I am guessing all of them are called from ip_protocol_deliver_rcu()
>
>If you think about this, LOCKDEP would have fired a warning years ago
>at line 769:
>
>in_dev = __in_dev_get_rcu(dev);
>
>
>pw-bot: cr