[PATCH bpf v2 0/2] bpf, sockmap: fix bpf_msg_pop_data() integer overflow

[PATCH bpf v2 0/2] bpf, sockmap: fix bpf_msg_pop_data() integer overflow

[Sechang Lim]

bpf_msg_pop_data() computes "u64 last = start + len" with u32 operands,
so a len close to U32_MAX wraps the sum and passes the bounds check. The
pop loop then walks off the end of the sk_msg scatterlist and
sk_msg_shift_left() calls put_page() on the empty msg->sg.end slot.

v2:
 - add selftest (Cong Wang)
 - change pop to u32 (Emil Tsalapatis)

v1:
 - https://lore.kernel.org/all/20260609183927.4021802-1-rhkrqnwk98@gmail.com/

Sechang Lim (2):
  bpf, sockmap: fix integer overflow in bpf_msg_pop_data() bounds check
  selftests/bpf: add test for bpf_msg_pop_data() overflow

 net/core/filter.c                             |  4 +-
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 48 +++++++++++++++++++
 .../bpf/progs/test_sockmap_msg_pop_data.c     | 27 +++++++++++
 3 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_pop_data.c

-- 
2.43.0

[PATCH bpf v2 1/2] 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>

Widen the addition with a (u64) cast so the bound is evaluated in
64-bit and a len near U32_MAX no longer wraps below msg->sg.size.

While here, change pop from int to u32. It counts bytes against the
unsigned scatterlist lengths and can never be negative, so the signed
type only invites sign-confusion in the pop loop.

Fixes: 7246d8ed4dcc ("bpf: helper to pop data from messages")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
 net/core/filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 80439767e0ee..9cdfec2ca11e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2974,8 +2974,8 @@ 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;
-	int pop;
+	u64 last = (u64)start + len;
+	u32 pop;
 
 	if (unlikely(flags))
 		return -EINVAL;
-- 
2.43.0

[PATCH bpf v2 2/2] selftests/bpf: add test for bpf_msg_pop_data() overflow

[Sechang Lim]

Add a test in sockmap_basic.c that calls bpf_msg_pop_data() with a length
close to U32_MAX, which overflows the start + len bounds check. The sk_msg
program records the return value over a sendmsg and the test checks that
the call is rejected with -EINVAL.

Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 48 +++++++++++++++++++
 .../bpf/progs/test_sockmap_msg_pop_data.c     | 27 +++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_pop_data.c

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index d2846579285f..cb3229711f93 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -14,6 +14,7 @@
 #include "test_sockmap_pass_prog.skel.h"
 #include "test_sockmap_drop_prog.skel.h"
 #include "test_sockmap_change_tail.skel.h"
+#include "test_sockmap_msg_pop_data.skel.h"
 #include "bpf_iter_sockmap.skel.h"
 
 #include "sockmap_helpers.h"
@@ -666,6 +667,51 @@ static void test_sockmap_skb_verdict_change_tail(void)
 	test_sockmap_change_tail__destroy(skel);
 }
 
