[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH net-next v3 08/15] net: skbuff: make skb_try_coalesce()'s len unsigned



skb_try_coalesce() keeps from->len, which is unsigned, in an int.  A
corrupt from->len above INT_MAX turns negative, gets past the
len <= skb_tailroom(to) check, and reaches skb_copy_bits() as a negative
length.  skb_copy_bits() returns -EFAULT for most such skbs.  If
skb_headlen(from) is still a small positive value, skb_copy_bits()
clamps the head copy to the negative length and memcpy() takes that as
2GB or more.

skb_put() used to run before the copy, and its overflow check caught
most of these lengths, but not the ones that wrap skb->tail back below
skb->end.  commit 279877d86c82 ("net: skbuff: don't BUG() when
skb_copy_bits() fails in copy helpers") moved the copy ahead of
skb_put(), which left only the tailroom check.

Make len unsigned, like from->len, so that such an skb doesn't take the
copy path.  It gets to the truesize sanity check instead, which sees
that from->len is more than from->truesize can hold.  That check only
warns, and only after the head of @from has been added to @to as a frag
of that size.  Make the check refuse the coalesce, and move it ahead of
the head steal so that @to isn't touched.  The check compares the int
delta against len, so it now tests delta < 0 explicitly.

An skb from a driver that under-reports truesize used to warn once and
get coalesced.  It isn't coalesced any more.

A test module that coalesces an skb with a page frag head and a len of
0x80000010 gets false and an untouched @to.

Found by the Sashiko AI review of the BUG_ON() series.

Fixes: 279877d86c82 ("net: skbuff: don't BUG() when skb_copy_bits() fails in 
copy helpers")
Fixes: bad43ca8325f ("net: introduce skb_try_coalesce()")
Link: 
https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@xxxxxxxxxxxxxx/
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 net/core/skbuff.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d79d8463c44e..e400990595f8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6478,7 +6478,8 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff 
*from,
                      bool *fragstolen, int *delta_truesize)
 {
        struct skb_shared_info *to_shinfo, *from_shinfo;
-       int i, delta, len = from->len;
+       unsigned int len = from->len;
+       int i, delta;
 
        *fragstolen = false;
 
@@ -6523,9 +6524,6 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff 
*from,
                return false;
 
        if (skb_headlen(from) != 0) {
-               struct page *page;
-               unsigned int offset;
-
                if (to_shinfo->nr_frags +
                    from_shinfo->nr_frags >= MAX_SKB_FRAGS)
                        return false;
@@ -6534,13 +6532,6 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff 
*from,
                        return false;
 
                delta = from->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
-
-               page = virt_to_head_page(from->head);
-               offset = from->data - (unsigned char *)page_address(page);
-
-               skb_fill_page_desc(to, to_shinfo->nr_frags,
-                                  page, offset, skb_headlen(from));
-               *fragstolen = true;
        } else {
                if (to_shinfo->nr_frags +
                    from_shinfo->nr_frags > MAX_SKB_FRAGS)
@@ -6549,7 +6540,19 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff 
*from,
                delta = from->truesize - SKB_TRUESIZE(skb_end_offset(from));
        }
 
-       WARN_ON_ONCE(delta < len);
+       /* @from claims more data than it has memory for.  Leave @to alone. */
+       if (WARN_ON_ONCE(delta < 0 || delta < len))
+               return false;
+
+       if (skb_headlen(from) != 0) {
+               struct page *page = virt_to_head_page(from->head);
+               unsigned int offset;
+
+               offset = from->data - (unsigned char *)page_address(page);
+               skb_fill_page_desc(to, to_shinfo->nr_frags,
+                                  page, offset, skb_headlen(from));
+               *fragstolen = true;
+       }
 
        memcpy(to_shinfo->frags + to_shinfo->nr_frags,
               from_shinfo->frags,

-- 
2.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.