Repository navigation
XM4 first-class support + parser fuzzing/hardening - #7
Merged
AmitRajput-Dev merged 16 commits intoOct 28, 2026
Merged
Conversation
…en tests In-progress work from the Codex session: XM4 (WH-1000XM4 v1) protocol support, 14 verified bug fixes (connection-thread crashes, command/notification races), 18 exact Bluetooth wire-format golden tests, and protocol-conformance harness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…parser Found uncommitted in the working tree (authored by the earlier audit pass, not this session); committed verbatim so the tree is clean and the analysis is not lost. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Message lives in the CommandSerializer namespace, and its seqNumber field is unsigned char; the harness used the unqualified name and narrowed through a signed char cast, so the libFuzzer target never built. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- -max_len referenced MAX_BLUETOOTH_MESSAGE_SIZE as a shell variable, which is never set, so 'set -u' killed the script before fuzzing; derive it from Constants.h instead. - Apple's clang++ has no libFuzzer runtime and the failure only appears at link time; probe with a real link and fall back to a Homebrew LLVM clang++. - When no libFuzzer-capable compiler exists at all, build driver_main.cpp: a deterministic seed-replay + structured-mutation driver (bit/byte flips, truncation, length-field tampering, escape/marker injection, splicing) over the same LLVMFuzzerTestOneInput entry point under ASan/UBSan. - Suppress libFuzzer's own never-joined RSS-monitor thread (lsan.supp) so leak detection stays on for the parser; skip detect_leaks on Apple ASan, which does not support it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both the fuzz campaign (8.2M execs, zero parser bugs) and the static audit flagged the same latent issue: three malformed-frame paths in unpackBtMessage threw bare std::runtime_error instead of the RecoverableException used by the sibling checks. Safe only because every current call site catches std::exception; a future site narrowing its catch would turn attacker- controlled input into std::terminate (remote DoS). Made all three throw RecoverableException(msg, true), matching the existing malformed-frame throws, and added regression assertions. Also broadened the LSan suppression from fuzzer::StartRssThread to fuzzer:: to cover an intermittent symbolizer leak in libFuzzer's PC printer (parser never on the stack). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…corruption) Covers the gap between the exact golden fixtures and the crash-only fuzzer with relational properties over programmatically generated inputs (fixed-seed splitmix64, fully deterministic): - P1 round-trip: unpack(strip_markers(pack(p, dt, seq))) recovers (p, dt, seq & 0xFF); pack output is also structurally verified against an independent re-implementation of the documented layout (declared BE size, mod-256 checksum, header bytes). - P2 escaping transparency: every escapable byte (60/61/62) at every payload position round-trips; the frame interior never contains a raw marker byte. - P3 seq domain: 0..255 round-trips exactly; wider values pack byte-identically to seq & 0xFF. - P4 framing: +/-1-byte damage at either end and marker-attached frames are all rejected with RecoverableException. - P5 determinism and inverse consistency: pack is byte-identical across calls; repacking an unpacked message reproduces the frame. - P6 corruption: exhaustive single-byte replacement (all 255 values, every position, four frames incl. escape-dense ones) plus per-bit flips on a 512-byte-payload frame - 31270 corrupted bodies, all rejected recoverably, none silently parsed. - P7 size boundary: 2039 plain payload bytes fill the 2048 frame exactly; 2040 overflows; escape-heavy payloads halve the budget (1016 x 0x3C fits, 1020 does not). Wired into Client/tests/run-macos.sh after the conformance suite, same clang++ -Wall -Wextra -Werror invocation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The RFCOMM data delegate buffered every frame unconditionally, but recv() only drains the queue during a command exchange, so a device streaming unsolicited frames grew receivedBytes without bound. Cap the buffered total at RECEIVE_BUFFER_CAP, dropping the oldest chunks; the wrapper's marker scan resynchronizes on what survives. A zero-length frame also made recv() return 0, which BluetoothWrapper reads as a closed connection - drop those at the delegate boundary instead of fabricating a disconnect. Covered by new offline tests that inject frames exactly as the delegate would, with no Bluetooth channel opened. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnKJraRSytaMh5E2G57ifx
[[device addressString] UTF8String] on a nil addressString hands std::string a null char*, which is undefined behaviour. getConnectedDevices() already guards the same call; apply the same check on the scan-and-connect path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnKJraRSytaMh5E2G57ifx
56k relational checks (round-trip, escaping transparency, single-byte corruption rejection, framing/truncation) over the protocol layer. All green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bounds the macOS connector receive queue (fixes unbounded attacker-driven allocation / OOM), drops zero-length frames, guards a nil addressString. Regression tests for the buffer cap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> # Conflicts: # Client/tests/run-macos.sh
Every integer step of an ambient-level or custom-EQ slider drag was enqueuing a full Bluetooth exchange on the serial command queue (~70ms measured per exchange, up to 2500ms worst-case each), so a 20-step drag queued ~1.4s+ of stale intermediate sends that kept hitting the headset after the user released. HeadphonesModel now mirrors the in-flight gate the ImGui client already uses (CrossPlatformGUI _sendFeatureCommand): the first change sends immediately; changes arriving while an exchange is in flight only overwrite a pending snapshot; when the exchange completes, at most one more send fires, and only if the pending snapshot differs from what was last sent. The pending value is an explicit snapshot (not re-read from the model at completion time) so syncFromBridge or the 2s poll overwriting @published state cannot clobber the user's latest drag value. All gate state is main-thread-confined - bridge completions arrive on the main queue - so no locks are needed. Isomorphism: end-state is unchanged - the user's final value always reaches the device (on failure with a newer pending value, the pending value is still sent; pending is cleared before resending so it is never sent twice), single clicks still send immediately with no added latency, and a drag now costs <=2-3 exchanges instead of one per step. Discrete controls (mode buttons, toggles) are untouched. disconnect() drops pending snapshots so an intentional disconnect does not fire a doomed resend. Verified: 4 macOS test suites green (protocol safety, 18 golden, 56109 metamorphic checks, connector buffer), 12s fuzz smoke exit 0, Release xcodebuild clean with no new warnings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every 2s poll tick and every interaction completion wrote 8-12 @published properties unconditionally; each setter fires objectWillChange, so the whole ContentView body re-evaluated every tick even when nothing changed. The wire poll only refreshes ambient state, so most per-tick writes (EQ/DSEE re-read from cached bridge values) were provably no-ops that still forced a full re-render — the recurring 0.4-1.2% idle CPU blips in .hotspots/evidence/connected_idle_cpu_series.txt, pure waste on battery. Mechanism: a generic assignIfChanged(keyPath, value) helper writes a @published property only when the new value differs from the current one, applied to both the refreshDynamic poll completion and syncFromBridge. The eqBands candidate array is still built the same way, then compared before assignment instead of assigned unconditionally. Isomorphism: reads, read order, and poll cadence are untouched — only writes became conditional. Equal value -> no write -> visible state identical; different value -> write fires exactly as before, so every device-side change still lands in the UI on the same tick it did before. The slider-coalescing convergence from the previous commit is preserved: after a successful push, syncFromBridge still overwrites mode/ambientLevel whenever device truth differs from the UI, and skipping the write when they are equal is indistinguishable. First sync after connect still fires every write, since all values transition from their defaults. Verified: 4 test suites green, fuzz smoke exit 0, Release build succeeded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Release configuration never chose a C++/ObjC++ optimization level: GCC_OPTIMIZATION_LEVEL was absent (Xcode default -Os), DEAD_CODE_STRIPPING resolved to NO, and SWIFT_COMPILATION_MODE was unpinned. Every C++ profile of the protocol stack was bounded by unchosen codegen (HOTSPOTS rank 3). Changes (Release blocks only, Debug untouched): - project-level Release: GCC_OPTIMIZATION_LEVEL = 2 (explicit -O2 — deliberate speed-over-size for the protocol paths; -O3 skipped, no measured win to justify vectorization bloat) - project-level Release: DEAD_CODE_STRIPPING = YES - target-level Release: SWIFT_COMPILATION_MODE = wholemodule (pins the Xcode-default; SWIFT_OPTIMIZATION_LEVEL already -O) Considered and skipped: LTO (unmeasured benefit vs real build-time and debuggability cost), C++ standard cleanup (gnu++14 vs c++17 hygiene, not this pass's lever). Isomorphism: config-only, zero source changes — -O2 vs -Os is a codegen-level change with identical observable semantics, and dead-code stripping removes only unreferenced symbols. Verification: - Resolved Release (was: GCC_OPTIMIZATION_LEVEL unset, DEAD_CODE_STRIPPING = NO, no SWIFT_COMPILATION_MODE) → now GCC_OPTIMIZATION_LEVEL = 2, DEAD_CODE_STRIPPING = YES, SWIFT_COMPILATION_MODE = wholemodule - Resolved Debug unchanged (0 / NO / -Onone) - Clean Release build: BUILD SUCCEEDED, no new warnings - Binary: 853,392 → 730,304 bytes (-14.4%; dead-strip outweighs -O2 growth) - Tests: all 4 suites green (protocol safety, 18 golden, 56,109 metamorphic checks, connector buffer); fuzz smoke exit 0 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…a sourceless run loop Live profiling (10s @1ms, connected to a physical XM4) proved the connectToMac pump loop dead weight: all 8537/8537 connector-thread samples sat in __psynch_cvwait at the wait_for, zero in the runUntilDate pump, while inbound RFCOMM data was delivered by -[IOBluetoothRFCOMMChannel stream:handleEvent:] on the MAIN thread's run loop (CFSocket source scheduled there). The connector thread's run loop has no sources; it never delivered anything. Change: replace the 100ms-pump/1000ms-timed-poll alternation with a plain cv wait on running. Removing the timed re-poll unmasks a lost-wakeup race (running is atomic, stored lock-free by notifiers; a notify landing between the waiter's predicate check and its block was previously bounded at 1s, now would sleep forever), so every running=false setter — disconnect(), handleChannelClosed(), connect()'s catch — now stores under disconnectionMutex before notifying. Hazards traced: 1. Channel lifetime: the wait keeps connectToMac's frame (and its strong `channel` ref) alive exactly as the loop did; release ordering at frame exit vs running=false/notify unchanged. closeConnection() retains its own strong ref before closeChannel; disconnect() still joins before the connector object is destroyed (destructor + connect() catch paths). 2. Open-complete: connectPromise.set_value() fires synchronously after openRFCOMMChannelAsync returns success, before the wait — connect() never depended on the delegate callback; rfcommChannelOpenComplete: exists only under SHC_DEBUG_PROTOCOL and only logs. All delegate callbacks arrive on main (profiled), so the pump was load-bearing for nothing. 3. connect() error path kept coherent: catch stores running=false under the mutex like the other setters. 4. recv()'s receiveDataConditionVariable path untouched. Isomorphism: connect, data flow, and disconnect end states unchanged (data never flowed through this thread). Disconnect wakeup is now immediate instead of worst-case ~1s, and the thread does zero spurious wakeups (was ~2/s). Offline test added: both running=false setters wake the exact connectToMac wait, 200 interleavings, lost wakeup = test failure. Verified: 4 test suites green, FUZZ_SECONDS=10 fuzz exit 0, Release xcodebuild BUILD SUCCEEDED. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Speak-to-Chat pauses audio whenever the mics hear speech, which is hostile to anyone who dictates constantly — every utterance stops the music. This adds a persisted preference (UserDefaults key KeepSpeakToChatOffOnConnect) that forces S2C off once per connect. Mechanism: connect() arms a per-connection flag; the refreshStatus completion — which fires a second time after probeCapabilities confirms optional features — consumes the flag the first time hasSpeakToChat is true, and only then sends a single setSpeakToChat(false) if the pref is on and the device reports S2C enabled. Devices without S2C, or with it already off, never receive a write; the flag also clears on disconnect, so the disable fires at most once per connection. The manual toggle is untouched: turning S2C on mid-session sticks until the next connect. The pref defaults to true per this build's user request; upstream may prefer defaulting to false — it's a one-line change (the `?? true` in HeadphonesModel.swift). The secondary "Keep off on connect" toggle sits under the S2C switch and is hidden with it when the capability is absent, matching the existing gating pattern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
v2 devices only, byte layout proven by Gadgetbridge on the WH-ULT900N
(ULT WEAR): commits 8cbc309346 ("WIP add ConnectTwoDevices feature"),
e8520865f3 ("fix: setting Connect Two Devices"), 5d7b37d1b7 ("fix read
Connect Two Devices"), files SonyProtocolImplV2.java and
SonyHeadphonesProtocol.java. State rides touch-sensor subtype d1 with an
inverted enable bit (GET d6 d1 -> RET d7 d1 00 <00=on/01=off>); a SET
sends d8 d1 00 <00=on/01=off> then the fixed apply commit 98 00 06 01
for both directions. The system-control RET (97 00 06) always reports 01
and is never used as a state source.
v1 (WH-1000XM4) gets no multipoint on purpose: Gadgetbridge's
SonyProtocolImplV1 explicitly logs "Connect two devices not implemented
for V1" and returns null, so no proven v1 frames exist. The v1 test now
asserts the XM4 path never emits any multipoint opcode and that
setMultipoint throws.
The probe (d6 d1) is appended to the v2 probeCapabilities sequence; the
toggle is capability-gated and mirrors the device-persistent state with
no app-enforced default. Full research trail with the negative findings
in docs/MULTIPOINT_RESEARCH.md; exact framing pinned in
conformance/fixtures/ult900n_v2_multipoint.golden.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
|
Merged via #30. Thank you, this is the most thorough contribution the project has had. The golden and metamorphic suites and the fuzz harness made the rest of the release much safer to land. One change when merging alongside #28 (which added multipoint discovery): "Connect to 2 devices" is now found by name, by looking for Ships in v0.4.0. |
fredybp
pushed a commit
to fredybp/SonyBridge
that referenced
this pull request
Nov 4, 2026
requestBattery() probed the single-battery sub-type first and returned on any reply, so TWS earbuds never reached the dual probe and L/R stayed unset. Probe order now follows the model name (WF-*/LinkBuds try dual first), with a fallback to the other layout either way. - add DUAL2 (0x01) for the WF-C500/C510/C700N line - keep the per-earbud charging flags instead of discarding them - treat a level of 0 as "not reporting" (docked), not a flat battery - read the charging case (0x0a) - re-read battery on the dynamic poll so it stops going stale (v2 only: 0x22 is POWER_OFF on v1) Sub-types and byte offsets verified against a WF-1000XM5 by docking one earbud and watching byte [4] track it; captured frames are in Constants.h. (cherry picked from commit ff1605d) Conflict resolution on top of AmitRajput-Dev#7/AmitRajput-Dev#28 (by maintainer): - keep AmitRajput-Dev#7's v1 battery branch (10 00); per-earbud probing is v2-only - a 0/0 dual reply is a real answer (both earbuds docked), not "unsupported": rejecting it fell through to DUAL2/single probes that time out, stalling the command queue ~5s on every battery poll - battery re-read runs on the serial command queue (no _dynamicTick race) and model writes are change-gated Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fredybp
pushed a commit
to fredybp/SonyBridge
that referenced
this pull request
Nov 4, 2026
rfcommchannel was a non-owning __bridge pointer that was never cleared when the link dropped, so a poll already in flight would writeSync: to a released channel and crash (EXC_BAD_ACCESS / pointer auth failure). Docking an earbud was enough to trigger it. Retain the channel and clear it under a mutex, so a concurrent send sees either a live channel or nil; send() now raises a recoverable error instead. Also fixes a self-join: the RFCOMM delegate runs on uthread's own run loop, so rfcommChannelClosed -> disconnect() was joining that thread from itself. recv() now wakes on disconnect rather than waiting out its full timeout. (cherry picked from commit ac78f13) Conflict resolution on top of AmitRajput-Dev#7 (by maintainer): AmitRajput-Dev#7 already fixed the self-join and the recv wake-up; kept its version of those and applied this commit's core fix to it - the channel pointer is now an owning reference guarded by channelMtx (send/isConnected take a strong ref under the lock), instead of AmitRajput-Dev#7's unretained atomic pointer, which a remote close could still free under an in-flight send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fredybp
pushed a commit
to fredybp/SonyBridge
that referenced
this pull request
Nov 4, 2026
A short audio dropout was enough to desync the response stream for the rest of the session: changing NC/Ambient or an EQ preset reverted a second or two later, and DSEE wouldn't toggle at all. _readMessage() left the parser stranded whenever a chunk arrived garbled or truncated - bytes before the first START_MARKER were concatenated into the message, and a second start marker threw. 60/61/62 are escaped inside a frame, so a start marker unambiguously begins one; resync on it in both cases, and cap the buffer so a stream that never delivers an END_MARKER can't grow it forever. _waitForAck() read exactly one frame and assumed it was the ACK. The device pushes unsolicited notifications on the same stream (a state change, a press of the headset's own button), so it could take _seqNumber from the wrong frame - and the device dedupes on that sequence number, silently discarding the next command as a duplicate. Wait for an actual ACK frame instead; skipped notifications are still acked back by _readMessage(). Also: - requestAmbientState() matches sub-type 0x17 rather than any 0x67, so a notification with a different layout is no longer parsed as the reply - the read-back no longer overwrites a change that hasn't been sent yet. It cleared the pending change too (isChanged() went false), so the command was never sent at all. discardAmbientChanges() releases that state when a write genuinely fails, so polling can't stay suppressed - the 2s poll stopped republishing dsee/eqPreset/eqBands, which it never re-read - they came straight back from the C++ cache, which only updates once a write succeeds. Failed writes roll back explicitly with the error instead, and the EQ sliders no longer fight the timer mid-drag (cherry picked from commit 40e65d4) Conflict resolution on top of AmitRajput-Dev#7/AmitRajput-Dev#28 (by maintainer): _waitForAck kept AmitRajput-Dev#7's equivalent ACK loop (now on MAX_FRAMES_PER_TRANSACTION); parser keeps this commit's resync-on-START_MARKER plus AmitRajput-Dev#7's noise skipping; the poll's change-gated writes and AmitRajput-Dev#7's slider coalescing are kept, with this commit's "sync either way" on a failed ambient write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fredybp
pushed a commit
to fredybp/SonyBridge
that referenced
this pull request
Nov 4, 2026
fixes AmitRajput-Dev#11) WH-1000XM6 answers the 66 17 noise-control inquiry with all zeros in every mode, which the 2s poll parsed as "Off" - so Noise Cancelling / Ambient switched the headset but the UI snapped straight back. It reports the same layout under inquired type 0x19. Its equalizer uses inquired type 0x04 with 10 raw bands (0..12) and no Clear Bass slot; the 5-band 58 00 frame is ACKed but ignored, which is why presets highlighted without changing the sound. - 66 19 on the XM6; any v2 reply whose byte [2] isn't 01 is ignored, so a zeroed answer can no longer read as "Off" on other models either - 56/58 04 ten-band read/preset/custom on the XM6; band count and the ten-band flag exposed to the UI, which shows 10 raw sliders, no Clear Bass, and only the Off/Custom presets (other XM6 preset codes are unnamed) - tests: XM6 frames, the zeroed-0x17 guard, and AmitRajput-Dev#7's multipoint test moved to the name-based general-settings slot lookup Layouts from btsnoop captures and hardware re-tests on a WH-1000XM6 (fw 3.1.5) by marconvcm/sony-device-center contributors (#44, #74). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings WH-1000XM4 (v1) support to first-class status, hardens the untrusted Bluetooth parsing layer, adds four measured performance fixes, and ships two features (Speak-to-Chat default control, multipoint toggle for v2 devices).
Protocol & correctness
CommandSerializer::unpackBtMessageunder ASan/UBSan with a no-libFuzzer fallback driver; an 8.2M-execution campaign found zero parser bugs. (The scaffolded harness didn't previously compile — fixed.)Client/fuzz/AUDIT_FINDINGS.md): no memory-safety/integer/escape/round-trip defects.Security hardening
addressStringguard on the connect path.RecoverableExceptionuniformly (was barestd::runtime_errorin 3 spots — a latent std::terminate/DoS if a future call site narrowed its catch); locked by regression tests.Performance (profiled first — evidence-driven, one lever per commit)
Profiled the Release app live against a physical XM4 (idle-connected CPU ~0.3%, ~70ms per poll exchange):
@Publishedwrites gated to real changes — the 2s poll re-wrote 8 properties unconditionally, re-rendering the whole view when provably nothing changed.-O2,DEAD_CODE_STRIPPING=YES,SWIFT_COMPILATION_MODE=wholemodule(C++ was silently at the -Os default); binary −14.4%.Features
?? truein HeadphonesModel.swift).docs/MULTIPOINT_RESEARCH.mdand the golden fixtures). The v1/XM4 path is deliberately untouched: no proven v1 frames exist anywhere (GadgetBridge explicitly declines v1), and this PR does not invent wire bytes. Capability-probed, exact-byte SET/GET tests + a 6-case golden fixture.Verification
sh Client/tests/run-macos.sh).Client/fuzz/run-macos.sh).🤖 Generated with Claude Code