Skip to content

isotp: extend long reassembly buffers in place - #5109

Merged
polybassa merged 2 commits into
secdev:masterfrom
KernelClint:perf/isotp-extended-reassembly
Aug 27, 2026
Merged

isotp: extend long reassembly buffers in place#5109
polybassa merged 2 commits into
secdev:masterfrom
KernelClint:perf/isotp-extended-reassembly

Conversation

@KernelClint

Copy link
Copy Markdown
Contributor

The opt-in software ISO-TP socket reassembles fragmented application messages carried over CAN. A
32-bit First Frame length starts an attacker-sized message at
scapy/contrib/isotp/isotp_soft_socket.py:986-1005,
and every valid Consecutive Frame copies the growing immutable receive buffer at
scapy/contrib/isotp/isotp_soft_socket.py:1061-1070.

From 131,072 to 1,048,576 application bytes, median receive processing grew from 32.10 ms to
1,206.87 ms, with a 2.06 exponent and a 2.3% noise floor. Patched times were 7.44 ms to 59.91 ms,
with a 0.99 exponent and a 6.3% noise floor.

This change uses a bytearray only for extended-length messages, extends it in place, and converts
once on completion. The focused regression failed on the unmodified revision and passed with the
patch.

AI-Assisted: yes (GPT-5.6-Cyber)
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.60%. Comparing base (ba8641a) to head (492e6bd).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5109   +/-   ##
=======================================
  Coverage   80.59%   80.60%           
=======================================
  Files         390      390           
  Lines       96892    96895    +3     
=======================================
+ Hits        78094    78098    +4     
+ Misses      18798    18797    -1     
Files with missing lines Coverage Δ
scapy/contrib/isotp/isotp_soft_socket.py 84.96% <100.00%> (+0.13%) ⬆️

... and 11 files with indirect coverage changes

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

Comment thread scapy/contrib/isotp/isotp_soft_socket.py Outdated
Drops the length conditional and the isinstance branches it required.

AI-Assisted: yes (GPT-5.6-Cyber)
@KernelClint

Copy link
Copy Markdown
Contributor Author

Yes — you're right, and I've changed it in 492e6bd0.

I'd added the length check to keep the short-message path untouched, but measuring it, that isn't
what happens. Reassembling one normal two-frame message:

ns/message, three runs
unmodified 710, 720, 700
conditional 777, 729, 756
always bytearray 800, 748, 748

The two variants are indistinguishable — the spread between repeated runs of the same build is
wider than the gap between builds. So the condition bought nothing and cost three isinstance
branches and a union type.

Always using a bytearray removes all of that: rx_buf is Optional[bytearray] throughout and
converts to bytes once, where it is sent to the queue. test/contrib/isotp_soft_socket.uts
passes.

@polybassa
polybassa merged commit 95adadd into secdev:master Aug 27, 2026
23 checks passed
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