+static void test_sockmap_msg_verdict_pop_data(void)
+{
+	struct test_sockmap_msg_pop_data *skel;
+	int err, map, verdict;
+	int c1 = -1, p1 = -1, sent;
+	int zero = 0;
+	char *buf;
+	const size_t len = 32 * 1024;
+
+	skel = test_sockmap_msg_pop_data__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	verdict = bpf_program__fd(skel->progs.prog_msg_pop_data);
+	map = bpf_map__fd(skel->maps.sock_map);
+
+	err = bpf_prog_attach(verdict, map, BPF_SK_MSG_VERDICT, 0);
+	if (!ASSERT_OK(err, "bpf_prog_attach"))
+		goto out;
+
+	err = create_pair(AF_INET, SOCK_STREAM, &c1, &p1);
+	if (!ASSERT_OK(err, "create_pair"))
+		goto out;
+
+	err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem"))
+		goto out_close;
+
+	buf = calloc(len, 1);
+	if (!ASSERT_OK_PTR(buf, "calloc"))
+		goto out_close;
+
+	sent = xsend(c1, buf, len, 0);
+	ASSERT_EQ(sent, (ssize_t)len, "xsend");
+	ASSERT_EQ(skel->data->pop_data_ret, -EINVAL, "pop_data_rejects overflow");
+
+	free(buf);
+
+out_close:
+	close(c1);
+	close(p1);
+out:
+	test_sockmap_msg_pop_data__destroy(skel);
+}
+
 static void test_sockmap_skb_verdict_peek_helper(int map)
 {
 	int err, c1, p1, zero = 0, sent, recvd, avail;
@@ -1373,6 +1419,8 @@ void test_sockmap_basic(void)
 		test_sockmap_skb_verdict_fionread(false);
 	if (test__start_subtest("sockmap skb_verdict change tail"))
 		test_sockmap_skb_verdict_change_tail();
+	if (test__start_subtest("sockmap msg_verdict pop_data overflow"))
+		test_sockmap_msg_verdict_pop_data();
 	if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
 		test_sockmap_skb_verdict_peek();
 	if (test__start_subtest("sockmap skb_verdict msg_f_peek with link"))
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_pop_data.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_pop_data.c
new file mode 100644
index 000000000000..301e65b95256
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_pop_data.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKMAP);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, int);
+} sock_map SEC(".maps");
+
+#define POP_START 0x48a3
+#define POP_LEN   0xfffffffd
+
+long pop_data_ret = 1;
+
+SEC("sk_msg")
+int prog_msg_pop_data(struct sk_msg_md *msg)
+{
+	if (msg->size <= POP_START)
+		return SK_PASS;
+
+	pop_data_ret = bpf_msg_pop_data(msg, POP_START, POP_LEN, 0);
+	return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Integer overflow in bpf_msg_push_data leading to out-of-bounds kernel memory write.
- [High] NULL pointer dereference when bpf_msg_pop_data completely empties the message.
- [High] sk_msg_shift_left and sk_msg_shift_right fail to shift the msg->sg.copy bitmap.
- [High] Information leak due to uninitialized memory allocation in bpf_msg_push_data.
--

commit 291c1f747004c017b1fe863b58e41dd47655f42e
Author: Sechang Lim <rhkrqnwk98@gmail.com>

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

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 can then be
passed with an out-of-range start/len. Widen the addition with a (u64) cast
so the bound is evaluated in 64-bit and a len near U32_MAX no longer wraps.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9590877b0714f..8dac0f4d9fcaf 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2974,8 +2974,8 @@ 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;
> -	int pop;
> +	u64 last = (u64)start + len;
> +	u32 pop;
>  
>  	if (unlikely(flags))
>  		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but I have a question about the end of this
function. If a BPF program pops the entire message payload (i.e. start == 0
and len == msg->sg.size), the bounds check passes and the pop loop shifts
out all elements, leaving an empty scatterlist.

At the end of bpf_msg_pop_data(), we unconditionally call:

    sk_msg_compute_data_pointers(msg);

Which does:

    struct scatterlist *sge = sk_msg_elem(msg, msg->sg.start);
    ...
    msg->data = sg_virt(sge);

Since the element is now empty, does this cause a NULL pointer dereference
when sg_virt() attempts to dereference the page struct?

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential integer overflow in
the related bpf_msg_push_data() function.

In net/core/filter.c:bpf_msg_push_data():

    page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
                       get_order(copy + len));
    ...
            to = raw + front + len;
            memcpy(to, from, back);

Since len is an unchecked u32 passed from the BPF program, if a very large
len is provided, could copy + len wrap around in 32-bit math and result in
a small memory allocation?

If so, the pointer arithmetic for the destination variable would calculate
an address far beyond the allocated page, potentially causing the memcpy()
to write controlled payload data out of bounds.

[Severity: High]
This isn't a bug introduced by this patch, but looking at the same
allocation in bpf_msg_push_data():

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

It appears to omit the __GFP_ZERO flag. Since only the original front and
back regions are explicitly initialized, does this leave the newly inserted
len bytes uninitialized?

If the BPF program does not manually overwrite these bytes, could
uninitialized kernel memory become part of the packet payload and be
transmitted over the network?

[Severity: High]
This is a pre-existing issue, but while reviewing the scatterlist shifts, I
noticed sk_msg_shift_left() in net/core/filter.c updates the data array:

    do {
        prev = i;
        sk_msg_iter_var_next(i);
        msg->sg.data[prev] = msg->sg.data[i];
    } while (i != msg->sg.end);

However, it doesn't appear to shift the corresponding bits in the
msg->sg.copy bitmap.

Could this desynchronization cause read-only or cloned elements to lose
their copy flag, allowing them to be incorrectly mapped as writable by
sk_msg_compute_data_pointers()?

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

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

[Alexei Starovoitov]

On Wed Jun 10, 2026 at 1:11 AM PDT, 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>
>
> Widen the addition with a (u64) cast so the bound is evaluated in
> 64-bit and a len near U32_MAX no longer wraps below msg->sg.size.
>
> While here, change pop from int to u32. It counts bytes against the
> unsigned scatterlist lengths and can never be negative, so the signed
> type only invites sign-confusion in the pop loop.
>
> Fixes: 7246d8ed4dcc ("bpf: helper to pop data from messages")
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
> ---
>  net/core/filter.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 80439767e0ee..9cdfec2ca11e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2974,8 +2974,8 @@ 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;
> -	int pop;
> +	u64 last = (u64)start + len;

sashiko is correct that there are 4 other issue in very similar code path
all in skmsg. Please fix them all in one go.

pw-bot: cr