Skip to content

btp: decode chunk headers from unaligned buffers - #79

Open
eastagiletracker wants to merge 1 commit into
Foundation-Devices:mainfrom
eastagiletracker:agile-board/btp-unaligned-header-decode
Open

btp: decode chunk headers from unaligned buffers#79
eastagiletracker wants to merge 1 commit into
Foundation-Devices:mainfrom
eastagiletracker:agile-board/btp-unaligned-header-decode

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes fixing btp::Chunk::decode, which rejects perfectly valid chunks when the caller hands it an unaligned buffer. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/326. You can sign in with your GitHub ID to claim ownership of the project.

The defect

Header::from_bytes reads the 8-byte BTP header with bytemuck::try_from_bytes, which is a reference cast and therefore requires the input slice to satisfy Header's 2-byte alignment. Chunk::decode passes &data[..HEADER_SIZE] straight through, so whether a chunk decodes depends on the address of the caller's buffer rather than on its contents. A receive path that hands the decoder a sub-slice of a larger packet buffer — say an application payload sitting at offset 3 of an ATT_MTU packet, ATT_MTU being APP_MTU + 3 in the consts crate — lands on an odd address, and then every chunk it delivers is rejected with DecodeError::InvalidHeader. Nothing about those bytes is malformed, so the peer keeps sending, Dechunker never fills, and the transfer stalls with no useful error. The existing tests miss it because they always decode from the start of a fresh allocation, which is aligned.

Reproduced on main at a0615c6, by copying a chunk produced by chunk() into a buffer at an odd offset and decoding that sub-slice:

$ cargo test -p btp unaligned
running 2 tests
test tests::decode_unaligned_chunk ... FAILED
test tests::receive_unaligned_chunks ... FAILED

---- tests::decode_unaligned_chunk stdout ----
thread 'tests::decode_unaligned_chunk' panicked at btp/src/tests.rs:277:40:
decoding must not depend on buffer alignment: InvalidHeader

---- tests::receive_unaligned_chunks stdout ----
thread 'tests::receive_unaligned_chunks' panicked at btp/src/tests.rs:291:14:
unaligned chunk should be received: Decode(InvalidHeader)

The change

Header::from_bytes now reads the header by value with bytemuck::try_pod_read_unaligned and returns Option<Header>; Chunk::decode drops the matching deref and moves the header into the Chunk. Header is Pod and Copy and was already being copied into the Chunk, so aligned input decodes byte-for-byte as it did before, and the size validation is unchanged — a slice that is not exactly HEADER_SIZE still yields DecodeError::InvalidHeader. from_bytes is private, so no public signature moves; the diff outside the tests is two lines.

Verification, all on the branch tree: the two regression tests above fail on main and pass with the change, covering both Chunk::decode directly and a full multi-chunk message reassembled through Dechunker::receive on odd-addressed slices. cargo test --workspace --all-features was run on a clean checkout before the change and again after — 112 tests passing before, 114 after (the two new ones), no test that passed before fails now. cargo fmt --all -- --check and cargo clippy --all-targets --all-features -- -D warnings are both clean, matching the checks in your Rust workflow. Nothing in api, backup-shard or quantum-link-macros is touched, so this should not conflict with the QLV2 work in flight.

How this was managed

We tracked this work on a live board imported from this repository's own pull requests and labels — 78 stories, one per pull request, plus the label they carry — and the fix itself is this story:

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

Chunk::decode read the header with bytemuck::try_from_bytes, which requires
the input slice to meet Header's 2-byte alignment. A receiver that passes a
sub-slice of a larger packet buffer (an ATT payload sitting at +3, for
instance) hands us an odd address, so every otherwise valid chunk was
rejected with DecodeError::InvalidHeader and the message never reassembled.

Read the header by value with try_pod_read_unaligned instead. Header is Copy
and was already copied into the Chunk, so aligned input behaves exactly as
before.
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.

1 participant