[PATCH net v3 1/1] rxrpc: fix encap_rcv skb accounting exhaustion

[zihan xi]

On Tue, Sep 8, 2026 at 8:17 PM Simon Horman <horms at kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/a2bd6f3ed29919a7328363b548299ae583f052b0.1788539302.git.zihanx@nebusec.ai?part=1
> ---
> > diff --git a/net/rxrpc/io_thread.c b/net/rxrpc/io_thread.c
> > --- a/net/rxrpc/io_thread.c
> > +++ b/net/rxrpc/io_thread.c
> > @@ -52,6 +50,19 @@ int rxrpc_encap_rcv(struct sock *udp_sk, struct sk_buff *skb)
> >       }
> >  #endif
> >
> > +     if (atomic_read(&udp_sk->sk_rmem_alloc) >= READ_ONCE(udp_sk->sk_rcvbuf) ||
> > +         !sk_rmem_schedule(udp_sk, skb, skb->truesize)) {
> > +             sk_drops_inc(udp_sk);
> > +             kfree_skb(skb);
> > +             return 0;
> > +     }
> > +
> > +     skb->dev = NULL;
> > +     skb_set_owner_r(skb, udp_sk);
> > +     skb_dst_force(skb);
> > +
> > +     skb->mark = RXRPC_SKB_MARK_PACKET;
> > +     rxrpc_new_skb(skb, rxrpc_skb_new_encap_rcv);
> >       skb_queue_tail(rx_queue, skb);
> >       wake_up_process(io_thread);
> >       return 0;
>
> [Severity: High]
> This borrows UDP's rmem accounting without borrowing the lock that
> protects it.
>
> rxrpc_encap_rcv() is called from the UDP receive path in softirq
> context with neither lock_sock(udp_sk) nor
> udp_sk->sk_receive_queue.lock held, and nothing serialises two CPUs
> delivering to the same local endpoint. Both new accounting calls in
> this hunk perform an unsynchronised read-modify-write on
> udp_sk->sk_forward_alloc: sk_rmem_schedule() reaches
> __sk_mem_schedule()/sk_forward_alloc_add(), and skb_set_owner_r()
> calls sk_mem_charge(), which is the same non-atomic
> WRITE_ONCE(sk->sk_forward_alloc, ...) update. The newly added
> skb_orphan() in rxrpc_io_thread() closes the loop from the kthread
> side: sock_rfree() -> sk_mem_uncharge() does the same non-atomic
> update concurrently with softirq enqueues on other CPUs. Note that
> only sk_rmem_alloc is atomic_t here; sk_forward_alloc is not, so the
> atomic_read() guard at the top of the hunk does not make any of this
> safe.
>
> The normal UDP path does not have this problem because
> __udp_enqueue_schedule_skb() performs the forward-alloc bookkeeping
> and the skb_set_owner_r() under sk->sk_receive_queue.lock, and
> udp_rmem_release() takes the same lock before reclaiming. The
> consequence of losing that serialisation is not confined to this
> socket: the drifted sk_forward_alloc is eventually handed to
> __sk_mem_reduce_allocated(), which subtracts it from the protocol-wide
> udp_prot memory_allocated counter, so a flood of RxRPC-shaped packets
> can skew global UDP memory accounting for every UDP socket on the
> machine. That is a worse failure mode than the unbounded rx_queue this
> patch is trying to fix.
>
> How do you want to serialise this? Options I can see are enqueueing
> via the existing UDP receive-queue locking rather than open-coding the
> charge, or holding local->rx_queue.lock across both the charge in
> rxrpc_encap_rcv() and the skb_orphan() in rxrpc_io_thread() so the
> charge and uncharge sides are mutually exclusive. Please say which you
> intend, and make sure the answer covers the I/O-thread uncharge path
> too, not just the two enqueue-side calls.

Thanks for the review. I agree this is racy: sk_forward_alloc is not
atomic, so a BH encap_rcv() on another CPU can update it while the
I/O thread orphans the skb.

I intend to keep the skbs on the RxRPC local queue and serialise UDP
rmem with sk->sk_receive_queue.lock, the same lock UDP uses:

- rxrpc_encap_rcv() already runs in BH, so it can take spin_lock()
  around sk_rmem_schedule() and skb_set_owner_r();
- the I/O thread takes spin_lock_bh() around skb_orphan() for PACKET
  skbs charged in encap_rcv(), so a concurrent BH charge cannot run
  at the same time.

I will not enqueue these packets on the UDP receive queue.

A v4 addressing this is coming shortly.