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.
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
Kernel just drops the packet over a bad checksum. With the fix, both
flow_dissector_classification and xdp_metadata pass.
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
Fixes: bcc00987bc56 ("selftests/bpf: add network helpers to generate udp
checksums")
Link: https://lore.kernel.org/bpf/[email protected]/T/#t
Signed-off-by: Madhav Khosla <[email protected]>
---
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
