Skip to content

tun: validate GSO checksum offset fits within HdrLen#63

Open
illotum wants to merge 1 commit into
tailscalefrom
illotum/tighten-gso-fuzz
Open

tun: validate GSO checksum offset fits within HdrLen#63
illotum wants to merge 1 commit into
tailscalefrom
illotum/tighten-gso-fuzz

Conversation

@illotum

@illotum illotum commented May 27, 2026

Copy link
Copy Markdown

GSOSplit only bounded the checksum offset against the input packet length, not against HdrLen. With CsumStart+CsumOffset+2 > HdrLen, the per-segment output buffer (sized to HdrLen+segmentDataLen) is too short and writing the transport checksum panics with an index out of range.
Reject the options up front instead.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a bounds check in GSOSplit to reject GSO options where the checksum field (CsumStart + CsumOffset + 2) extends past HdrLen, preventing an index-out-of-range panic when writing the per-segment transport checksum into a buffer sized to HdrLen + segmentDataLen. Also normalizes fuzzer inputs to conform to the virtio_net_hdr contract so Fuzz_GSOSplit exercises the split logic instead of hitting shapes the kernel never produces.

Changes:

  • Validate CsumStart + CsumOffset + 2 <= HdrLen early in GSOSplit and return a descriptive error otherwise.
  • In Fuzz_GSOSplit, clamp CsumStart and HdrLen to minimum IP/transport header sizes per GSO type, and skip cases where the packet is shorter than the resulting HdrLen.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tun/offload.go Adds the new CsumStart+CsumOffset+2 > HdrLen validation in GSOSplit.
tun/offload_test.go Normalizes fuzzer-generated GSOOptions to match kernel virtio_net_hdr expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@illotum
illotum requested a review from jwhited May 27, 2026 00:14
Comment thread tun/offload_test.go Outdated
Comment thread tun/offload.go
@illotum
illotum force-pushed the illotum/tighten-gso-fuzz branch from ace7ee3 to 40c9695 Compare May 27, 2026 02:58
GSOSplit only bounded the checksum offset against the input packet
length, not against HdrLen. With CsumStart+CsumOffset+2 > HdrLen, the
per-segment output buffer (sized to HdrLen+segmentDataLen) is too short
and writing the transport checksum panics with an index out of range.
Reject the options up front instead.

Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
Change-Id: Id073f0951d52e01a05b9a1338594161c6a6a6964
@illotum
illotum force-pushed the illotum/tighten-gso-fuzz branch from 40c9695 to ab16a28 Compare May 27, 2026 03:01
@illotum

illotum commented May 27, 2026

Copy link
Copy Markdown
Author

@jwhited The other guards will be added in #62 where they were observed.

@illotum
illotum requested a review from jwhited May 27, 2026 03:02

@jwhited jwhited left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bounds check LGTM.

We should add the associated corpus entry to the fuzz test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants