Skip to content

Commit

Permalink
Fix null-deref crash in OnSessionEstablishmentError. (#14654)
Browse files Browse the repository at this point in the history
We ended up crashing because
mServer->getBleLayerObject()->mBleEndPoint was null.

The changes mostly remove the changes from #9845 because those changes
made us try to close some random "whatever last BLE endpoint was
created", even if it's already closed or has nothing to do with our
PASE session.

So instead, just do the same thing as we do for CASE establishment and
tear down all BLE sessions, if we need to tear down anything at all.

The remaining changes are making ReleaseBleConnection private again
and removing the broken mBleEndPoint member from BleLayer.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jul 12, 2023
1 parent 2a1861e commit bb0efb9
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void CommissioningWindowManager::OnSessionEstablishmentError(CHIP_ERROR err)
ChipLogError(AppServer, "Commissioning failed (attempt %d): %s", mFailedCommissioningAttempts, ErrorStr(err));

#if CONFIG_NETWORK_LAYER_BLE
mServer->getBleLayerObject()->mBleEndPoint->ReleaseBleConnection();
mServer->GetBleLayerObject()->CloseAllBleConnections();
#endif
if (mFailedCommissioningAttempts < kMaxFailedCommissioningAttempts)
{
Expand Down
2 changes: 1 addition & 1 deletion src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class Server
TransportMgrBase & GetTransportManager() { return mTransports; }

#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * getBleLayerObject() { return mBleLayer; }
Ble::BleLayer * GetBleLayerObject() { return mBleLayer; }
#endif

CommissioningWindowManager & GetCommissioningWindowManager() { return mCommissioningWindowManager; }
Expand Down
2 changes: 1 addition & 1 deletion src/ble/BLEEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ class DLL_EXPORT BLEEndPoint
bool ConnectionObjectIs(BLE_CONNECTION_OBJECT connObj) { return connObj == mConnObj; }
void Close();
void Abort();
void ReleaseBleConnection();

private:
BleLayer * mBle; ///< [READ-ONLY] Pointer to the BleLayer object that owns this object.
Expand Down Expand Up @@ -224,6 +223,7 @@ class DLL_EXPORT BLEEndPoint
// Close functions:
void DoCloseCallback(uint8_t state, uint8_t flags, CHIP_ERROR err);
void FinalizeClose(uint8_t state, uint8_t flags, CHIP_ERROR err);
void ReleaseBleConnection();
void Free();
void FreeBtpEngine();

Expand Down
2 changes: 0 additions & 2 deletions src/ble/BleLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ CHIP_ERROR BleLayer::Init(BlePlatformDelegate * platformDelegate, BleConnectionD
#if CHIP_ENABLE_CHIPOBLE_TEST
mTestBleEndPoint = NULL;
#endif
mBleEndPoint = NULL;

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -427,7 +426,6 @@ CHIP_ERROR BleLayer::NewBleEndPoint(BLEEndPoint ** retEndPoint, BLE_CONNECTION_O
#if CHIP_ENABLE_CHIPOBLE_TEST
mTestBleEndPoint = *retEndPoint;
#endif
mBleEndPoint = *retEndPoint;

return CHIP_NO_ERROR;
}
Expand Down
2 changes: 0 additions & 2 deletions src/ble/BleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,6 @@ class DLL_EXPORT BleLayer
BLEEndPoint * mTestBleEndPoint;
#endif

BLEEndPoint * mBleEndPoint;

private:
// Private data members:

Expand Down
2 changes: 0 additions & 2 deletions src/transport/raw/BLE.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ class DLL_EXPORT BLEBase : public Base, public Ble::BleLayerDelegate

CHIP_ERROR SetEndPoint(Ble::BLEEndPoint * endPoint) override;

Ble::BLEEndPoint * GetEndPoint() { return mBleEndPoint; }

private:
void ClearState();

Expand Down

0 comments on commit bb0efb9

Please sign in to comment.