[PATCH bpf-next v3] selftests/bpf: Fix csum_partial() dropping trailing byte on odd length

[PATCH bpf-next v3] selftests/bpf: Fix csum_partial() dropping trailing byte on odd length

[Madhav Khosla]

csum_partial() computes num_u16 = len >> 1 and only sums that many
16-bit words, so the last byte of an odd-length buffer never gets
added to the checksum. RFC 1071 says it should be padded with a zero
byte and summed as one more word, not dropped.

This backs build_ip_csum(), build_udp_v4_csum() and
build_udp_v6_csum(), used by flow_dissector_classification.c and
xdp_metadata.c to hand-build packets. No current caller builds an
odd-length payload, so nothing fails today, but a future one would
get a silently wrong checksum.

Also bump flow_dissector_classification's TEST_PACKET_LEN from 100 to
99 so this actually gets exercised instead of staying latent.

f4504af68575 wrote the len >> 1 division, but it only ever passed
sizeof(iphdr), always even, so it couldn't hit the bug. Tagging
bcc00987bc56 instead, since it added the first caller,
build_udp_v4_csum()/build_udp_v6_csum(), that can pass an odd length.

Verified with test_progs under vmtest.sh: without the fix, an odd
TEST_PACKET_LEN makes the kernel drop the packet over a bad checksum
and flow_dissector_classification fails; with the fix, both
flow_dissector_classification and xdp_metadata pass.

Fixes: bcc00987bc56 ("selftests/bpf: add network helpers to generate udp checksums")
Signed-off-by: Madhav Khosla <madhav.khoslaa@gmail.com>
---
v2 -> v3:
- Move the revision changelog and sample test output below --- so
  they don't end up in the permanent commit history (Alexis)
- Drop the Link tag; one is added automatically on merge (Alexis)
- Paste a passing run too, not just the failure (Alexis)
- Also run xdp_metadata, since it uses the same helpers (Alexis)

v1 -> v2:
- comment style: opening /* on its own line, per BPF selftests style
- Retarget the Fixes tag to bcc00987bc56. commit f4504af68575
  ("selftests/bpf: move ip checksum helper to network helpers") moved
  the helper, but sizeof(iphdr) is always a multiple of 32 bit words /
  4 Bytes (iph->ihl counts in 4-byte words), so the odd-length path
  was never reachable through build_ip_csum(). csum_partial() first
  gets called with a length that isn't guaranteed even in
  bcc00987bc56, via build_udp_v4_csum()/build_udp_v6_csum().
- TEST_PACKET_LEN 100 -> 99 so an existing test catches this instead
  of the bug staying unexercised

Without the fix and with TEST_PACKET_LEN=99, flow_dissector_classification
fails under vmtest.sh:

    test_flow_dissector_classification:FAIL:test third port unexpected
    test third port: actual 0 != expected 10
    #137/6   flow_dissector_classification/ipv6:FAIL
    #137     flow_dissector_classification:FAIL
    Summary: 1/0 PASSED, 0 SKIPPED, 1/6 FAILED

With the fix, ./test_progs -t flow_dissector_classification passes under
vmtest.sh:

    #137/1   flow_dissector_classification/ipv4:OK
    #137/2   flow_dissector_classification/ipv4_continue_dissect:OK
    #137/3   flow_dissector_classification/ipip:OK
    #137/4   flow_dissector_classification/gre:OK
    #137/5   flow_dissector_classification/port_range:OK
    #137/6   flow_dissector_classification/ipv6:OK
    #137     flow_dissector_classification:OK
    Summary: 1/6 PASSED, 0 SKIPPED, 0/0 FAILED

TEST_PACKET_LEN is 99 by default now, so no need to force it anymore.
Ran alongside xdp_metadata too, since it's the other user of
build_udp_v4_csum()/build_udp_v6_csum():

    #137/1   flow_dissector_classification/ipv4:OK
    #137/2   flow_dissector_classification/ipv4_continue_dissect:OK
    #137/3   flow_dissector_classification/ipip:OK
    #137/4   flow_dissector_classification/gre:OK
    #137/5   flow_dissector_classification/port_range:OK
    #137/6   flow_dissector_classification/ipv6:OK
    #137     flow_dissector_classification:OK
    #755     xdp_metadata:OK
    Summary: 2/6 PASSED, 0 SKIPPED, 0/0 FAILED
---
 tools/testing/selftests/bpf/network_helpers.h     | 15 +++++++++++++--
 .../prog_tests/flow_dissector_classification.c    |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index 75133119c04a..878c9fc5c37c 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -129,12 +129,23 @@ static __u16 csum_fold(__u32 csum)
 
 static __wsum csum_partial(const void *buf, int len, __wsum sum)
 {
-	__u16 *p = (__u16 *)buf;
+	const __u8 *p = buf;
 	int num_u16 = len >> 1;
 	int i;
 
 	for (i = 0; i < num_u16; i++)
-		sum += p[i];
+		sum += ((const __u16 *)p)[i];
+
+	/*
+	 * RFC 1071: an odd-length buffer's trailing byte is paired with
+	 * a zero pad byte to form the final 16-bit word.
+	 */
+	if (len & 1) {
+		__u16 tail = 0;
+
+		__builtin_memcpy(&tail, p + len - 1, 1);
+		sum += tail;
+	}
 
 	return sum;
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c
index 80b153d3ddec..421dfa6c4ea3 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c
@@ -27,7 +27,7 @@
 #define TEST_NAME_MAX_LEN (32 + SUBTEST_NAME_MAX_LEN)
 #define MAX_SOURCE_PORTS 3
 #define TEST_PACKETS_COUNT 10
-#define TEST_PACKET_LEN 100
+#define TEST_PACKET_LEN 99
 #define TEST_PACKET_PATTERN 'a'
 #define TEST_IPV4 "192.168.0.1/32"
 #define TEST_IPV6 "100::a/128"
-- 
2.55.0

Re: [PATCH bpf-next v3] selftests/bpf: Fix csum_partial() dropping trailing byte on odd length

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

> csum_partial() computes num_u16 = len >> 1 and only sums that many
> 16-bit words, so the last byte of an odd-length buffer never gets
> added to the checksum. RFC 1071 says it should be padded with a zero
> byte and summed as one more word, not dropped.
>
> This backs build_ip_csum(), build_udp_v4_csum() and
> build_udp_v6_csum(), used by flow_dissector_classification.c and
> xdp_metadata.c to hand-build packets. No current caller builds an
> odd-length payload, so nothing fails today, but a future one would
> get a silently wrong checksum.
>
> Also bump flow_dissector_classification's TEST_PACKET_LEN from 100 to
> 99 so this actually gets exercised instead of staying latent.
>
> f4504af68575 wrote the len >> 1 division, but it only ever passed
> sizeof(iphdr), always even, so it couldn't hit the bug. Tagging
> bcc00987bc56 instead, since it added the first caller,
> build_udp_v4_csum()/build_udp_v6_csum(), that can pass an odd length.
>
> Verified with test_progs under vmtest.sh: without the fix, an odd
> TEST_PACKET_LEN makes the kernel drop the packet over a bad checksum
> and flow_dissector_classification fails; with the fix, both
> flow_dissector_classification and xdp_metadata pass.
>
> Fixes: bcc00987bc56 ("selftests/bpf: add network helpers to generate udp checksums")
> Signed-off-by: Madhav Khosla <madhav.khoslaa@gmail.com>

Should the Fixes tag target f4504af68575 instead of bcc00987bc56?

The tag currently points to bcc00987bc56, but that commit never touched
csum_partial(). Looking at the commit:

  git show --stat bcc00987bc56

shows it adds 42 lines to network_helpers.h consisting only of
csum_tcpudp_magic(), csum_ipv6_magic(), build_udp_v4_csum() and
build_udp_v6_csum().

The buggy code being fixed here was introduced by f4504af68575
("selftests/bpf: move ip checksum helper to network helpers"), whose diff
adds:

  +static __wsum csum_partial(const void *buf, int len, __wsum sum)
  +{
  +	__u16 *p = (__u16 *)buf;
  +	int num_u16 = len >> 1;

The commit message's rationale for retargeting the tag is also
self-undermining. It says bcc00987bc56 "added the first caller ... that
*can* pass an odd length", while the paragraph above states "No current
caller builds an odd-length payload, so nothing fails today".

As of every released kernel, no caller of build_udp_v4_csum() or
build_udp_v6_csum() passes an odd length. Before this patch TEST_PACKET_LEN
was 100, so udph->len was 108, and xdp_metadata.c only ever passes
iph->ihl << 2 == 20. The odd length is introduced by this very patch.

So either the tag belongs on f4504af68575, which actually wrote the
len >> 1 truncation, or the bug is latent-only and there should be no Fixes
tag at all. The tag as written points at a commit that neither contains the
defect nor exposes it.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35501508732

Re: [PATCH bpf-next v3] selftests/bpf: Fix csum_partial() dropping trailing byte on odd length

[patchwork-bot+netdevbpf]

Hello:

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

On Sun, 20 Sep 2026 14:25:49 +0530 you wrote:
> csum_partial() computes num_u16 = len >> 1 and only sums that many
> 16-bit words, so the last byte of an odd-length buffer never gets
> added to the checksum. RFC 1071 says it should be padded with a zero
> byte and summed as one more word, not dropped.
> 
> This backs build_ip_csum(), build_udp_v4_csum() and
> build_udp_v6_csum(), used by flow_dissector_classification.c and
> xdp_metadata.c to hand-build packets. No current caller builds an
> odd-length payload, so nothing fails today, but a future one would
> get a silently wrong checksum.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3] selftests/bpf: Fix csum_partial() dropping trailing byte on odd length
    https://git.kernel.org/bpf/bpf-next/c/ddaa827dcea4

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