[PATCH net 1/1] bpf: sockmap: fix tail fragment offset in bpf_msg_push_data

[PATCH net 1/1] bpf: sockmap: fix tail fragment offset in bpf_msg_push_data

[Ren Wei]

From: Yuqi Xu <xuyq21@lenovo.com>

When bpf_msg_push_data() inserts data in the middle of a scatterlist
entry, it splits the original entry into a left fragment and a right
fragment.

The right fragment offset is page-local, but the code advances it with
`start`, which is the message-global insertion point. For inserts into a
non-first SG entry, this over-advances the offset and leaves the split
layout inconsistent.

Advance the right fragment offset by the fragment-local delta,
`start - offset`, which matches the length removed from the front of the
original entry.

Fixes: 6fff607e2f14 ("bpf: sk_msg program helper bpf_msg_push_data")
Cc: stable@kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:GPT-5.4
Signed-off-by: Yuqi Xu <xuyq21@lenovo.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
 net/core/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 80a3b702a2d4..d8e637f35f68 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2860,7 +2860,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
 
 		psge->length = start - offset;
 		rsge.length -= psge->length;
-		rsge.offset += start;
+		rsge.offset += start - offset;
 
 		sk_msg_iter_var_next(i);
 		sg_unmark_end(psge);
-- 
2.54.0

Re: [PATCH net 1/1] bpf: sockmap: fix tail fragment offset in bpf_msg_push_data

[John Fastabend]

On Wed, May 27, 2026 at 11:48:15AM +0800, Ren Wei wrote:
>From: Yuqi Xu <xuyq21@lenovo.com>
>
>When bpf_msg_push_data() inserts data in the middle of a scatterlist
>entry, it splits the original entry into a left fragment and a right
>fragment.
>
>The right fragment offset is page-local, but the code advances it with
>`start`, which is the message-global insertion point. For inserts into a
>non-first SG entry, this over-advances the offset and leaves the split
>layout inconsistent.
>
>Advance the right fragment offset by the fragment-local delta,
>`start - offset`, which matches the length removed from the front of the
>original entry.
>
>Fixes: 6fff607e2f14 ("bpf: sk_msg program helper bpf_msg_push_data")
>Cc: stable@kernel.org
>Reported-by: Yuan Tan <yuantan098@gmail.com>
>Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
>Reported-by: Xin Liu <bird@lzu.edu.cn>
>Assisted-by: Codex:GPT-5.4
>Signed-off-by: Yuqi Xu <xuyq21@lenovo.com>
>Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>---
> net/core/filter.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks.

eviewed-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH net 1/1] bpf: sockmap: fix tail fragment offset in bpf_msg_push_data

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 27 May 2026 11:48:15 +0800 you wrote:
> From: Yuqi Xu <xuyq21@lenovo.com>
> 
> When bpf_msg_push_data() inserts data in the middle of a scatterlist
> entry, it splits the original entry into a left fragment and a right
> fragment.
> 
> The right fragment offset is page-local, but the code advances it with
> `start`, which is the message-global insertion point. For inserts into a
> non-first SG entry, this over-advances the offset and leaves the split
> layout inconsistent.
> 
> [...]

Here is the summary with links:
  - [net,1/1] bpf: sockmap: fix tail fragment offset in bpf_msg_push_data
    https://git.kernel.org/netdev/net/c/f72eed9b84fb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net 1/1] bpf: sockmap: fix tail fragment offset in bpf_msg_push_data

[sashiko-bot]

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

Pre-existing issues:
- [Critical] bpf_msg_push_data allocates memory for new message data without the __GFP_ZERO flag, leaving the newly inserted bytes uninitialized and exposing kernel heap memory.
- [High] When appending data to a full or nearly full sk_msg, bpf_msg_push_data incorrectly reads the length from the inactive msg->sg.end index, bypassing the coalescing logic and causing a ring buffer wrap-around, out-of-bounds access, or information leak.
- [High] Calling bpf_msg_push_data on an empty sk_msg causes the scatterlist iteration loop to traverse inactive ring buffer slots, leading to ring buffer corruption.
- [High] Shifting scatterlist elements in bpf_msg_push_data and bpf_msg_pop_data does not properly shift the msg->sg.copy bitmap, breaking the Copy-On-Write mechanism.
- [High] The len parameter in bpf_msg_push_data is not bounds-checked, allowing a maliciously large len to cause an integer overflow, resulting in a small allocation followed by a massive heap out-of-bounds write.
--

