Skip to content

iface: fix checksums of fragmented ICMP and TCP packets - #1213

Closed
snabb wants to merge 1 commit into
smoltcp-rs:mainfrom
snabb:fix-fragment-checksum
Closed

snabb wants to merge 1 commit into
smoltcp-rs:mainfrom
snabb:fix-fragment-checksum

Conversation

@snabb

@snabb snabb commented Oct 8, 2026

Copy link
Copy Markdown

When an IPv4 packet exceeds the MTU, dispatch_ip emits it whole into Fragmenter::buffer and sends it in pieces. emit_ip is handed the entire buffer, so emit_payload gets a slice running past the end of the packet. UDP checksums only its own length, but Icmpv4Packet::fill_checksum and TcpPacket::fill_checksum sum the whole slice, including whatever an earlier, larger packet left there. The buffer is never cleared, so after one larger packet, smaller fragmented ICMP and TCP packets go out with a wrong checksum and the receiver drops them.

Seen on a Linux host as echo replies to fragmented pings failing with IcmpInCsumErrors after a single larger ping. The checksum sent equals the checksum over the reply plus the tail of the earlier reply.

The fix passes only total_ip_len bytes of the buffer to emit_ip. The new test fills the buffer with junk first, then dispatches an oversized ICMP echo reply and TCP segment and checks the checksums of the packets as the fragmenter holds them; without the fix it fails for both.

TCP sockets cap segments at the MTU, so in practice this affects ICMP (echo replies to fragmented requests). TCP hits it only through dispatch_ip directly.

Tested with cargo test, ./ci.sh test stable, ./ci.sh clippy and ./ci.sh check_unused. On hardware, with embassy-net 0.9.1 and smoltcp 0.13.1 carrying the same one-line change: after a 1400-byte ping, fragmented pings of 500–1400 bytes went from 0/27 to 30/30.

🤖 Generated with Claude Code

dispatch_ip emitted an oversized IPv4 packet into the whole fragmentation
buffer, so emit_payload got a slice running past the end of the packet.
ICMP and TCP checksum the slice they are given, which then included bytes
an earlier, larger packet left in the buffer. Pass only the packet's
length.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.72%. Comparing base (87b463f) to head (acffa08).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1213      +/-   ##
==========================================
+ Coverage   82.64%   82.72%   +0.07%     
==========================================
  Files          83       83              
  Lines       26261    26261              
==========================================
+ Hits        21703    21724      +21     
+ Misses       4558     4537      -21     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ld-cd

ld-cd commented Oct 9, 2026

Copy link
Copy Markdown

Could you open a corresponding github issue discribing the problem to assist with triage?

Also per the contributing guidelines we're following the NLNet LLM policy and as such cannot accept LLM generated PRs. If you'd like after we've confirmed and reproduced the issue I should have time sometime in the next couple of weeks to hop on a call and pair program a fix for this.

@ld-cd ld-cd closed this Oct 9, 2026
@snabb

snabb commented Oct 9, 2026

Copy link
Copy Markdown
Author

No I am not going to hop on a call. This PR contains all the details and test to reproduce it. If you don't want it, we'll leave at that. Ridiculous policy. Good bye.

@smoltcp-rs smoltcp-rs locked as resolved and limited conversation to collaborators Oct 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants