[PATCH bpf] bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

[PATCH bpf] bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

[Sechang Lim]

start and len are u32, so

	u64 last = start + len;

evaluates start + len in 32-bit and wraps before storing it in last.
The bounds check

	if (start >= offset + l || last > msg->sg.size)
		return -EINVAL;

can then be passed with an out-of-range start/len, after which the pop
loop runs off the end of the scatterlist and sk_msg_shift_left() calls
put_page() on the empty msg->sg.end slot:

  Oops: general protection fault, probably for non-canonical address
  0xdffffc0000000001: 0000 [#1] SMP KASAN PTI
  KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
  RIP: 0010:sk_msg_shift_left net/core/filter.c:2957 [inline]
  RIP: 0010:____bpf_msg_pop_data net/core/filter.c:3103 [inline]
  RIP: 0010:bpf_msg_pop_data+0x753/0x1a10 net/core/filter.c:2984
  Call Trace:
   <TASK>
   bpf_prog_4cc92c278f4d5d56+0x1b1/0x1e8
   bpf_prog_run_pin_on_cpu+0x107/0x320 include/linux/filter.h:746
   sk_psock_msg_verdict+0x357/0x7f0 net/core/skmsg.c:934
   tcp_bpf_send_verdict net/ipv4/tcp_bpf.c:420 [inline]
   tcp_bpf_sendmsg+0x766/0x1ae0 net/ipv4/tcp_bpf.c:583
   __sock_sendmsg+0x153/0x1c0 net/socket.c:802
   __sys_sendto+0x326/0x430 net/socket.c:2265
   __x64_sys_sendto+0xe3/0x100 net/socket.c:2268
   do_syscall_64+0x14c/0x480
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

Fixes: 7246d8ed4dcc ("bpf: helper to pop data from messages")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
 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 80439767e0ee..d6d2fc589218 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2974,7 +2974,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
 	   u32, len, u64, flags)
 {
 	u32 i = 0, l = 0, space, offset = 0;
-	u64 last = start + len;
+	u64 last = (u64)start + len;
 	int pop;
 
 	if (unlikely(flags))
-- 
2.43.0

Re: [PATCH bpf] bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

[sashiko-bot]

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

Pre-existing issues:
- [High] Unbounded `len` parameter in `bpf_msg_push_data` causes a 32-bit integer overflow when calculating the page allocation size (`copy + len`), resulting in an out-of-bounds heap write.
- [High] Incorrect scatterlist element offset calculation when splitting an element in `bpf_msg_push_data` inflates the page offset, leading to out-of-bounds memory accesses.
--

commit 619f5b1d1aebdbd6457c3ac6e3a92bc45be7278c
Author: Sechang Lim <rhkrqnwk98@gmail.com>

bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

start and len are u32, so start + len evaluates in 32-bit and wraps before
storing it in last. The bounds check can then be passed with an out-of-range
start/len.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9590877b0714f..31467c0b4279e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2974,7 +2974,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
>  	   u32, len, u64, flags)
>  {
>  	u32 i = 0, l = 0, space, offset = 0;
> -	u64 last = start + len;
> +	u64 last = (u64)start + len;

[Severity: High]
This isn't a bug introduced by this patch, but while fixing the integer
overflow in bpf_msg_pop_data(), could a similar overflow occur in
bpf_msg_push_data()?

Looking at net/core/filter.c:bpf_msg_push_data(), both copy and len are u32:

	page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
			   get_order(copy + len));

If a large len is provided, copy + len could wrap around to a small value,
resulting in a much smaller allocation than required.

Later in the same function:

			to = raw + front + len;

			memcpy(to, from, back);

Since len is zero-extended to a 64-bit pointer offset, to evaluates to an
address far outside the allocated page. Could this result in an out-of-bounds
write if a BPF program provides a maliciously large len?

[Severity: High]
This is another pre-existing issue in bpf_msg_push_data(), but does the
scatterlist element split calculate the offset correctly?

When splitting an element into psge and rsge:

		psge->length = start - offset;
		rsge.length -= psge->length;
		rsge.offset += start;

Is start the absolute byte offset from the beginning of the entire message?
If the split element is not the first one in the message, wouldn't rsge.offset
be inflated by the cumulative length of all previous elements instead of the
relative offset (start - offset)?

When the network stack eventually reads or writes to this element via sg_virt(),
could this cause an out-of-bounds access past the end of the physical page?

>  	int pop;
>  
>  	if (unlikely(flags))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609183927.4021802-1-rhkrqnwk98@gmail.com?part=1

Re: [PATCH bpf] bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

[Cong Wang]

On Tue, Jun 09, 2026 at 06:39:26PM +0000, Sechang Lim wrote:
> start and len are u32, so
> 
> 	u64 last = start + len;
> 
> evaluates start + len in 32-bit and wraps before storing it in last.
> The bounds check
> 
> 	if (start >= offset + l || last > msg->sg.size)
> 		return -EINVAL;
> 
> can then be passed with an out-of-range start/len, after which the pop
> loop runs off the end of the scatterlist and sk_msg_shift_left() calls
> put_page() on the empty msg->sg.end slot:
> 
>   Oops: general protection fault, probably for non-canonical address
>   0xdffffc0000000001: 0000 [#1] SMP KASAN PTI
>   KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
>   RIP: 0010:sk_msg_shift_left net/core/filter.c:2957 [inline]
>   RIP: 0010:____bpf_msg_pop_data net/core/filter.c:3103 [inline]
>   RIP: 0010:bpf_msg_pop_data+0x753/0x1a10 net/core/filter.c:2984
>   Call Trace:
>    <TASK>
>    bpf_prog_4cc92c278f4d5d56+0x1b1/0x1e8
>    bpf_prog_run_pin_on_cpu+0x107/0x320 include/linux/filter.h:746
>    sk_psock_msg_verdict+0x357/0x7f0 net/core/skmsg.c:934
>    tcp_bpf_send_verdict net/ipv4/tcp_bpf.c:420 [inline]
>    tcp_bpf_sendmsg+0x766/0x1ae0 net/ipv4/tcp_bpf.c:583
>    __sock_sendmsg+0x153/0x1c0 net/socket.c:802
>    __sys_sendto+0x326/0x430 net/socket.c:2265
>    __x64_sys_sendto+0xe3/0x100 net/socket.c:2268
>    do_syscall_64+0x14c/0x480
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    </TASK>

If you have a reproducer, please consider adding a selftest either
together this patch or as a follow-up.

Thanks,
Cong

Re: [PATCH bpf] bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

[Emil Tsalapatis]

On Tue Jun 9, 2026 at 2:52 PM EDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Unbounded `len` parameter in `bpf_msg_push_data` causes a 32-bit integer overflow when calculating the page allocation size (`copy + len`), resulting in an out-of-bounds heap write.
> - [High] Incorrect scatterlist element offset calculation when splitting an element in `bpf_msg_push_data` inflates the page offset, leading to out-of-bounds memory accesses.
> --
>
> commit 619f5b1d1aebdbd6457c3ac6e3a92bc45be7278c
> Author: Sechang Lim <rhkrqnwk98@gmail.com>

The change seems solid, the error is apparent from code inspection
alone (though a repro wouldn't hurt). Can you also turn pop from an int
to a u32? It should never be negative AFAICT, and we assign to it from
a u32 so since we're fixing int types...

>
> bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check
>
> start and len are u32, so start + len evaluates in 32-bit and wraps before
> storing it in last. The bounds check can then be passed with an out-of-range
> start/len.
>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 9590877b0714f..31467c0b4279e 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -2974,7 +2974,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
>>  	   u32, len, u64, flags)
>>  {
>>  	u32 i = 0, l = 0, space, offset = 0;
>> -	u64 last = start + len;
>> +	u64 last = (u64)start + len;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but while fixing the integer
> overflow in bpf_msg_pop_data(), could a similar overflow occur in
> bpf_msg_push_data()?
>
> Looking at net/core/filter.c:bpf_msg_push_data(), both copy and len are u32:
>
> 	page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
> 			   get_order(copy + len));
>

This seems like a valid concern. Can you add it to the patch? Making
copy a u64 should do it.

> If a large len is provided, copy + len could wrap around to a small value,
> resulting in a much smaller allocation than required.
>
> Later in the same function:
>
> 			to = raw + front + len;
>
> 			memcpy(to, from, back);
>
> Since len is zero-extended to a 64-bit pointer offset, to evaluates to an
> address far outside the allocated page. Could this result in an out-of-bounds
> write if a BPF program provides a maliciously large len?
>

As for hte other:

> [Severity: High]
> This is another pre-existing issue in bpf_msg_push_data(), but does the
> scatterlist element split calculate the offset correctly?
>
> When splitting an element into psge and rsge:
>
> 		psge->length = start - offset;
> 		rsge.length -= psge->length;
> 		rsge.offset += start;
>
> Is start the absolute byte offset from the beginning of the entire message?

It is.

> If the split element is not the first one in the message, wouldn't rsge.offset
> be inflated by the cumulative length of all previous elements instead of the
> relative offset (start - offset)?

It does, this seems like a real bug. AFAICT the offset of rsge should be 

rsge.offset = start + len.

Can you confirm and add to the next version?

>
> When the network stack eventually reads or writes to this element via sg_virt(),
> could this cause an out-of-bounds access past the end of the physical page?


>
>>  	int pop;
>>  
>>  	if (unlikely(flags))

Re: [PATCH bpf] bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check

[Sechang Lim]

On Tue, Jun 09, 2026 at 05:49:49PM -0400, Emil Tsalapatis wrote:
>On Tue Jun 9, 2026 at 2:52 PM EDT, sashiko-bot wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] Unbounded `len` parameter in `bpf_msg_push_data` causes a 32-bit integer overflow when calculating the page allocation size (`copy + len`), resulting in an out-of-bounds heap write.
>> - [High] Incorrect scatterlist element offset calculation when splitting an element in `bpf_msg_push_data` inflates the page offset, leading to out-of-bounds memory accesses.
>> --
>>
>> commit 619f5b1d1aebdbd6457c3ac6e3a92bc45be7278c
>> Author: Sechang Lim <rhkrqnwk98@gmail.com>
>
>The change seems solid, the error is apparent from code inspection
>alone (though a repro wouldn't hurt). Can you also turn pop from an int
>to a u32? It should never be negative AFAICT, and we assign to it from
>a u32 so since we're fixing int types...
>

Will do in v2.

>>
>> bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check
>>
>> start and len are u32, so start + len evaluates in 32-bit and wraps before
>> storing it in last. The bounds check can then be passed with an out-of-range
>> start/len.
>>
>>> diff --git a/net/core/filter.c b/net/core/filter.c
>>> index 9590877b0714f..31467c0b4279e 100644
>>> --- a/net/core/filter.c
>>> +++ b/net/core/filter.c
>>> @@ -2974,7 +2974,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
>>>  	   u32, len, u64, flags)
>>>  {
>>>  	u32 i = 0, l = 0, space, offset = 0;
>>> -	u64 last = start + len;
>>> +	u64 last = (u64)start + len;
>>
>> [Severity: High]
>> This isn't a bug introduced by this patch, but while fixing the integer
>> overflow in bpf_msg_pop_data(), could a similar overflow occur in
>> bpf_msg_push_data()?
>>
>> Looking at net/core/filter.c:bpf_msg_push_data(), both copy and len are u32:
>>
>> 	page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
>> 			   get_order(copy + len));
>>
>
>This seems like a valid concern. Can you add it to the patch? Making
>copy a u64 should do it.
>

Already handled by Weiming Shi:
https://lore.kernel.org/bpf/20260424191602.1522411-3-bestswngs@gmail.com/

>> If the split element is not the first one in the message, wouldn't rsge.offset
>> be inflated by the cumulative length of all previous elements instead of the
>> relative offset (start - offset)?
>
>It does, this seems like a real bug. AFAICT the offset of rsge should be
>
>rsge.offset = start + len.
>
>Can you confirm and add to the next version?
>

Already fixed upstream:
https://lore.kernel.org/bpf/8b129d10566aa3eb43f61a8f9757bcf51707d324.1779636774.git.xuyq21@lenovo.com/

So I'll keep v2 to the bpf_msg_pop_data() fix plus the pop type change.

Thanks,
Sechang