| Message ID | 20211028191723.1.I94a358fc5abdb596412a2e22dd2b73b71f56fa82@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | Bluetooth: Fix removing adv when processing cmd complete | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/buildkernel | success | Build Kernel PASS |
| tedd_an/testrunnersetup | success | Test Runner Setup PASS |
| tedd_an/testrunnerl2cap-tester | success | Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerbnep-tester | success | Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnermgmt-tester | fail | Total: 468, Passed: 463 (98.9%), Failed: 5, Not Run: 0 |
| tedd_an/testrunnerrfcomm-tester | success | Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersco-tester | success | Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersmp-tester | success | Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunneruserchan-tester | success | Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0 |
Hi Archie, On Thu, Oct 28, 2021 at 4:17 AM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > If we remove one instance of adv using Set Extended Adv Enable, there > is a possibility of issue occurs when processing the Command Complete > event. Especially, the adv_info might not be found since we already > remove it in hci_req_clear_adv_instance() -> hci_remove_adv_instance(). > If that's the case, we will mistakenly proceed to remove all adv > instances instead of just one single instance. > > This patch fixes the issue by checking the content of the HCI command > instead of checking whether the adv_info is found. > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> > > --- > > net/bluetooth/hci_event.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 3cba2bbefcd6..894670419a27 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -1326,8 +1326,10 @@ static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, > &conn->le_conn_timeout, > conn->conn_timeout); > } else { > - if (adv) { > - adv->enabled = false; > + if (cp->num_of_sets) { > + if (adv) > + adv->enabled = false; > + > /* If just one instance was disabled check if there are > * any other instance enabled before clearing HCI_LE_ADV > */ > -- > 2.33.0.1079.g6e70778dc9-goog Applied, thanks.
Hi Luiz, >> If we remove one instance of adv using Set Extended Adv Enable, there >> is a possibility of issue occurs when processing the Command Complete >> event. Especially, the adv_info might not be found since we already >> remove it in hci_req_clear_adv_instance() -> hci_remove_adv_instance(). >> If that's the case, we will mistakenly proceed to remove all adv >> instances instead of just one single instance. >> >> This patch fixes the issue by checking the content of the HCI command >> instead of checking whether the adv_info is found. >> >> Signed-off-by: Archie Pusaka <apusaka@chromium.org> >> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> >> >> --- >> >> net/bluetooth/hci_event.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c >> index 3cba2bbefcd6..894670419a27 100644 >> --- a/net/bluetooth/hci_event.c >> +++ b/net/bluetooth/hci_event.c >> @@ -1326,8 +1326,10 @@ static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, >> &conn->le_conn_timeout, >> conn->conn_timeout); >> } else { >> - if (adv) { >> - adv->enabled = false; >> + if (cp->num_of_sets) { >> + if (adv) >> + adv->enabled = false; >> + >> /* If just one instance was disabled check if there are >> * any other instance enabled before clearing HCI_LE_ADV >> */ I haven’t applied this yet since I wanted to make sure it doesn’t interfere with our set of 23 patches. Do you need to rebase? Regards Marcel
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 3cba2bbefcd6..894670419a27 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1326,8 +1326,10 @@ static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, &conn->le_conn_timeout, conn->conn_timeout); } else { - if (adv) { - adv->enabled = false; + if (cp->num_of_sets) { + if (adv) + adv->enabled = false; + /* If just one instance was disabled check if there are * any other instance enabled before clearing HCI_LE_ADV */