Hi Ahmad, On Tue, May 24, 2022 at 8:55 AM Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote: > > Hello Luiz, > > On 24.05.22 16:48, Ahmad Fatoum wrote: > > On 20.05.22 20:37, Luiz Augusto von Dentz wrote: > >> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > >> > >> The handling of connection failures shall be handled by the request > >> completion callback as already done by hci_cs_le_create_conn, also make > >> sure to use hci_conn_failed instead of hci_le_conn_failed as the later > >> don't actually call hci_conn_del to cleanup. > >> > >> Link: https://github.com/bluez/bluez/issues/340 > >> Fixes: 8e8b92ee60de5 ("Bluetooth: hci_sync: Add hci_le_create_conn_sync") > >> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > > > > A bit late, as I am not subscribed to linux-bluetooth and didn't notice this > > patch, but FWIW: Tested-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> > > > > Bluetooth: hci0: Opcode 0x200d failed: -110 > > Bluetooth: hci0: request failed to create LE connection: err -110 > > > > now, whereas before it crashed the kernel. > > I see now that this fix doesn't build for v5.17 because hci_conn_failed > was only introduced in v5.18. Can the hci_conn.c hunk be safely dropped? Are you talking about: if (status) { - hci_le_conn_failed(conn, status); + hci_conn_failed(conn, status); goto unlock; } You just need to replace hci_conn_failed with hci_le_conn_failed or well in the code above the end result is the same since it is not supposed to cleanup in the event handler. > Thanks, > Ahmad > > > > > Cheers, > > Ahmad > > > >> --- > >> net/bluetooth/hci_conn.c | 5 +++-- > >> net/bluetooth/hci_event.c | 8 +++++--- > >> 2 files changed, 8 insertions(+), 5 deletions(-) > >> > >> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > >> index 882a7df13005..ac06c9724c7f 100644 > >> --- a/net/bluetooth/hci_conn.c > >> +++ b/net/bluetooth/hci_conn.c > >> @@ -943,10 +943,11 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err) > >> > >> bt_dev_err(hdev, "request failed to create LE connection: err %d", err); > >> > >> - if (!conn) > >> + /* Check if connection is still pending */ > >> + if (conn != hci_lookup_le_connect(hdev)) > >> goto done; > >> > >> - hci_le_conn_failed(conn, err); > >> + hci_conn_failed(conn, err); > >> > >> done: > >> hci_dev_unlock(hdev); > >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > >> index 0270e597c285..af17dfb20e01 100644 > >> --- a/net/bluetooth/hci_event.c > >> +++ b/net/bluetooth/hci_event.c > >> @@ -5632,10 +5632,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, > >> status = HCI_ERROR_INVALID_PARAMETERS; > >> } > >> > >> - if (status) { > >> - hci_conn_failed(conn, status); > >> + /* All connection failure handling is taken care of by the > >> + * hci_conn_failed function which is triggered by the HCI > >> + * request completion callbacks used for connecting. > >> + */ > >> + if (status) > >> goto unlock; > >> - } > >> > >> if (conn->dst_type == ADDR_LE_DEV_PUBLIC) > >> addr_type = BDADDR_LE_PUBLIC; > > > > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- Luiz Augusto von Dentz
