* [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync @ 2022-05-20 18:37 Luiz Augusto von Dentz 2022-05-20 19:11 ` bluez.test.bot ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Luiz Augusto von Dentz @ 2022-05-20 18:37 UTC (permalink / raw) To: linux-bluetooth From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> 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@intel.com> --- 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; -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-20 18:37 [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync Luiz Augusto von Dentz @ 2022-05-20 19:11 ` bluez.test.bot 2022-05-24 12:30 ` patchwork-bot+bluetooth 2022-05-24 14:48 ` Ahmad Fatoum 2 siblings, 0 replies; 8+ messages in thread From: bluez.test.bot @ 2022-05-20 19:11 UTC (permalink / raw) To: linux-bluetooth, luiz.dentz [-- Attachment #1: Type: text/plain, Size: 1100 bytes --] This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=643739 ---Test result--- Test Summary: CheckPatch PASS 1.85 seconds GitLint PASS 1.06 seconds SubjectPrefix PASS 0.89 seconds BuildKernel PASS 37.24 seconds BuildKernel32 PASS 33.03 seconds Incremental Build with patchesPASS 43.64 seconds TestRunner: Setup PASS 538.97 seconds TestRunner: l2cap-tester PASS 18.95 seconds TestRunner: bnep-tester PASS 6.88 seconds TestRunner: mgmt-tester PASS 111.80 seconds TestRunner: rfcomm-tester PASS 10.68 seconds TestRunner: sco-tester PASS 10.41 seconds TestRunner: smp-tester PASS 10.40 seconds TestRunner: userchan-tester PASS 7.14 seconds --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-20 18:37 [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync Luiz Augusto von Dentz 2022-05-20 19:11 ` bluez.test.bot @ 2022-05-24 12:30 ` patchwork-bot+bluetooth 2022-05-24 14:48 ` Ahmad Fatoum 2 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+bluetooth @ 2022-05-24 12:30 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Fri, 20 May 2022 11:37:13 -0700 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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. > > [...] Here is the summary with links: - [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync https://git.kernel.org/bluetooth/bluetooth-next/c/c9f73a2178c1 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-20 18:37 [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync Luiz Augusto von Dentz 2022-05-20 19:11 ` bluez.test.bot 2022-05-24 12:30 ` patchwork-bot+bluetooth @ 2022-05-24 14:48 ` Ahmad Fatoum 2022-05-24 15:55 ` Ahmad Fatoum 2 siblings, 1 reply; 8+ messages in thread From: Ahmad Fatoum @ 2022-05-24 14:48 UTC (permalink / raw) To: Luiz Augusto von Dentz, linux-bluetooth; +Cc: Pengutronix Kernel Team On 20.05.22 20:37, Luiz Augusto von Dentz wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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@intel.com> 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@pengutronix.de> Bluetooth: hci0: Opcode 0x200d failed: -110 Bluetooth: hci0: request failed to create LE connection: err -110 now, whereas before it crashed the kernel. 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 | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-24 14:48 ` Ahmad Fatoum @ 2022-05-24 15:55 ` Ahmad Fatoum 2022-05-24 18:08 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 8+ messages in thread From: Ahmad Fatoum @ 2022-05-24 15:55 UTC (permalink / raw) To: Luiz Augusto von Dentz, linux-bluetooth; +Cc: Pengutronix Kernel Team 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@intel.com> >> >> 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@intel.com> > > 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@pengutronix.de> > > 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? 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 | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-24 15:55 ` Ahmad Fatoum @ 2022-05-24 18:08 ` Luiz Augusto von Dentz 2022-05-24 22:01 ` Ahmad Fatoum 0 siblings, 1 reply; 8+ messages in thread From: Luiz Augusto von Dentz @ 2022-05-24 18:08 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: linux-bluetooth, Pengutronix Kernel Team Hi Ahmad, On Tue, May 24, 2022 at 8:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> 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@intel.com> > >> > >> 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@intel.com> > > > > 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@pengutronix.de> > > > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-24 18:08 ` Luiz Augusto von Dentz @ 2022-05-24 22:01 ` Ahmad Fatoum 2022-05-24 23:44 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 8+ messages in thread From: Ahmad Fatoum @ 2022-05-24 22:01 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Pengutronix Kernel Team Hello Luiz, On 24.05.22 20:08, Luiz Augusto von Dentz wrote: > On Tue, May 24, 2022 at 8:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: >> On 24.05.22 16:48, Ahmad Fatoum wrote: >> 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. Yes, that cleanup in le_conn_complete_evt() needs to be removed. I am talking about the other hunk in hci_conn.c: - 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); Can this be dropped for v5.17? Cheers, Ahmad -- 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 | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync 2022-05-24 22:01 ` Ahmad Fatoum @ 2022-05-24 23:44 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 8+ messages in thread From: Luiz Augusto von Dentz @ 2022-05-24 23:44 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: linux-bluetooth, Pengutronix Kernel Team Hi Ahmad, On Tue, May 24, 2022 at 3:01 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > Hello Luiz, > > On 24.05.22 20:08, Luiz Augusto von Dentz wrote: > > On Tue, May 24, 2022 at 8:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > >> On 24.05.22 16:48, Ahmad Fatoum wrote: > >> 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. > > Yes, that cleanup in le_conn_complete_evt() needs to be removed. > I am talking about the other hunk in hci_conn.c: > > - 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); > > > Can this be dropped for v5.17? I guess it should be alright but perhaps keep if (conn != hci_lookup_le_connect(hdev)) just in case. > Cheers, > Ahmad > > -- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-24 23:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-20 18:37 [PATCH-stable] Bluetooth: hci_conn: Fix hci_connect_le_sync Luiz Augusto von Dentz 2022-05-20 19:11 ` bluez.test.bot 2022-05-24 12:30 ` patchwork-bot+bluetooth 2022-05-24 14:48 ` Ahmad Fatoum 2022-05-24 15:55 ` Ahmad Fatoum 2022-05-24 18:08 ` Luiz Augusto von Dentz 2022-05-24 22:01 ` Ahmad Fatoum 2022-05-24 23:44 ` Luiz Augusto von Dentz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.