|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |