From: Eric Dumazet edumazet@google.com
[ Upstream commit b62a59c18b692f892dcb8109c1c2e653b2abc95c ]
Use RCU to avoid a pair of atomic operations and a potential UAF on dst_dev()->flags.
Signed-off-by: Eric Dumazet edumazet@google.com Reviewed-by: David Ahern dsahern@kernel.org Link: https://patch.msgid.link/20250828195823.3958522-8-edumazet@google.com Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Explanation and rationale - What it fixes - Eliminates a race that can lead to a use-after-free when reading `dev->flags` from a `dst_entry` without RCU protection. The pre- change pattern `sk_dst_get()` → `dst_dev()` → `dev->flags` → `dst_release()` can observe a freed `struct net_device` and dereference `dev->flags`, risking UAF. - The change uses RCU to safely dereference the route device and avoid the refcount pair on `dst` (performance benefit is secondary to correctness).
- Code specifics - Affected function: `net/ipv4/tcp_fastopen.c:559` (tcp_fastopen_active_disable_ofo_check) - Before (conceptually): `dst = sk_dst_get(sk); dev = dst ? dst_dev(dst) : NULL; if (!(dev && (dev->flags & IFF_LOOPBACK))) atomic_set(..., 0); dst_release(dst);` - Problem: `dev->flags` is read without RCU or a device reference; `struct net_device` is RCU-freed, so this can race and UAF. - After: - `rcu_read_lock();` - `dst = __sk_dst_get(sk);` (RCU-protected view of `sk->sk_dst_cache`; `include/net/sock.h:2142`) - `dev = dst ? dst_dev_rcu(dst) : NULL;` (RCU-safe deref of device; `include/net/dst.h:574`) - `if (!(dev && (dev->flags & IFF_LOOPBACK))) atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);` - `rcu_read_unlock();` - See current code at `net/ipv4/tcp_fastopen.c:581` for the RCU pattern. - The function is invoked in normal teardown paths, so it can be hit in practice: - `net/ipv4/tcp_ipv4.c:2570` - `net/ipv4/tcp.c:3382`
- Scope and risk - Small, contained change in a single function, no ABI changes, no architectural refactors. - Only affects active TCP Fast Open logic when clearing the global backoff counter on non-loopback devices. - Behavior is unchanged except making the device lookup and flag read concurrency-safe and cheaper (no `dst` refcount inc/dec). - Reading `IFF_LOOPBACK` under RCU is safe; the bit is effectively stable for the loopback device, and RCU guarantees pointer lifetime during the check.
- Stable backport fit - Fixes a real concurrency/UAF bug that can crash the kernel; it’s not a feature change. - Minimal risk of regression and confined to TCP/TFO. - Uses widely available helpers: - `__sk_dst_get()` at `include/net/sock.h:2142` - `dst_dev_rcu()` at `include/net/dst.h:574` - If an older stable branch lacked `dst_dev_rcu()`, the change is trivially adaptable using `rcu_dereference(dst->dev)` under `rcu_read_lock()`. But in maintained series this helper is already present in the networking core.
- Why it matters - Even if exploitation is unlikely (requires racing TFO teardown with route/device changes), it’s a correctness and reliability fix in a core network path and should be in stable trees.
Conclusion - This is a clear bug fix for a potential UAF with a minimal, localized RCU conversion. It aligns with stable criteria and should be backported.
net/ipv4/tcp_fastopen.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c index f1884f0c9e523..7d945a527daf0 100644 --- a/net/ipv4/tcp_fastopen.c +++ b/net/ipv4/tcp_fastopen.c @@ -576,11 +576,12 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk) } } else if (tp->syn_fastopen_ch && atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) { - dst = sk_dst_get(sk); - dev = dst ? dst_dev(dst) : NULL; + rcu_read_lock(); + dst = __sk_dst_get(sk); + dev = dst ? dst_dev_rcu(dst) : NULL; if (!(dev && (dev->flags & IFF_LOOPBACK))) atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0); - dst_release(dst); + rcu_read_unlock(); } }