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@xxxxxxxxxx> wrote:
>On Tue, Jun 30, 2026 at 5:24 AM Lei Huang <huanglei814@xxxxxxx> wrote:
>>
>> From: Lei Huang <huanglei@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>
>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