| Message ID | 20211028191723.1.I94a358fc5abdb596412a2e22dd2b73b71f56fa82@changeid (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Bluetooth: Fix removing adv when processing cmd complete | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 12 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | No static functions without inline keyword in header files |
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 */