commit fefea84ac268cf25e699730bfbde310ae44b83f8
Author: Yuqi Xu <xuyq21@lenovo.com>

bpf: sockmap: fix tail fragment offset in bpf_msg_push_data

When bpf_msg_push_data() inserts data in the middle of a scatterlist
entry, it splits the original entry into a left fragment and a right
fragment. The right fragment offset is advanced with the message-global
insertion point, which over-advances the offset. This commit advances
the offset by the fragment-local delta instead.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9590877b0714f..80439767e0eea 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2869,7 +2869,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,

[Severity: High]
This isn't a bug introduced by this patch, but when appending data to a full
or nearly full sk_msg, does bpf_msg_push_data incorrectly read the length
from the inactive msg->sg.end index?

If a BPF program appends data to the end of a message, the search loop
terminates with i == msg->sg.end. If the ring buffer is full (space == 0),
copy = msg->sg.data[i].length reads from the inactive slot.

If the stale length is 0, the if (copy) block is skipped. The function then
calls sk_msg_iter_next(msg, end) on a full ring buffer, wrapping msg->sg.end
to equal msg->sg.start, logically emptying the message and leaking all
original pages.

If the stale length is non-zero, it sizes the new allocation incorrectly. If
the previous element's length exceeds the size of the allocation order for
copy + len, the subsequent memcpy will overflow the allocated heap buffer.

[Severity: High]
This is a pre-existing issue, but can calling bpf_msg_push_data on an empty
sk_msg cause the scatterlist iteration loop to traverse inactive ring buffer
slots?

If the message is completely empty (e.g., drained via bpf_msg_pop_data),
msg->sg.start == msg->sg.end.

The do { ... } while (i != msg->sg.end) loop executes at least once because
of the do-while construct. It traverses uninitialized or stale elements,
accumulating stale length values into offset.

If the loop condition start < offset + l fails to break, it iterates through
the entire ring buffer, leaving offset heavily corrupted. This leads to
front = start - offset becoming a large negative number, causing a massive
out-of-bounds memcpy that will reliably panic the kernel.

[Severity: High]
This isn't a bug introduced by this patch, but could shifting scatterlist
elements in bpf_msg_push_data and bpf_msg_pop_data fail to properly shift
the msg->sg.copy bitmap, breaking the Copy-On-Write mechanism?

msg->sg.copy is a bitmap used to track which scatterlist elements are shared
(e.g., from TCP zerocopy) and require a private copy before modification.

When elements are shifted in the ring buffer during bpf_msg_push_data
(shifted right) or bpf_msg_pop_data (shifted left), their corresponding
msg->sg.copy bits are not shifted.

This desynchronization clears the copy protection for elements that moved.
A BPF program calling bpf_msg_pull_data on these elements will be granted
direct write access instead of triggering a copy, allowing it to corrupt
shared memory or page-cache data.

[Severity: Critical]
This is a pre-existing issue, but is bpf_msg_push_data allocating memory
for new message data without the __GFP_ZERO flag?

bpf_msg_push_data allocates new pages via alloc_pages(__GFP_NOWARN |
GFP_ATOMIC | __GFP_COMP, ...). The kernel inserts len bytes into the page
but never initializes this specific region.

A BPF program can request this memory expansion without writing any data to
it. If the sk_msg is subsequently transmitted over a network socket, the
uninitialized kernel heap memory will be sent across the network, leading
to a significant information leak.

[Severity: High]
This isn't a bug introduced by this patch, but is the len parameter in
bpf_msg_push_data missing a bounds check?

The len parameter is a u32 value passed from the BPF program, with no
verifier bounds check (ARG_ANYTHING). An excessively large len
(e.g., 0xFFFFFFFF) causes the 32-bit addition copy + len to overflow to a
small value.

alloc_pages successfully allocates a small buffer. During the fallback copy
logic, the pointer arithmetic to = raw + front + len expands the 32-bit len
to a 64-bit unsigned offset, advancing the destination pointer by ~4GB.

The subsequent memcpy(to, from, back) performs an out-of-bounds write on
the heap, allowing a verified BPF program to corrupt kernel memory and cause
a kernel panic.

>  
>  		psge->length = start - offset;
>  		rsge.length -= psge->length;
> -		rsge.offset += start;
> +		rsge.offset += start - offset;
>  
>  		sk_msg_iter_var_next(i);
>  		sg_unmark_end(psge);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/8b129d10566aa3eb43f61a8f9757bcf51707d324.1779636774.git.xuyq21@lenovo.com?part=1