Re: [PATCH net v3] tcp: free saved_syn after an RCU grace period

[Eric Dumazet]

Le mer. 4 nov. 2026 à 13:32, Yingjie Wang <x-professor@xxxxxxxxxxx> a écrit :
>
> tcp_reqsk_record_syn() allocates saved_syn separately from its request
> socket. bpf_sock_ops_get_syn() can load that pointer while processing a
> retransmitted SYN under RCU.
>
> There are two ownership transfers which can let another CPU reclaim the
> allocation while such a reader is still using it.
>
> First, tcp_move_syn() transfers req->saved_syn to a newly created child
> socket. The child can be closed and tcp_saved_syn_free() can reclaim the
> allocation before a reader of the old request finishes.
>
> Second, inet_reqsk_clone() shallow-copies request_sock during request
> migration. After reqsk_migrate_reset() clears the old request pointer, the
> migrated clone owns it. However, a reader which already loaded the pointer
> from the old request can still be using it when the clone is released.
>
> I found this through LLM-assisted concurrency fuzzing.
>
> Mark the request-socket saved_syn pointer as RCU protected and use RCU
> accessors for its reads and updates. Keep the full-socket pointer plain
> because all of its accesses hold the socket lock. Embed an rcu_head in
> struct saved_syn and use kfree_rcu() in both teardown paths. This lets a
> request-side reader which loaded the object before an ownership transfer
> finish.
>
> This follows commit e3f33b0a1d89 ("ipv4: free inet_opt and ireq_opt
> after an RCU grace period"), which fixed the same ownership-transfer
> pattern for IPv4 options.
>
> The issue was confirmed with KASAN on v7.3-16007-g36aad21d50d1 using
> temporary kernel instrumentation to widen the reader and request-migration
> window. KASAN reported a slab-use-after-free read in
> bpf_sock_ops_get_syn(), with the allocation freed by __reqsk_free() from
> reqsk_timer_handler(). The same explicit migration test completed without
> a report with deferred reclamation. A patched kernel without the delay
> instrumentation also completed a successful migration and seven saved-SYN
> reads without a KASAN report.
>
> Fixes: 0813a841566f ("bpf: tcp: Allow bpf prog to write and parse TCP header option")
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Yingjie Wang <x-professor@xxxxxxxxxxx>
> ---
> Changes in v3:
> - Keep tcp_sock::saved_syn under the socket lock and apply RCU protection
>   only to request_sock::saved_syn, as suggested by Eric Dumazet.
> - Use unrcu_pointer() when transferring or reclaiming the request-side
>   pointer.
> - Describe the discovery as LLM-assisted concurrency fuzzing.
> - Rebase onto net/main commit 241dec6f9ab7.
>
> v2: https://lore.kernel.org/r/3dd45a687afdd0213ee82685c52777e5b0855e20.1793704772.git.x-professor@xxxxxxxxxxx
> - Add RCU annotations and accessors following Kuniyuki Iwashima's review.
>
> v1: https://lore.kernel.org/r/34f32e63b803f9f7faf0b0a5754219e48a986136.1793616950.git.x-professor@xxxxxxxxxxx
>
>  include/linux/tcp.h             | 6 +++---
>  include/net/request_sock.h      | 4 +++-
>  net/core/filter.c               | 2 +-
>  net/ipv4/inet_connection_sock.c | 6 +++---
>  net/ipv4/tcp_input.c            | 2 +-
>  5 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index d18f7bc1e5d0..3ae57ae65ec4 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -618,13 +618,13 @@ static inline void fastopen_queue_tune(struct sock *sk, int backlog)
>  static inline void tcp_move_syn(struct tcp_sock *tp,
>                                 struct request_sock *req)
>  {
> -       tp->saved_syn = req->saved_syn;
> -       req->saved_syn = NULL;
> +       tp->saved_syn = unrcu_pointer(req->saved_syn);
> +       RCU_INIT_POINTER(req->saved_syn, NULL);
>  }
>
>  static inline void tcp_saved_syn_free(struct tcp_sock *tp)
>  {
> -       kfree(tp->saved_syn);
> +       kfree_rcu(tp->saved_syn, rcu);
>         tp->saved_syn = NULL;
>  }
>
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 5a9c826a7092..ddc134f1d15d 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -16,6 +16,7 @@
>  #include <linux/types.h>
>  #include <linux/bug.h>
>  #include <linux/refcount.h>
> +#include <linux/rcupdate.h>

nit: this include is not needed.

Not worth a new version though.

Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>