Fix use-after-free segfault on BluezConnection pointer#72821
Draft
feasel0 wants to merge 3 commits into
Draft
Conversation
for more information, see https://pre-commit.ci
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #72821 +/- ##
=======================================
Coverage 56.62% 56.63%
=======================================
Files 1642 1642
Lines 113154 113159 +5
Branches 13235 13234 -1
=======================================
+ Hits 64079 64091 +12
+ Misses 49075 49068 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
PR #72821: Size comparison from d9d3108 to ac95f61 Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
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
1 participant
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.
What is wrong
SetUpCodePairercan discover the same commissionee over BLE and UDP/DNS-SD concurrently. If a UDP PASE attempt is already in progress (mWaitingForPASE == true) when BLE discovery succeeds, the old code queued the liveBluezConnection *inmDiscoveredParametersfor a later retry. At that point noBLEEndPointhad been created for the connection, so the BLE layer had no endpoint associated with that pointer.If the DUT closes the BLE link while the UDP PASE attempt is still in flight,
BluezEndpointremoves theBluezConnectionfrom its map and schedules it for deletion.BleLayer::HandleConnectionErrorlogs that there is no endpoint for the connection error and returns, leaving the queued raw pointer untouched. When UDP PASE later fails and the pairer retries the queued BLE entry, it can pass a freedBluezConnection *intoBleLayer::NewBleConnectionByObject. The resulting endpoint wraps a stale connection object, which can fail on the first write becausemC1is gone and then crash while trying to close the same freed connection.How it was fixed
In
SetUpCodePairer::OnDiscoveredDeviceOverBle, whenmWaitingForPASE == true, the live connection handle is no longer stored. Instead, the pairer queues a reconnectable BLE entry: a BLE peer address plus the matched long discriminator, when one was reported. When the deferred retry runs,ConnectToDiscoveredDevice()resolves the discriminator back to the matching setup payload and fills in the setup PIN/discriminator.DeviceCommissioner::EstablishPASEConnection()then uses those parameters to callBleLayer::NewBleConnectionByDiscriminator, starting a fresh BLE scan instead of reusing the old connection object.When
mWaitingForPASE == false, the existing fast path is preserved: the live connection handle is queued and consumed immediately, creating aBLEEndPointfor the active connection instead of leaving the raw pointer dormant.Testing
TestSetUpCodePairer.cpp gets a new test,
DeferredBleDiscoveryQueuesReconnectableParams, which:mWaitingForPASE = trueto simulate an in-flight PASE attemptOnDiscoveredDeviceOverBlewith a representative connection object and long discriminatorConnectionObjectand noDiscoveredObject— confirming the raw pointer was not storedSetUpCodePairerTestAccessis extended withGetDiscoveredParametersCount(),FrontDiscoveredParameters(), andCallOnDiscoveredDeviceOverBle()to expose the state needed by the new test without modifying production code visibility.Issues affected
Fixes #72252