[PATCH] bpf, sockmap: Fix ring capacity invariant break in bpf_msg_push_data()

[PATCH] bpf, sockmap: Fix ring capacity invariant break in bpf_msg_push_data()

[bo cao]

From: Bo Cao <treebacker@foxmail.com>

A push at msg->size ends the element walk on the ring's free slot, so
the "no space, absorb by copy" fallback reads the free slot's length
instead of the last live element's. Nothing is absorbed and the split
path runs anyway: with the ring already holding MAX_MSG_FRAGS live
elements it advances sg.end onto sg.start, all NR_MSG_FRAG_IDS slots
become live, and the occupancy (the distance from sg.start to
sg.end) reads 0 -- "empty". Every capacity check in push/pop is then
bypassed and later transforms corrupt the element list.

Without the fix, a single send() with an SK_MSG verdict program on a
full ring deterministically crashes the SK_DROP teardown (arm64,
7.4.0-rc2 + the bpf master sockmap series):

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
  ESR = 0x0000000096000004  FSC = 0x04: level 0 translation fault
  Internal error: Oops: 0000000096000004 [#1]  SMP
  CPU: 2 UID: 0 PID: 546 Comm: skmsg_panic Tainted: G        W           7.4.0-rc2 #1 PREEMPT(full)
  pc : sk_msg_free_elem+0x90/0x148
  lr : __sk_msg_free+0x8c/0x10c
  x14: 4141414141414141
  Call trace:
   sk_msg_free_elem+0x90/0x148
   __sk_msg_free+0x8c/0x10c
   sk_msg_free+0x18/0x24
   tcp_bpf_sendmsg+0x298/0xa54
   inet_sendmsg+0x44/0x70
   __sys_sendto+0x120/0x198

put_page() on a zeroed scatterlist entry. With the fix the helper
returns -ENOSPC, the message frees normally, and the crash is gone.

Never read the free slot in the fallback: absorb into the last live
element when the walk ended on it and the ring is full, or fail with
-ENOSPC if there is nothing to absorb, so the guard slot is never
consumed.

The bug was found with LLM-assisted source auditing and verified end
to end: it is reachable with CAP_BPF/CAP_NET_ADMIN and confirmed
exploitable for local privilege escalation (details reported to the
maintainers separately). Regression tested with
tools/testing/selftests/bpf/test_sockmap.

Tested on top of v7.4-rc2 plus the sockmap fix series currently in
bpf/bpf.git master (b889ea1f6dbe..0048086bd361); neither that series
nor the June sk_msg fixes (f3f34ca45b96 and friends) touch the ring
capacity invariant, and the patch applies to mainline and bpf master
alike.

Fixes: 6fff607e2f14 ("bpf: sk_msg program helper bpf_msg_push_data")
Signed-off-by: Bo Cao <treebacker@foxmail.com>
---
 net/core/filter.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 1f81a3dd6a5c..371c090d7f8d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2892,8 +2892,29 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
 	 * offset case because its the common case for inserting a
 	 * header.
 	 */
-	if (!space || (space == 1 && start != offset))
-		copy = msg->sg.data[i].length;
+	if (!space || (space == 1 && start != offset)) {
+		/* A push at msg->size leaves the walk on the ring's free
+		 * slot, which is not a live element: its length reads 0
+		 * (or stale data left by an earlier ring rotation), so
+		 * the copy fallback would absorb nothing and the split
+		 * path below would run. With the ring already full
+		 * (space == 0) that path advances sg.end onto sg.start,
+		 * leaving all NR_MSG_FRAG_IDS slots live -- the ring can
+		 * no longer tell "full" from "empty" and later
+		 * transforms corrupt state. Absorb into the last live
+		 * element instead, or fail if there is nothing to absorb.
+		 */
+		if (i == msg->sg.end) {
+			if (!space) {
+				sk_msg_iter_var_prev(i);
+				copy = msg->sg.data[i].length;
+				if (!copy)
+					return -ENOSPC;
+			}
+		} else {
+			copy = msg->sg.data[i].length;
+		}
+	}
 
 	if (unlikely(copy + len < copy))
 		return -EINVAL;
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] bpf, sockmap: Fix ring capacity invariant break in bpf_msg_push_data()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [Low] bpf: non-standard multi-line comment style in
  bpf_msg_push_data()
- [Low] bpf, sockmap: inaccurate -ENOSPC claim and dead check in
  bpf_msg_push_data()
- [Low] bpf, sockmap: inaccurate Fixes trailer in bpf_msg_push_data()
  fix
- [Low] bpf: unreachable sg.end check in bpf_msg_push_data() copy path
--

commit a346141a5e25b55660858c935ff1dea8fc5392a6
Author: Bo Cao <treebacker@foxmail.com>

> bpf, sockmap: Fix ring capacity invariant break in bpf_msg_push_data()
>
> A push at msg->size ends the element walk on the ring's free slot, so
> the "no space, absorb by copy" fallback reads the free slot's length
> instead of the last live element's. Nothing is absorbed and the split
> path runs anyway: with the ring already holding MAX_MSG_FRAGS live
> elements it advances sg.end onto sg.start, all NR_MSG_FRAG_IDS slots
> become live, and the occupancy (the distance from sg.start to
> sg.end) reads 0 -- "empty". Every capacity check in push/pop is then
> bypassed and later transforms corrupt the element list.
>
> Without the fix, a single send() with an SK_MSG verdict program on a
> full ring deterministically crashes the SK_DROP teardown (arm64,
> 7.4.0-rc2 + the bpf master sockmap series):
>
>   Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
>   ESR = 0x0000000096000004  FSC = 0x04: level 0 translation fault
>   Internal error: Oops: 0000000096000004 [#1]  SMP
>   CPU: 2 UID: 0 PID: 546 Comm: skmsg_panic Tainted: G        W           7.4.0-rc2 #1 PREEMPT(full)
>   pc : sk_msg_free_elem+0x90/0x148
>   lr : __sk_msg_free+0x8c/0x10c
>   x14: 4141414141414141
>   Call trace:
>    sk_msg_free_elem+0x90/0x148
>    __sk_msg_free+0x8c/0x10c
>    sk_msg_free+0x18/0x24
>    tcp_bpf_sendmsg+0x298/0xa54
>    inet_sendmsg+0x44/0x70
>    __sys_sendto+0x120/0x198
>
> put_page() on a zeroed scatterlist entry. With the fix the helper
> returns -ENOSPC, the message frees normally, and the crash is gone.
>
> Never read the free slot in the fallback: absorb into the last live
> element when the walk ended on it and the ring is full, or fail with
> -ENOSPC if there is nothing to absorb, so the guard slot is never
> consumed.
>
> The bug was found with LLM-assisted source auditing and verified end
> to end: it is reachable with CAP_BPF/CAP_NET_ADMIN and confirmed
> exploitable for local privilege escalation (details reported to the
> maintainers separately). Regression tested with
> tools/testing/selftests/bpf/test_sockmap.
>
> Tested on top of v7.4-rc2 plus the sockmap fix series currently in
> bpf/bpf.git master (b889ea1f6dbe..0048086bd361); neither that series
> nor the June sk_msg fixes (f3f34ca45b96 and friends) touch the ring
> capacity invariant, and the patch applies to mainline and bpf master
> alike.
>
> Fixes: 6fff607e2f14 ("bpf: sk_msg program helper bpf_msg_push_data")
> Signed-off-by: Bo Cao <treebacker@foxmail.com>

[Severity: Low]
Prior to commit 15ab0548e310 ("bpf, sockmap: Several fixes to
bpf_msg_push_data"), bpf_msg_push_data() checked start >= offset + l after
the scatterlist walk and returned -EINVAL whenever the loop ended at
i == msg->sg.end on a non-empty ring.  Commit 15ab0548e310 ("bpf, sockmap:
Several fixes to bpf_msg_push_data") relaxed that check to allow pushing at
the end of a message and added the sk_msg_iter_var_prev(i) adjustment inside
if (copy), overlooking that copy = msg->sg.data[i].length was read earlier.
Should the Fixes: tag point to that commit instead?

  Fixes: 15ab0548e310 ("bpf, sockmap: Several fixes to bpf_msg_push_data")

> diff --git a/net/core/filter.c b/net/core/filter.c
> index d68b521504050..9ecb94a56d548 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2892,8 +2892,29 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
>  	 * offset case because its the common case for inserting a
>  	 * header.
>  	 */
> -	if (!space || (space == 1 && start != offset))
> -		copy = msg->sg.data[i].length;
> +	if (!space || (space == 1 && start != offset)) {
> +		/* A push at msg->size leaves the walk on the ring's free
> +		 * slot, which is not a live element: its length reads 0
> +		 * (or stale data left by an earlier ring rotation), so
> +		 * the copy fallback would absorb nothing and the split
> +		 * path below would run. With the ring already full
> +		 * (space == 0) that path advances sg.end onto sg.start,
> +		 * leaving all NR_MSG_FRAG_IDS slots live -- the ring can
> +		 * no longer tell "full" from "empty" and later
> +		 * transforms corrupt state. Absorb into the last live
> +		 * element instead, or fail if there is nothing to absorb.
> +		 */

[Severity: Low]
This isn't a bug, but BPF coding style places the opening /* of a multi-line
comment on its own line with the comment text starting on the next line.

> +		if (i == msg->sg.end) {
> +			if (!space) {
> +				sk_msg_iter_var_prev(i);
> +				copy = msg->sg.data[i].length;
> +				if (!copy)
> +					return -ENOSPC;
> +			}
> +		} else {
> +			copy = msg->sg.data[i].length;
> +		}
> +	}
>  
>  	if (unlikely(copy + len < copy))
>  		return -EINVAL;

[Severity: Low]
Can copy ever be zero here when !space holds, or is the commit message
inaccurate in stating that the helper returns -ENOSPC on a full ring?

Every live scatterlist element in [msg->sg.start, msg->sg.end) has a
non-zero length.  When !space holds, the ring has MAX_MSG_FRAGS live
elements, so stepping i back from msg->sg.end selects the last live element
with msg->sg.data[i].length > 0; bpf_msg_push_data() then absorbs the push
into a newly allocated page and returns 0 (or -ENOMEM) rather than -ENOSPC.

[Severity: Low]
Does stepping i back from msg->sg.end here make the second msg->sg.end check
inside if (copy) unreachable dead code?

net/core/filter.c:bpf_msg_push_data() {
    ...
	if (copy) {
		int front, back;

		raw = page_address(page);

		if (i == msg->sg.end)
			sk_msg_iter_var_prev(i);
    ...
}

copy is initialized to 0 and is only set to a non-zero value when
i != msg->sg.end or after the !space branch above has already decremented i
away from msg->sg.end, so i can never equal msg->sg.end when entering
if (copy).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_84A432FD1AE7054D384BEB36958789AAFA05@qq.com?part=1

Re: [PATCH] bpf, sockmap: Fix ring capacity invariant break in bpf_msg_push_data()

[Alexei Starovoitov]

On Wed, Nov 11, 2026 at 07:13 PM bo cao <treebacker@foxmail.com> wrote:
> A push at msg->size ends the element walk on the ring's free slot, so
> the "no space, absorb by copy" fallback reads the free slot's length
> instead of the last live element's. Nothing is absorbed and the split

Way too long.
A push at msg->size on a full ring reads the length of the free slot.
It is 0, so nothing is copied and sg.end moves onto sg.start.
The ring looks empty and the next push overwrites live elements.
The prog needs CAP_BPF and CAP_NET_ADMIN.
That's all that needs to be said.

> put_page() on a zeroed scatterlist entry. With the fix the helper
> returns -ENOSPC, the message frees normally, and the crash is gone.

It doesn't.
I don't think you tested your patch.

> Fixes: 6fff607e2f14 ("bpf: sk_msg program helper bpf_msg_push_data")

Wrong tag.
A push at msg->size returned -EINVAL before
commit 15ab0548e310 ("bpf, sockmap: Several fixes to bpf_msg_push_data").

> +	if (!space || (space == 1 && start != offset)) {
> +		/* A push at msg->size leaves the walk on the ring's free
[...]
> +		if (i == msg->sg.end) {
> +			if (!space) {
> +				sk_msg_iter_var_prev(i);
> +				copy = msg->sg.data[i].length;
> +				if (!copy)
> +					return -ENOSPC;
> +			}
> +		} else {
> +			copy = msg->sg.data[i].length;
> +		}
> +	}

This is overkill.
Replace your slop with improved slop:

diff --git a/net/core/filter.c b/net/core/filter.c
index 9245009..c0db554 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2897,6 +2897,10 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
 	if (start > offset + l)
 		return -EINVAL;

+	/* A push at the end of the msg works on the last element */
+	if (i == msg->sg.end && start != offset)
+		sk_msg_iter_var_prev(i);
+
 	space = MAX_MSG_FRAGS - sk_msg_elem_used(msg);

 	/* If no space available will fallback to copy, we need at
@@ -2922,8 +2926,6 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,

 		raw = page_address(page);

-		if (i == msg->sg.end)
-			sk_msg_iter_var_prev(i);
 		psge = sk_msg_elem(msg, i);
 		front = start - offset;
 		back = psge->length - front;
@@ -2945,8 +2947,6 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
 	}

 	if (start - offset) {
-		if (i == msg->sg.end)
-			sk_msg_iter_var_prev(i);
 		psge = sk_msg_elem(msg, i);
 		rsge = sk_msg_elem_cpy(msg, i);
 		rsge_copy = sk_msg_elem_is_copy(msg, i);

Pls tag the respin as [PATCH bpf v2].

pw-bot: cr