Cordio BLE: GAP::cancelConnect() results in Hard Fault/memory corruption

[mg-unleashed]

Description of defect

Use of Gap::cancelConnect() results in memory corruption or Hard Fault

The calling of Gap::cancelConnect() results in an invalid message being created by DmConnClose().

Inside DmConnClose a message is created with the following header info:
event = DM_CONN_MSG_API_CLOSE
param = DM_CONN_ID_NONE (0).
status = 0
clientId = DM_CLIENT_ID_APP

The assignment of param = DM_CONN_ID_NONE is the cause of the error when the message is processed. When the message is by dm_conn.c:dmConnMsgHandler() pMsg->param is used to determine the ccb, however DM_CONN_ID_NONE is incorrectly processed by dmConnCcbById() and an invalid ccb pointer is returned. This results in dmConnSmExecute() operating on an invalid ccb pointer.

In my case, it generates a Hard Fault, but depending on the memory layout, memory corruption could also result.

Looking at this issue from a more systematic view, it seems cancelConnect() needs a connection identifier to know which connection to cancel, however this would require connect() to provide such an identifier. A possible solution would be to modify cancelConnect() to take the peer address as a parameter. This could likely serve as a pre-connection handle so that the ccb or connId could be looked up by Gap::cancel_connection_creation_)_.

Call stack to DmConnClose():
DmConnClose() at dm_conn.c:1,027 0x197cc
ble::pal::vendor::cordio::Gap<ble::generic::GenericGap<ble::pal::vendor::cordio::Gap, ble::pal::SecurityManager<ble::pal::vendor::cordio::CordioSecurityManager<ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> > >::cancel_connection_creation_() at CordioPalGap.tpp:278 0x1221e
ble::pal::Gap<ble::pal::vendor::cordio::Gap<ble::generic::GenericGap<ble::pal::vendor::cordio::Gap, ble::pal::SecurityManager<ble::pal::vendor::cordio::CordioSecurityManager<ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> > >, ble::generic::GenericGap<ble::pal::vendor::cordio::Gap, ble::pal::SecurityManager<ble::pal::vendor::cordio::CordioSecurityManager<ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> > >::cancel_connection_creation() at PalGap.h:1,598 0x13ba4
ble::generic::GenericGap<ble::pal::vendor::cordio::Gap, ble::pal::SecurityManager<ble::pal::vendor::cordio::CordioSecurityManager<ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >::cancelConnect_() at GenericGap.tpp:828 0x13ba4
ble::interface::Gap<ble::generic::GenericGap<ble::pal::vendor::cordio::Gap, ble::pal::SecurityManager<ble::pal::vendor::cordio::CordioSecurityManager<ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> >, ble::generic::GenericSecurityManager<ble::pal::vendor::cordio::CordioSecurityManager, ble::vendor::cordio::SigningEventMonitor> > >::cancelConnect() at Gap.tpp:311 0x13bac

Target(s) affected by this defect ?

NRF52

Toolchain(s) (name and version) displaying this defect ?

GCC9.2.1 (GNU Tools 9-2019-q4-major)

What version of Mbed-os are you using (tag or sha) ?

mbed-os-5.15.1

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Mbed-cli 1.10.4

How is this defect reproduced ?

1. Call Gap::connect()
2. Call Gap::cancelConnect() before a connection is established.

[ciarmcom]

@mg-unleashed thank you for raising this issue.Please take a look at the following comments:

Could you add some more detail to the description? A good description should be at least 25 words.

NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered.
Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2774

[0xc0170]

cc @mbed-os-pan Please review

[paul-szczepanek-arm]

Thank you for the detailed report and analysis. I will try to schedule this ASAP.

[paul-szczepanek-arm]

tracked https://jira.arm.com/browse/IOTPAN-651

[paul-szczepanek-arm]

Thank, this is now fixed: #13390.