[PATCH bpf 1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs

[PATCH bpf 1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs

[Daniel Borkmann]

Cilium generates ICMP "frag needed" replies from BPF when a LB DSR
packet exceeds the egress MTU. The reply is built by first trimming the
packet down to target size via bpf_skb_change_tail(), and then pushing
the ICMP error headers in front of it.

The trim is rejected for skbs which carry a checksum offload, e.g. TCP
packets aggregated by GRO on ingress where tcp_gro_complete() leaves
the skb as CHECKSUM_PARTIAL. __bpf_skb_min_len() raises the minimum
length to the end of the L4 checksum field, so a trim to 42 bytes bails
out with -EINVAL given a min_len of 52 in this case, and due to that
the ICMP generator fails. This is not the case if GRO is turned off.

Fix this bpf_skb_change_tail() restriction and drop the checksum offload
when the new length no longer covers the checksum field. The BPF program
rewrites the skb into an ICMP error and computes the checksum itself
anyway.

Fixes: 5293efe62df8 ("bpf: add bpf_skb_change_tail helper")
Reported-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
Reported-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/filter.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 61940e753552..8513167a858a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3961,12 +3961,6 @@ static u32 __bpf_skb_min_len(const struct sk_buff *skb)
 		if (offset > 0)
 			min_len = offset;
 	}
-	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		offset = skb_checksum_start_offset(skb) +
-			 skb->csum_offset + sizeof(__sum16);
-		if (offset > 0)
-			min_len = offset;
-	}
 	return min_len;
 }
 
@@ -3983,6 +3977,11 @@ static int bpf_skb_grow_rcsum(struct sk_buff *skb, unsigned int new_len)
 
 static int bpf_skb_trim_rcsum(struct sk_buff *skb, unsigned int new_len)
 {
+	if (skb->ip_summed == CHECKSUM_PARTIAL &&
+	    new_len < skb_checksum_start_offset(skb) + skb->csum_offset +
+		      sizeof(__sum16))
+		skb->ip_summed = CHECKSUM_NONE;
+
 	return __skb_trim_rcsum(skb, new_len);
 }
 
-- 
2.43.0

[PATCH bpf 2/2] selftests/bpf: Add test for bpf_skb_change_tail on csum partial skbs

[Daniel Borkmann]

Add a test which builds an ICMP error out of a TCP segment. A tcx prog
on the client's egress side trims the first data segment down to the
target size and pushes the ICMP error headers in front of it to then
reflect the packet back to the sender.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t tc_change_tail
  [...]
  #509     tc_change_tail:OK
  #510     tc_change_tail_pmtu:OK
  Summary: 2/0 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../bpf/prog_tests/tc_change_tail_pmtu.c      | 125 +++++++++++++++++
 .../bpf/progs/test_tc_change_tail_pmtu.c      | 129 ++++++++++++++++++
 2 files changed, 254 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tc_change_tail_pmtu.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_tc_change_tail_pmtu.c

diff --git a/tools/testing/selftests/bpf/prog_tests/tc_change_tail_pmtu.c b/tools/testing/selftests/bpf/prog_tests/tc_change_tail_pmtu.c
new file mode 100644
index 000000000000..7acdbd5757a9
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/tc_change_tail_pmtu.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <netinet/tcp.h>
+
+#include "test_progs.h"
+#include "network_helpers.h"
+#include "test_tc_change_tail_pmtu.skel.h"
+
+#define CLIENT_NS	"tc-change-tail-cli-ns"
+#define SERVER_NS	"tc-change-tail-srv-ns"
+#define CLIENT_IP	"192.168.1.1"
+#define SERVER_IP	"192.168.1.2"
+
+#define TEST_PMTU	1000
+#define TEST_MSS_MAX	(TEST_PMTU - 20 - 20)
+#define TIMEOUT_MS	3000
+#define XFER_BYTES	8192
+
+void test_tc_change_tail_pmtu(void)
+{
+	LIBBPF_OPTS(bpf_tcx_opts, tcx_opts);
+	int mss_before = 0, mss_after = 0, ifindex, port;
+	int srv_fd = -1, srv_conn_fd = -1, cli_fd = -1;
+	struct test_tc_change_tail_pmtu *skel = NULL;
+	struct nstoken *nstoken = NULL;
+	static char buf[XFER_BYTES];
+	socklen_t optlen;
+	ssize_t bytes;
+	size_t total;
+
+	if (!ASSERT_OK(make_netns(CLIENT_NS), "make client ns"))
+		return;
+	if (!ASSERT_OK(make_netns(SERVER_NS), "make server ns"))
+		goto out_client_ns;
+
+	nstoken = open_netns(CLIENT_NS);
+	if (!ASSERT_OK_PTR(nstoken, "open client ns"))
+		goto out;
+	SYS(out, "ip link add veth1 type veth peer name veth2 netns " SERVER_NS);
+	SYS(out, "ip -4 addr add " CLIENT_IP "/24 dev veth1");
+	SYS(out, "ip link set veth1 up");
+	ifindex = if_nametoindex("veth1");
+	if (!ASSERT_NEQ(ifindex, 0, "if_nametoindex"))
+		goto out;
+	close_netns(nstoken);
+	nstoken = NULL;
+
+	nstoken = open_netns(SERVER_NS);
+	if (!ASSERT_OK_PTR(nstoken, "open server ns"))
+		goto out;
+	SYS(out, "ip -4 addr add " SERVER_IP "/24 dev veth2");
+	SYS(out, "ip link set veth2 up");
+	srv_fd = start_server(AF_INET, SOCK_STREAM, SERVER_IP, 0, TIMEOUT_MS);
+	if (!ASSERT_OK_FD(srv_fd, "start server"))
+		goto out;
+	close_netns(nstoken);
+	nstoken = NULL;
+
+	skel = test_tc_change_tail_pmtu__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open and load skeleton"))
+		goto out;
+
+	port = get_socket_local_port(srv_fd);
+	if (!ASSERT_GE(port, 0, "get server port"))
+		goto out;
+
+	skel->bss->server_port = port;
+	skel->bss->pmtu = TEST_PMTU;
+
+	nstoken = open_netns(CLIENT_NS);
+	if (!ASSERT_OK_PTR(nstoken, "open client ns"))
+		goto out;
+
+	skel->links.change_tail_icmp =
+		bpf_program__attach_tcx(skel->progs.change_tail_icmp, ifindex,
+					&tcx_opts);
+	if (!ASSERT_OK_PTR(skel->links.change_tail_icmp, "attach tcx"))
+		goto out;
+
+	cli_fd = connect_to_fd(srv_fd, TIMEOUT_MS);
+	if (!ASSERT_OK_FD(cli_fd, "connect to server"))
+		goto out;
+	srv_conn_fd = accept(srv_fd, NULL, NULL);
+	if (!ASSERT_OK_FD(srv_conn_fd, "accept connection"))
+		goto out;
+	if (!ASSERT_OK(settimeo(srv_conn_fd, TIMEOUT_MS), "set server timeout"))
+		goto out;
+
+	optlen = sizeof(mss_before);
+	if (!ASSERT_OK(getsockopt(cli_fd, IPPROTO_TCP, TCP_MAXSEG, &mss_before,
+				  &optlen), "get mss before"))
+		goto out;
+
+	bytes = send(cli_fd, buf, sizeof(buf), 0);
+	if (!ASSERT_EQ(bytes, (ssize_t)sizeof(buf), "send data"))
+		goto out;
+
+	for (total = 0; total < sizeof(buf); total += bytes) {
+		bytes = recv(srv_conn_fd, buf, sizeof(buf), 0);
+		if (bytes <= 0)
+			break;
+	}
+
+	ASSERT_EQ(total, sizeof(buf), "receive data");
+	ASSERT_OK(skel->data->change_tail_ret, "change tail");
+	ASSERT_OK(skel->bss->adjust_room_ret, "adjust room");
+	ASSERT_TRUE(skel->bss->icmp_sent, "icmp sent");
+
+	optlen = sizeof(mss_after);
+	if (!ASSERT_OK(getsockopt(cli_fd, IPPROTO_TCP, TCP_MAXSEG, &mss_after,
+				  &optlen), "get mss after"))
+		goto out;
+
+	ASSERT_LT(mss_after, mss_before, "mss reduced");
+	ASSERT_LE(mss_after, TEST_MSS_MAX, "mss below pmtu");
+out:
+	close(srv_conn_fd);
+	close(cli_fd);
+	close(srv_fd);
+	test_tc_change_tail_pmtu__destroy(skel);
+	close_netns(nstoken);
+	remove_netns(SERVER_NS);
+out_client_ns:
+	remove_netns(CLIENT_NS);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_tc_change_tail_pmtu.c b/tools/testing/selftests/bpf/progs/test_tc_change_tail_pmtu.c
new file mode 100644
index 000000000000..5c4c07545bc9
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_tc_change_tail_pmtu.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdbool.h>
+#include <stddef.h>
+
+#include <linux/bpf.h>
+#include <linux/icmp.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+#define ICMP_SAMPLE_LEN	(sizeof(struct iphdr) + 8)
+#define ICMP_HDRS_LEN	(sizeof(struct iphdr) + sizeof(struct icmphdr))
+
+__be16 server_port = 0;
+__u16 pmtu = 0;
+
+long change_tail_ret = 1;
+long adjust_room_ret = 0;
+bool icmp_sent = false;
+bool icmp_err = false;
+
+static __always_inline __sum16 csum_fold(__wsum csum)
+{
+	csum = (csum & 0xffff) + (csum >> 16);
+	csum = (csum & 0xffff) + (csum >> 16);
+
+	return (__sum16)~csum;
+}
+
+SEC("tc/egress")
+int change_tail_icmp(struct __sk_buff *skb)
+{
+	__u8 smac[ETH_ALEN], dmac[ETH_ALEN];
+	void *data, *data_end;
+	struct icmphdr *icmp;
+	struct ethhdr *eth;
+	struct tcphdr *tcp;
+	__be32 saddr, daddr;
+	struct iphdr *ip;
+	__wsum csum;
+
+	if (icmp_sent || icmp_err)
+		return TCX_PASS;
+
+	data = (void *)(long)skb->data;
+	data_end = (void *)(long)skb->data_end;
+
+	eth = data;
+	if ((void *)(eth + 1) > data_end)
+		return TCX_PASS;
+	if (eth->h_proto != bpf_htons(ETH_P_IP))
+		return TCX_PASS;
+
+	ip = (void *)(eth + 1);
+	if ((void *)(ip + 1) > data_end)
+		return TCX_PASS;
+	if (ip->ihl != 5 || ip->protocol != IPPROTO_TCP)
+		return TCX_PASS;
+
+	tcp = (void *)(ip + 1);
+	if ((void *)(tcp + 1) > data_end)
+		return TCX_PASS;
+	if (tcp->dest != server_port)
+		return TCX_PASS;
+	if (bpf_ntohs(ip->tot_len) <= sizeof(*ip) + tcp->doff * 4)
+		return TCX_PASS;
+
+	__builtin_memcpy(smac, eth->h_source, ETH_ALEN);
+	__builtin_memcpy(dmac, eth->h_dest, ETH_ALEN);
+	saddr = ip->saddr;
+	daddr = ip->daddr;
+
+	change_tail_ret = bpf_skb_change_tail(skb, ETH_HLEN + ICMP_SAMPLE_LEN, 0);
+	if (change_tail_ret) {
+		icmp_err = true;
+		return TCX_PASS;
+	}
+
+	adjust_room_ret = bpf_skb_adjust_room(skb, ICMP_HDRS_LEN,
+					      BPF_ADJ_ROOM_MAC,
+					      BPF_F_ADJ_ROOM_NO_CSUM_RESET);
+	if (adjust_room_ret) {
+		icmp_err = true;
+		return TCX_DROP;
+	}
+
+	data = (void *)(long)skb->data;
+	data_end = (void *)(long)skb->data_end;
+
+	eth = data;
+	ip = (void *)(eth + 1);
+	icmp = (void *)(ip + 1);
+	if ((void *)icmp + sizeof(*icmp) + ICMP_SAMPLE_LEN > data_end) {
+		icmp_err = true;
+		return TCX_DROP;
+	}
+
+	__builtin_memcpy(eth->h_dest, smac, ETH_ALEN);
+	__builtin_memcpy(eth->h_source, dmac, ETH_ALEN);
+
+	__builtin_memset(icmp, 0, sizeof(*icmp));
+	icmp->type = ICMP_DEST_UNREACH;
+	icmp->code = ICMP_FRAG_NEEDED;
+	icmp->un.frag.mtu = bpf_htons(pmtu);
+
+	__builtin_memset(ip, 0, sizeof(*ip));
+	ip->version = 4;
+	ip->ihl = 5;
+	ip->ttl = 64;
+	ip->protocol = IPPROTO_ICMP;
+	ip->tot_len = bpf_htons(ICMP_HDRS_LEN + ICMP_SAMPLE_LEN);
+	ip->saddr = daddr;
+	ip->daddr = saddr;
+
+	csum = bpf_csum_diff(NULL, 0, (__be32 *)icmp,
+			     sizeof(*icmp) + ICMP_SAMPLE_LEN, 0);
+	icmp->checksum = csum_fold(csum);
+	csum = bpf_csum_diff(NULL, 0, (__be32 *)ip, sizeof(*ip), 0);
+	ip->check = csum_fold(csum);
+	icmp_sent = true;
+	return bpf_redirect(skb->ifindex, BPF_F_INGRESS);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH bpf 1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs

[sashiko-bot]

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

Pre-existing issues:
- [High] The checksum field space check in `bpf_skb_trim_rcsum()` assumes a 2-byte checksum (`sizeof(__sum16)`), but SCTP checksum offloads (`skb->csum_not_inet == 1`) require a 4-byte CRC32c. This allows BPF programs to trim SCTP packets such that only the first 2 bytes of the checksum field remain. The hardware will then attempt to process a 4-byte checksum, causing an out-of-bounds access.
--

commit 4cea525b891060e3211b5353724699e42d836d25
Author: Daniel Borkmann <daniel@iogearbox.net>
bpf: Fix bpf_skb_change_tail wrt csum partial skbs

This commit fixes an issue where bpf_skb_change_tail() rejects trims on skbs
carrying a checksum offload, such as those from GRO. It removes the minimum
length restriction from __bpf_skb_min_len() and modifies bpf_skb_trim_rcsum()
to downgrade the checksum to CHECKSUM_NONE if the new length no longer covers
the checksum field.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 61940e7535523..8513167a858a8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -3983,6 +3977,11 @@ static int bpf_skb_grow_rcsum(struct sk_buff *skb, unsigned int new_len)
>  
>  static int bpf_skb_trim_rcsum(struct sk_buff *skb, unsigned int new_len)
>  {
> +	if (skb->ip_summed == CHECKSUM_PARTIAL &&
> +	    new_len < skb_checksum_start_offset(skb) + skb->csum_offset +
> +		      sizeof(__sum16))
> +		skb->ip_summed = CHECKSUM_NONE;
> +

[Severity: High]
This is a pre-existing issue, but this patch replicates the flawed 2-byte
assumption from __bpf_skb_min_len(). The checksum field space check in
bpf_skb_trim_rcsum() assumes a 2-byte checksum (sizeof(__sum16)), but SCTP
checksum offloads (skb->csum_not_inet == 1) require a 4-byte CRC32c.

This allows BPF programs to trim SCTP packets such that only the first 2 bytes
of the checksum field remain. If trimmed so that exactly 2 or 3 bytes of the
4-byte CRC32c remain, the length check evaluates to false, and CHECKSUM_PARTIAL
is not cleared. The packet is then handed to the NIC driver.

Passing a truncated CHECKSUM_PARTIAL packet to the NIC hardware causes the
offload engine to access beyond the packet boundary, causing a hardware stall
or OOB write on the wire.

Could we check for skb->csum_not_inet and use a 4-byte length for the checksum
when processing SCTP packets here?

>  	return __skb_trim_rcsum(skb, new_len);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907121025.1923656-1-daniel@iogearbox.net?part=1

Re: [PATCH bpf 1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs

[Daniel Borkmann]

On 9/7/26 2:24 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The checksum field space check in `bpf_skb_trim_rcsum()` assumes a 2-byte checksum (`sizeof(__sum16)`), but SCTP checksum offloads (`skb->csum_not_inet == 1`) require a 4-byte CRC32c. This allows BPF programs to trim SCTP packets such that only the first 2 bytes of the checksum field remain. The hardware will then attempt to process a 4-byte checksum, causing an out-of-bounds access.
yeap, unrelated, and I'm not sure anyone has used it in combo with sctp

Re: [PATCH bpf 1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Mon,  7 Sep 2026 14:10:24 +0200 you wrote:
> Cilium generates ICMP "frag needed" replies from BPF when a LB DSR
> packet exceeds the egress MTU. The reply is built by first trimming the
> packet down to target size via bpf_skb_change_tail(), and then pushing
> the ICMP error headers in front of it.
> 
> The trim is rejected for skbs which carry a checksum offload, e.g. TCP
> packets aggregated by GRO on ingress where tcp_gro_complete() leaves
> the skb as CHECKSUM_PARTIAL. __bpf_skb_min_len() raises the minimum
> length to the end of the L4 checksum field, so a trim to 42 bytes bails
> out with -EINVAL given a min_len of 52 in this case, and due to that
> the ICMP generator fails. This is not the case if GRO is turned off.
> 
> [...]

Here is the summary with links:
  - [bpf,1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs
    https://git.kernel.org/bpf/bpf/c/3b55f350c68a
  - [bpf,2/2] selftests/bpf: Add test for bpf_skb_change_tail on csum partial skbs
    https://git.kernel.org/bpf/bpf/c/15e2565f1c43

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