Skip to content

Disconnect LE credit-based channels on invalid SDU lengths - #999

Merged
zxzxwu merged 1 commit into
google:mainfrom
deadcaf3:worktree-fix-l2cap-buffer
Oct 5, 2026
Merged

zxzxwu merged 1 commit into
google:mainfrom
deadcaf3:worktree-fix-l2cap-buffer

Conversation

@deadcaf3

@deadcaf3 deadcaf3 commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Problem

LeCreditBasedChannel.on_pdu used an SDU length of 0 to mean "length not known yet". A PDU announcing a zero-length SDU therefore never completed, and every PDU that followed was appended to the same buffer without limit. Separately, an SDU length above the channel's MTU was accepted and the SDU delivered.

Reproduction on a channel with an MTU of 100 bytes:

  • 2000 PDUs of 50 bytes, the first announcing SDU length 0: 0 SDUs delivered, buffer growing with every PDU.
  • One SDU of 5000 bytes: delivered in full.

Change

All in LeCreditBasedChannel.on_pdu:

  • A zero-length SDU is delivered as an empty SDU.
  • An SDU length above the MTU, or more payload than the SDU length announced, sends a disconnection request. This resolves the existing TODO: we should disconnect.
  • PDUs received while the channel is not connected are dropped, as the log message already said. Without this, a peer that ignores the disconnection request could keep feeding the channel.

Testing

  • New tests in tests/l2cap_test.py: test_zero_length_sdu and test_invalid_sdu_length (3 cases). All 4 fail without the fix and pass with it.
  • Full test suite: 1020 passed, 9 skipped.
  • black -S --check, ruff check, and pylint -E are clean; mypy reports nothing for the changed files.

An SDU length of 0 was also used to mean "length not known yet", so a
PDU announcing a zero-length SDU never completed, and every PDU that
followed was appended to it without limit. An SDU length above the
channel's MTU was accepted and the SDU delivered.

A zero-length SDU is now delivered as an empty SDU. An SDU length above
the MTU, or more payload than the SDU length announced, now sends a
disconnection request, as the TODO asked. PDUs received while the
channel is not connected are now dropped, as the log message already
said, so a peer that ignores the request cannot keep feeding the channel.

@zxzxwu zxzxwu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks!

@zxzxwu
zxzxwu self-requested a review October 5, 2026 08:37
@zxzxwu
zxzxwu merged commit 3514925 into google:main Oct 5, 2026
36 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.

2 participants