[PATCH net] inet: frags: bound the wmem charge of the socket completing a reassembly

[PATCH net] inet: frags: bound the wmem charge of the socket completing a reassembly

[Eric Dumazet]

When a socket owned fragment completes a reassembly queue,
inet_frag_reasm_prepare() and inet_frag_reasm_finish() charge the
truesize of all other fragments to its sk_wmem_alloc. These fragments
were orphaned when queued, and might have been sent by someone else,
for instance padded TUN packets.

Commit 3ae20e4697eb ("inet: frags: limit the truesize of a reassembly
queue") bounds this charge to INT_MAX / 2 per queue, but a socket can
complete several queues while the reassembled skbs are still alive,
for instance held in a qdisc. sk_wmem_alloc can then go past INT_MAX,
refcount_add() saturates it with a WARN, and the socket is leaked.

This is reachable from an unprivileged user/network namespace, by
raising ipfrag_high_thresh.

Refuse the reassembly if the charge would push sk_wmem_alloc above
2 * sk_sndbuf, the bound ip_append_data() uses, plus 4MB of slack for
fragments inflated by a refragmentation with a small MTU.
SO_SNDBUFFORCE requires CAP_NET_ADMIN in the init user namespace,
so an unprivileged user can not raise sk_sndbuf above 2 * wmem_max.

The check is done before inet_frag_reasm_prepare() modifies the queue.
All callers already kill the queue when it returns NULL.

Fixes: 18685451fc4e ("inet: inet_defrag: prevent sk release while still in use")
Reported-by: Paolo Abeni <pabeni@redhat.com>
Closes: https://lore.kernel.org/netdev/24feda7d-ae49-45af-9596-2cc5f149f28e@redhat.com/
Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv4/inet_fragment.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index 0b8d22ffafca69b2bccd98eca3d4bda93ebb816a..c11d7e976dfa534b773f99f33f47fcc562ed569b 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -17,6 +17,7 @@
 #include <linux/rtnetlink.h>
 #include <linux/slab.h>
 #include <linux/rhashtable.h>
+#include <linux/sizes.h>
 
 #include <net/sock.h>
 #include <net/inet_frag.h>
@@ -519,6 +520,25 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
 }
 EXPORT_SYMBOL(inet_frag_queue_insert);
 
+/* When a socket owned fragment completes a queue, the truesize of all
+ * other fragments is charged to its sk_wmem_alloc, although they were
+ * orphaned when queued and possibly sent by someone else.
+ * Bound this like ip_append_data() bounds the memory a socket can
+ * allocate (2 * sk_sndbuf), plus slack for fragments inflated by a
+ * refragmentation using a small MTU. Otherwise, several reassembled skbs
+ * alive at the same time can saturate sk_wmem_alloc, and leak the socket.
+ */
+#define INET_FRAG_WMEM_SLACK	SZ_4M
+
+static bool inet_frag_wmem_ok(const struct inet_frag_queue *q,
+			      const struct sock *sk)
+{
+	u64 limit = 2ULL * READ_ONCE(sk->sk_sndbuf) + INET_FRAG_WMEM_SLACK;
+	u64 wmem = (u64)refcount_read(&sk->sk_wmem_alloc) + q->truesize;
+
+	return wmem <= min_t(u64, limit, INET_FRAG_MAX_TRUESIZE);
+}
+
 void *inet_frag_reasm_prepare(struct inet_frag_queue *q, struct sk_buff *skb,
 			      struct sk_buff *parent)
 {
@@ -530,6 +550,8 @@ void *inet_frag_reasm_prepare(struct inet_frag_queue *q, struct sk_buff *skb,
 	int delta;
 
 	if (sk && is_skb_wmem(skb)) {
+		if (!inet_frag_wmem_ok(q, sk))
+			return NULL;
 		/* TX: skb->sk might have been passed as argument to
 		 * dst->output and must remain valid until tx completes.
 		 *
-- 
2.53.0

Re: [PATCH net] inet: frags: bound the wmem charge of the socket completing a reassembly

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] inet: frags: bound the wmem charge of the socket completing a reassembly

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] In net/ipv4/inet_fragment.c, inet_frag_wmem_ok() (lines 533-540)
  reads sk->sk_wmem_alloc without any socket-wide lock or reservation…

--

inet: frags: bound the wmem charge of the socket completing a reassembly

This patch adds inet_frag_wmem_ok(). The new helper refuses a reassembly
when charging the queue's truesize to the completing socket would push
sk_wmem_alloc above 2 * sk_sndbuf + 4MB, capped at INET_FRAG_MAX_TRUESIZE.
The aim is to keep sk_wmem_alloc from saturating while several reassembled
skbs owned by the same socket are alive at the same time.

Closes: https://lore.kernel.org/netdev/24feda7d-ae49-45af-9596-2cc5f149f28e@redhat.com/

> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index 0b8d22ffafca6..c11d7e976dfa5 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c

[ ... ]

> @@ -519,6 +520,25 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
>  }
>  EXPORT_SYMBOL(inet_frag_queue_insert);
>  
> +/* When a socket owned fragment completes a queue, the truesize of all
> + * other fragments is charged to its sk_wmem_alloc, although they were
> + * orphaned when queued and possibly sent by someone else.
> + * Bound this like ip_append_data() bounds the memory a socket can
> + * allocate (2 * sk_sndbuf), plus slack for fragments inflated by a
> + * refragmentation using a small MTU. Otherwise, several reassembled skbs
> + * alive at the same time can saturate sk_wmem_alloc, and leak the socket.
> + */
> +#define INET_FRAG_WMEM_SLACK	SZ_4M
> +
> +static bool inet_frag_wmem_ok(const struct inet_frag_queue *q,
> +			      const struct sock *sk)
> +{
> +	u64 limit = 2ULL * READ_ONCE(sk->sk_sndbuf) + INET_FRAG_WMEM_SLACK;
> +	u64 wmem = (u64)refcount_read(&sk->sk_wmem_alloc) + q->truesize;
> +
> +	return wmem <= min_t(u64, limit, INET_FRAG_MAX_TRUESIZE);
> +}

[Severity: High]
Is this check safe when other reassemblies charge the same socket at the
same time?

inet_frag_wmem_ok() reads sk_wmem_alloc without a lock. The charge it
approves is applied later, in two steps:

inet_frag_reasm_prepare()
    refcount_add(ts_delta, &sk->sk_wmem_alloc);

inet_frag_reasm_finish()
    /* after the loop over every fragment */
    refcount_add(sum_truesize - head_truesize, &sk->sk_wmem_alloc);

The only lock held across this is the per-queue lock, for example
spin_lock(&qp->q.lock) in ip_defrag(). The IPv6, nf_conntrack and 6lowpan
callers follow the same pattern. That lock serializes one queue, not all
the queues charging the same socket.

Suppose several CPUs complete different queues for the same socket. Can
they all read the same sk_wmem_alloc value W, all see
W + q->truesize <= limit, and then all add their charge?

The limit is at most INET_FRAG_MAX_TRUESIZE (INT_MAX / 2). So three such
completions racing would seem to be enough to push sk_wmem_alloc past
INT_MAX. That is the saturation and socket leak the commit message
describes.

One possible path from an unprivileged user/net namespace:

tun_chr_open() sets tfile->sk.sk_sndbuf = INT_MAX, so the limit works out
to min(2 * INT_MAX + 4M, INT_MAX / 2) = INT_MAX / 2.

tun_build_skb() and the sock_alloc_send_pskb() path leave the skbs owned
by the tun socket via skb_set_owner_w(), so is_skb_wmem() is true. These
skbs are not orphaned before tc ingress, and tun_get_user() takes no
per-write lock outside the napi_frags mode.

Add clsact plus act_ct on the tun device and raise ipfrag_high_thresh.
Then several threads writing the final fragments of different datagrams
to the same tun fd would each reach this path on their own CPU, each
under a different q->lock:

netif_receive_skb()
  tcf_ct_handle_fragments()
    nf_ct_handle_fragments()
      ip_defrag()
        inet_frag_reasm_prepare()
          inet_frag_wmem_ok()

The window between the check and the second refcount_add() covers
skb_clone(), skb_morph(), skb_unclone() and the whole fragment loop in
inet_frag_reasm_finish().

Would this need an atomic reservation on sk_wmem_alloc instead? For
example, add the charge first and then recheck and roll back, or use a
cmpxchg loop.

> +
>  void *inet_frag_reasm_prepare(struct inet_frag_queue *q, struct sk_buff *skb,
>  			      struct sk_buff *parent)
>  {

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103092957.4164530-1-edumazet%40kernel.org

Re: [PATCH net] inet: frags: bound the wmem charge of the socket completing a reassembly

[Eric Dumazet]

Le mer. 4 nov. 2026 à 10:30, <netdev-bot+sashiko@kernel.org> a écrit :
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] In net/ipv4/inet_fragment.c, inet_frag_wmem_ok() (lines 533-540)
>   reads sk->sk_wmem_alloc without any socket-wide lock or reservation…
>
> --
>
> inet: frags: bound the wmem charge of the socket completing a reassembly
>
> This patch adds inet_frag_wmem_ok(). The new helper refuses a reassembly
> when charging the queue's truesize to the completing socket would push
> sk_wmem_alloc above 2 * sk_sndbuf + 4MB, capped at INET_FRAG_MAX_TRUESIZE.
> The aim is to keep sk_wmem_alloc from saturating while several reassembled
> skbs owned by the same socket are alive at the same time.
>
> Closes: https://lore.kernel.org/netdev/24feda7d-ae49-45af-9596-2cc5f149f28e@redhat.com/
>
> > diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> > index 0b8d22ffafca6..c11d7e976dfa5 100644
> > --- a/net/ipv4/inet_fragment.c
> > +++ b/net/ipv4/inet_fragment.c
>
> [ ... ]
>
> > @@ -519,6 +520,25 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
> >  }
> >  EXPORT_SYMBOL(inet_frag_queue_insert);
> >
> > +/* When a socket owned fragment completes a queue, the truesize of all
> > + * other fragments is charged to its sk_wmem_alloc, although they were
> > + * orphaned when queued and possibly sent by someone else.
> > + * Bound this like ip_append_data() bounds the memory a socket can
> > + * allocate (2 * sk_sndbuf), plus slack for fragments inflated by a
> > + * refragmentation using a small MTU. Otherwise, several reassembled skbs
> > + * alive at the same time can saturate sk_wmem_alloc, and leak the socket.
> > + */
> > +#define INET_FRAG_WMEM_SLACK SZ_4M
> > +
> > +static bool inet_frag_wmem_ok(const struct inet_frag_queue *q,
> > +                           const struct sock *sk)
> > +{
> > +     u64 limit = 2ULL * READ_ONCE(sk->sk_sndbuf) + INET_FRAG_WMEM_SLACK;
> > +     u64 wmem = (u64)refcount_read(&sk->sk_wmem_alloc) + q->truesize;
> > +
> > +     return wmem <= min_t(u64, limit, INET_FRAG_MAX_TRUESIZE);
> > +}
>
> [Severity: High]
> Is this check safe when other reassemblies charge the same socket at the
> same time?
>
> inet_frag_wmem_ok() reads sk_wmem_alloc without a lock. The charge it
> approves is applied later, in two steps:
>
> inet_frag_reasm_prepare()
>     refcount_add(ts_delta, &sk->sk_wmem_alloc);
>
> inet_frag_reasm_finish()
>     /* after the loop over every fragment */
>     refcount_add(sum_truesize - head_truesize, &sk->sk_wmem_alloc);
>
> The only lock held across this is the per-queue lock, for example
> spin_lock(&qp->q.lock) in ip_defrag(). The IPv6, nf_conntrack and 6lowpan
> callers follow the same pattern. That lock serializes one queue, not all
> the queues charging the same socket.
>
> Suppose several CPUs complete different queues for the same socket. Can
> they all read the same sk_wmem_alloc value W, all see
> W + q->truesize <= limit, and then all add their charge?
>
> The limit is at most INET_FRAG_MAX_TRUESIZE (INT_MAX / 2). So three such
> completions racing would seem to be enough to push sk_wmem_alloc past
> INT_MAX. That is the saturation and socket leak the commit message
> describes.
>
> One possible path from an unprivileged user/net namespace:
>
> tun_chr_open() sets tfile->sk.sk_sndbuf = INT_MAX, so the limit works out
> to min(2 * INT_MAX + 4M, INT_MAX / 2) = INT_MAX / 2.
>
> tun_build_skb() and the sock_alloc_send_pskb() path leave the skbs owned
> by the tun socket via skb_set_owner_w(), so is_skb_wmem() is true. These
> skbs are not orphaned before tc ingress, and tun_get_user() takes no
> per-write lock outside the napi_frags mode.
>
> Add clsact plus act_ct on the tun device and raise ipfrag_high_thresh.
> Then several threads writing the final fragments of different datagrams
> to the same tun fd would each reach this path on their own CPU, each
> under a different q->lock:
>
> netif_receive_skb()
>   tcf_ct_handle_fragments()
>     nf_ct_handle_fragments()
>       ip_defrag()
>         inet_frag_reasm_prepare()
>           inet_frag_wmem_ok()
>
> The window between the check and the second refcount_add() covers
> skb_clone(), skb_morph(), skb_unclone() and the whole fragment loop in
> inet_frag_reasm_finish().
>
> Would this need an atomic reservation on sk_wmem_alloc instead? For
> example, add the charge first and then recheck and roll back, or use a
> cmpxchg loop.

Right, and tun sockets default to sk_sndbuf = INT_MAX, so the limit is
INT_MAX/2 there.

I will send v2 with an atomic reservation (cmpxchg) in
inet_frag_reasm_prepare().

pw-bot: cr