Thread (3 messages) 3 messages, 2 authors, 24d ago

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

From: huanglei <hidden>
Date: 2026-07-01 03:13:16
Also in: lkml

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" [off-list ref] wrote:
On Tue, Jun 30, 2026 at 5:24 AM Lei Huang [off-list ref] wrote:
quoted
From: Lei Huang <redacted>

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 <redacted>
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help