diff mbox series

[RFC,5/5] Bluetooth: hci_sync: Make use of hci_cmd_sync_queue set 3

Message ID 20210528000136.52352-5-luiz.dentz@gmail.com (mailing list archive)
State Superseded
Headers show
Series [RFC,1/5] Bluetooth: Add helper for serialized HCI command execution | expand

Commit Message

Luiz Augusto von Dentz May 28, 2021, 12:01 a.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This make use of hci_cmd_sync_queue for the following MGMT commands:

    Add Device
    Remove Device

Tested with:

mgmt-tester -s "Add Device"

Test Summary
------------
Add Device - Invalid Params 1                        Passed       0.017 seconds
Add Device - Invalid Params 2                        Passed       0.013 seconds
Add Device - Invalid Params 3                        Passed       0.013 seconds
Add Device - Invalid Params 4                        Passed       0.013 seconds
Add Device - Success 1                               Passed       0.014 seconds
Add Device - Success 2                               Passed       0.014 seconds
Add Device - Success 3                               Passed       0.014 seconds
Add Device - Success 4                               Passed       0.017 seconds
Add Device - Success 5                               Passed       0.017 seconds
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
Overall execution time: 0.14 seconds

mgmt-tester -s "Remove Device"

Test Summary
------------
Remove Device - Invalid Params 1                     Passed       0.153 seconds
Remove Device - Invalid Params 2                     Passed       0.014 seconds
Remove Device - Invalid Params 3                     Passed       0.013 seconds
Remove Device - Success 1                            Passed       0.016 seconds
Remove Device - Success 2                            Passed       0.017 seconds
Remove Device - Success 3                            Passed       1.022 seconds
Remove Device - Success 4                            Passed       1.021 seconds
Remove Device - Success 5                            Passed       1.022 seconds
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
Overall execution time: 3.29 seconds

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/hci_sync.c | 606 ++++++++++++++++++++++++++++++++++++++-
 net/bluetooth/hci_sync.h |   2 +
 net/bluetooth/mgmt.c     |  19 +-
 3 files changed, 622 insertions(+), 5 deletions(-)

Comments

Tedd Ho-Jeong An June 1, 2021, 8:24 p.m. UTC | #1
Hi Luiz,

On Thu, 2021-05-27 at 17:01 -0700, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This make use of hci_cmd_sync_queue for the following MGMT commands:
> 
>     Add Device
>     Remove Device
> 
> Tested with:
> 
> mgmt-tester -s "Add Device"
> 
> Test Summary
> ------------
> Add Device - Invalid Params 1                        Passed       0.017 seconds
> Add Device - Invalid Params 2                        Passed       0.013 seconds
> Add Device - Invalid Params 3                        Passed       0.013 seconds
> Add Device - Invalid Params 4                        Passed       0.013 seconds
> Add Device - Success 1                               Passed       0.014 seconds
> Add Device - Success 2                               Passed       0.014 seconds
> Add Device - Success 3                               Passed       0.014 seconds
> Add Device - Success 4                               Passed       0.017 seconds
> Add Device - Success 5                               Passed       0.017 seconds
> Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> Overall execution time: 0.14 seconds
> 
> mgmt-tester -s "Remove Device"
> 
> Test Summary
> ------------
> Remove Device - Invalid Params 1                     Passed       0.153 seconds
> Remove Device - Invalid Params 2                     Passed       0.014 seconds
> Remove Device - Invalid Params 3                     Passed       0.013 seconds
> Remove Device - Success 1                            Passed       0.016 seconds
> Remove Device - Success 2                            Passed       0.017 seconds
> Remove Device - Success 3                            Passed       1.022 seconds
> Remove Device - Success 4                            Passed       1.021 seconds
> Remove Device - Success 5                            Passed       1.022 seconds
> Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> Overall execution time: 3.29 seconds
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  net/bluetooth/hci_sync.c | 606 ++++++++++++++++++++++++++++++++++++++-
>  net/bluetooth/hci_sync.h |   2 +
>  net/bluetooth/mgmt.c     |  19 +-
>  3 files changed, 622 insertions(+), 5 deletions(-)
> 

While running new test cases for checking LL Privacy (submitted the series to mailing list),
some test cases caused the kernel oops:

general protection fault, probably for non-canonical address 0xdead000000000116: 0000 [#1] PTI
CPU: 0 PID: 113 Comm: kworker/u3:2 Not tainted 5.12.0-g01861ba6bbe9-dirty #11
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014
Workqueue: hci0 hci_cmd_sync_work
RIP: 0010:hci_passive_scan_sync.part.0+0xed/0x820
Code: 7c 24 13 00 75 12 48 8b 85 00 10 00 00 48 0f ba e0 29 0f 83 97 02 00 00 80 44 24 1e 01 4d 8b 3f 4c 39 3c 24 0f 84 25 01 00 00 <41> 0f b6 57 16 4d 8d 67 10 4c 89 ef 4c 89 e6 e8 2f 95 fb ff 41 0f
RSP: 0018:ffffad9400187dc8 EFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff8d0a01850ca8 RSI: ffff8d0a0186a210 RDI: ffff8d0a01850000
RBP: ffff8d0a01850000 R08: ffff8d0a01803ae6 R09: 0000000000004ffb
R10: 0000000078563412 R11: 3fffffffffffffff R12: ffff8d0a0186a210
R13: ffff8d0a01850cf8 R14: ffff8d0a01850d08 R15: dead000000000100
FS:  0000000000000000(0000) GS:ffffffff87846000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000558641956130 CR3: 00000000018a2000 CR4: 00000000000006f0
Call Trace:
 ? unblock_device+0xe0/0xe0
 hci_update_background_scan_sync+0x268/0x310
 hci_cmd_sync_work+0x91/0xe0
 process_one_work+0x19d/0x2f0
 worker_thread+0x5a/0x3b0
 ? rescuer_thread+0x330/0x330
 kthread+0x108/0x120
 ? __kthread_create_worker+0xf0/0xf0
 ret_from_fork+0x22/0x30
---[ end trace efd7eab9e13c521e ]---
RIP: 0010:hci_passive_scan_sync.part.0+0xed/0x820
Code: 7c 24 13 00 75 12 48 8b 85 00 10 00 00 48 0f ba e0 29 0f 83 97 02 00 00 80 44 24 1e 01 4d 8b 3f 4c 39 3c 24 0f 84 25 01 00 00 <41> 0f b6 57 16 4d 8d 67 10 4c 89 ef 4c 89 e6 e8 2f 95 fb ff 41 0f
RSP: 0018:ffffad9400187dc8 EFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff8d0a01850ca8 RSI: ffff8d0a0186a210 RDI: ffff8d0a01850000
RBP: ffff8d0a01850000 R08: ffff8d0a01803ae6 R09: 0000000000004ffb
R10: 0000000078563412 R11: 3fffffffffffffff R12: ffff8d0a0186a210
R13: ffff8d0a01850cf8 R14: ffff8d0a01850d08 R15: dead000000000100
FS:  0000000000000000(0000) GS:ffffffff87846000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000558641956130 CR3: 00000000018a2000 CR4: 00000000000006f0


However, it is not seen on the current bluetooth-next tree.

Regards,

Tedd
Luiz Augusto von Dentz June 1, 2021, 9:20 p.m. UTC | #2
Hi Tedd,

On Tue, Jun 1, 2021 at 1:24 PM Tedd Ho-Jeong An <tedd.an@linux.intel.com> wrote:
>
> Hi Luiz,
>
> On Thu, 2021-05-27 at 17:01 -0700, Luiz Augusto von Dentz wrote:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This make use of hci_cmd_sync_queue for the following MGMT commands:
> >
> >     Add Device
> >     Remove Device
> >
> > Tested with:
> >
> > mgmt-tester -s "Add Device"
> >
> > Test Summary
> > ------------
> > Add Device - Invalid Params 1                        Passed       0.017 seconds
> > Add Device - Invalid Params 2                        Passed       0.013 seconds
> > Add Device - Invalid Params 3                        Passed       0.013 seconds
> > Add Device - Invalid Params 4                        Passed       0.013 seconds
> > Add Device - Success 1                               Passed       0.014 seconds
> > Add Device - Success 2                               Passed       0.014 seconds
> > Add Device - Success 3                               Passed       0.014 seconds
> > Add Device - Success 4                               Passed       0.017 seconds
> > Add Device - Success 5                               Passed       0.017 seconds
> > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> > Overall execution time: 0.14 seconds
> >
> > mgmt-tester -s "Remove Device"
> >
> > Test Summary
> > ------------
> > Remove Device - Invalid Params 1                     Passed       0.153 seconds
> > Remove Device - Invalid Params 2                     Passed       0.014 seconds
> > Remove Device - Invalid Params 3                     Passed       0.013 seconds
> > Remove Device - Success 1                            Passed       0.016 seconds
> > Remove Device - Success 2                            Passed       0.017 seconds
> > Remove Device - Success 3                            Passed       1.022 seconds
> > Remove Device - Success 4                            Passed       1.021 seconds
> > Remove Device - Success 5                            Passed       1.022 seconds
> > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> > Overall execution time: 3.29 seconds
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> >  net/bluetooth/hci_sync.c | 606 ++++++++++++++++++++++++++++++++++++++-
> >  net/bluetooth/hci_sync.h |   2 +
> >  net/bluetooth/mgmt.c     |  19 +-
> >  3 files changed, 622 insertions(+), 5 deletions(-)
> >
>
> While running new test cases for checking LL Privacy (submitted the series to mailing list),
> some test cases caused the kernel oops:
>
> general protection fault, probably for non-canonical address 0xdead000000000116: 0000 [#1] PTI
> CPU: 0 PID: 113 Comm: kworker/u3:2 Not tainted 5.12.0-g01861ba6bbe9-dirty #11
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014
> Workqueue: hci0 hci_cmd_sync_work
> RIP: 0010:hci_passive_scan_sync.part.0+0xed/0x820
> Code: 7c 24 13 00 75 12 48 8b 85 00 10 00 00 48 0f ba e0 29 0f 83 97 02 00 00 80 44 24 1e 01 4d 8b 3f 4c 39 3c 24 0f 84 25 01 00 00 <41> 0f b6 57 16 4d 8d 67 10 4c 89 ef 4c 89 e6 e8 2f 95 fb ff 41 0f
> RSP: 0018:ffffad9400187dc8 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: ffff8d0a01850ca8 RSI: ffff8d0a0186a210 RDI: ffff8d0a01850000
> RBP: ffff8d0a01850000 R08: ffff8d0a01803ae6 R09: 0000000000004ffb
> R10: 0000000078563412 R11: 3fffffffffffffff R12: ffff8d0a0186a210
> R13: ffff8d0a01850cf8 R14: ffff8d0a01850d08 R15: dead000000000100
> FS:  0000000000000000(0000) GS:ffffffff87846000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000558641956130 CR3: 00000000018a2000 CR4: 00000000000006f0
> Call Trace:
>  ? unblock_device+0xe0/0xe0
>  hci_update_background_scan_sync+0x268/0x310
>  hci_cmd_sync_work+0x91/0xe0
>  process_one_work+0x19d/0x2f0
>  worker_thread+0x5a/0x3b0
>  ? rescuer_thread+0x330/0x330
>  kthread+0x108/0x120
>  ? __kthread_create_worker+0xf0/0xf0
>  ret_from_fork+0x22/0x30
> ---[ end trace efd7eab9e13c521e ]---
> RIP: 0010:hci_passive_scan_sync.part.0+0xed/0x820
> Code: 7c 24 13 00 75 12 48 8b 85 00 10 00 00 48 0f ba e0 29 0f 83 97 02 00 00 80 44 24 1e 01 4d 8b 3f 4c 39 3c 24 0f 84 25 01 00 00 <41> 0f b6 57 16 4d 8d 67 10 4c 89 ef 4c 89 e6 e8 2f 95 fb ff 41 0f
> RSP: 0018:ffffad9400187dc8 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: ffff8d0a01850ca8 RSI: ffff8d0a0186a210 RDI: ffff8d0a01850000
> RBP: ffff8d0a01850000 R08: ffff8d0a01803ae6 R09: 0000000000004ffb
> R10: 0000000078563412 R11: 3fffffffffffffff R12: ffff8d0a0186a210
> R13: ffff8d0a01850cf8 R14: ffff8d0a01850d08 R15: dead000000000100
> FS:  0000000000000000(0000) GS:ffffffff87846000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000558641956130 CR3: 00000000018a2000 CR4: 00000000000006f0

Remove Device - Success 7 - Remove from whitelist - setup complete
Remove Device - Success 7 - Remove from whitelist - run
  Registering Device Removed notification
  Test condition added, total 1
  Registering HCI command callback
  Test condition added, total 2
  Sending Remove Device (0x0034)
  Test condition added, total 3
  Remove Device (0x0034): Success (0x00)
  Test condition complete, 2 left
  New Device Removed event received
  Test condition complete, 1 left
  HCI Command 0x2042 length 6
[   58.818744] ==================================================================
[   58.819253] BUG: KASAN: use-after-free in
hci_passive_scan_sync.part.0+0x52e/0xaf0
[   58.819833] Read of size 6 at addr ffff8880019bca10 by task kworker/u3:0/49
[   58.820305]
[   58.820584]
[   58.820708] Allocated by task 92:
[   58.820979]
[   58.821092] Freed by task 93:
[   58.821345]
[   58.821489] The buggy address belongs to the object at ffff8880019bca00
[   58.821489]  which belongs to the cache kmalloc-32 of size 32
[   58.822488] The buggy address is located 16 bytes inside of
[   58.822488]  32-byte region [ffff8880019bca00, ffff8880019bca20)
[   58.823363] The buggy address belongs to the page:
[   58.823773]
[   58.823881] Memory state around the buggy address:
[   58.824215]  ffff8880019bc900: fb fb fb fb fc fc fc fc fa fb fb fb
fc fc fc fc
[   58.824771]  ffff8880019bc980: 00 00 00 00 fc fc fc fc fb fb fb fb
fc fc fc fc
[   58.825275] >ffff8880019bca00: fa fb fb fb fc fc fc fc fa fb fb fb
fc fc fc fc
[   58.825884]                          ^
[   58.826169]  ffff8880019bca80: fb fb fb fb fc fc fc fc fb fb fb fb
fc fc fc fc
[   58.826790]  ffff8880019bcb00: fb fb fb fb fc fc fc fc fb fb fb fb
fc fc fc fc
[   58.827330] ==================================================================
  HCI Command 0x2012 length 7
  Test condition complete, 0 left

Btw, it is a good idea to enable KSAN when testing, Im afraid this
might be related to hdev_lock.

>
> However, it is not seen on the current bluetooth-next tree.
>
> Regards,
>
> Tedd
>
Luiz Augusto von Dentz June 1, 2021, 9:56 p.m. UTC | #3
Hi Tedd,

On Tue, Jun 1, 2021 at 2:20 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Tedd,
>
> On Tue, Jun 1, 2021 at 1:24 PM Tedd Ho-Jeong An <tedd.an@linux.intel.com> wrote:
> >
> > Hi Luiz,
> >
> > On Thu, 2021-05-27 at 17:01 -0700, Luiz Augusto von Dentz wrote:
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This make use of hci_cmd_sync_queue for the following MGMT commands:
> > >
> > >     Add Device
> > >     Remove Device
> > >
> > > Tested with:
> > >
> > > mgmt-tester -s "Add Device"
> > >
> > > Test Summary
> > > ------------
> > > Add Device - Invalid Params 1                        Passed       0.017 seconds
> > > Add Device - Invalid Params 2                        Passed       0.013 seconds
> > > Add Device - Invalid Params 3                        Passed       0.013 seconds
> > > Add Device - Invalid Params 4                        Passed       0.013 seconds
> > > Add Device - Success 1                               Passed       0.014 seconds
> > > Add Device - Success 2                               Passed       0.014 seconds
> > > Add Device - Success 3                               Passed       0.014 seconds
> > > Add Device - Success 4                               Passed       0.017 seconds
> > > Add Device - Success 5                               Passed       0.017 seconds
> > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> > > Overall execution time: 0.14 seconds
> > >
> > > mgmt-tester -s "Remove Device"
> > >
> > > Test Summary
> > > ------------
> > > Remove Device - Invalid Params 1                     Passed       0.153 seconds
> > > Remove Device - Invalid Params 2                     Passed       0.014 seconds
> > > Remove Device - Invalid Params 3                     Passed       0.013 seconds
> > > Remove Device - Success 1                            Passed       0.016 seconds
> > > Remove Device - Success 2                            Passed       0.017 seconds
> > > Remove Device - Success 3                            Passed       1.022 seconds
> > > Remove Device - Success 4                            Passed       1.021 seconds
> > > Remove Device - Success 5                            Passed       1.022 seconds
> > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> > > Overall execution time: 3.29 seconds
> > >
> > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > ---
> > >  net/bluetooth/hci_sync.c | 606 ++++++++++++++++++++++++++++++++++++++-
> > >  net/bluetooth/hci_sync.h |   2 +
> > >  net/bluetooth/mgmt.c     |  19 +-
> > >  3 files changed, 622 insertions(+), 5 deletions(-)
> > >
> >
> > While running new test cases for checking LL Privacy (submitted the series to mailing list),
> > some test cases caused the kernel oops:
> >
> > general protection fault, probably for non-canonical address 0xdead000000000116: 0000 [#1] PTI
> > CPU: 0 PID: 113 Comm: kworker/u3:2 Not tainted 5.12.0-g01861ba6bbe9-dirty #11
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014
> > Workqueue: hci0 hci_cmd_sync_work
> > RIP: 0010:hci_passive_scan_sync.part.0+0xed/0x820
> > Code: 7c 24 13 00 75 12 48 8b 85 00 10 00 00 48 0f ba e0 29 0f 83 97 02 00 00 80 44 24 1e 01 4d 8b 3f 4c 39 3c 24 0f 84 25 01 00 00 <41> 0f b6 57 16 4d 8d 67 10 4c 89 ef 4c 89 e6 e8 2f 95 fb ff 41 0f
> > RSP: 0018:ffffad9400187dc8 EFLAGS: 00010202
> > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> > RDX: ffff8d0a01850ca8 RSI: ffff8d0a0186a210 RDI: ffff8d0a01850000
> > RBP: ffff8d0a01850000 R08: ffff8d0a01803ae6 R09: 0000000000004ffb
> > R10: 0000000078563412 R11: 3fffffffffffffff R12: ffff8d0a0186a210
> > R13: ffff8d0a01850cf8 R14: ffff8d0a01850d08 R15: dead000000000100
> > FS:  0000000000000000(0000) GS:ffffffff87846000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000558641956130 CR3: 00000000018a2000 CR4: 00000000000006f0
> > Call Trace:
> >  ? unblock_device+0xe0/0xe0
> >  hci_update_background_scan_sync+0x268/0x310
> >  hci_cmd_sync_work+0x91/0xe0
> >  process_one_work+0x19d/0x2f0
> >  worker_thread+0x5a/0x3b0
> >  ? rescuer_thread+0x330/0x330
> >  kthread+0x108/0x120
> >  ? __kthread_create_worker+0xf0/0xf0
> >  ret_from_fork+0x22/0x30
> > ---[ end trace efd7eab9e13c521e ]---
> > RIP: 0010:hci_passive_scan_sync.part.0+0xed/0x820
> > Code: 7c 24 13 00 75 12 48 8b 85 00 10 00 00 48 0f ba e0 29 0f 83 97 02 00 00 80 44 24 1e 01 4d 8b 3f 4c 39 3c 24 0f 84 25 01 00 00 <41> 0f b6 57 16 4d 8d 67 10 4c 89 ef 4c 89 e6 e8 2f 95 fb ff 41 0f
> > RSP: 0018:ffffad9400187dc8 EFLAGS: 00010202
> > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> > RDX: ffff8d0a01850ca8 RSI: ffff8d0a0186a210 RDI: ffff8d0a01850000
> > RBP: ffff8d0a01850000 R08: ffff8d0a01803ae6 R09: 0000000000004ffb
> > R10: 0000000078563412 R11: 3fffffffffffffff R12: ffff8d0a0186a210
> > R13: ffff8d0a01850cf8 R14: ffff8d0a01850d08 R15: dead000000000100
> > FS:  0000000000000000(0000) GS:ffffffff87846000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000558641956130 CR3: 00000000018a2000 CR4: 00000000000006f0
>
> Remove Device - Success 7 - Remove from whitelist - setup complete
> Remove Device - Success 7 - Remove from whitelist - run
>   Registering Device Removed notification
>   Test condition added, total 1
>   Registering HCI command callback
>   Test condition added, total 2
>   Sending Remove Device (0x0034)
>   Test condition added, total 3
>   Remove Device (0x0034): Success (0x00)
>   Test condition complete, 2 left
>   New Device Removed event received
>   Test condition complete, 1 left
>   HCI Command 0x2042 length 6
> [   58.818744] ==================================================================
> [   58.819253] BUG: KASAN: use-after-free in
> hci_passive_scan_sync.part.0+0x52e/0xaf0
> [   58.819833] Read of size 6 at addr ffff8880019bca10 by task kworker/u3:0/49
> [   58.820305]
> [   58.820584]
> [   58.820708] Allocated by task 92:
> [   58.820979]
> [   58.821092] Freed by task 93:
> [   58.821345]
> [   58.821489] The buggy address belongs to the object at ffff8880019bca00
> [   58.821489]  which belongs to the cache kmalloc-32 of size 32
> [   58.822488] The buggy address is located 16 bytes inside of
> [   58.822488]  32-byte region [ffff8880019bca00, ffff8880019bca20)
> [   58.823363] The buggy address belongs to the page:
> [   58.823773]
> [   58.823881] Memory state around the buggy address:
> [   58.824215]  ffff8880019bc900: fb fb fb fb fc fc fc fc fa fb fb fb
> fc fc fc fc
> [   58.824771]  ffff8880019bc980: 00 00 00 00 fc fc fc fc fb fb fb fb
> fc fc fc fc
> [   58.825275] >ffff8880019bca00: fa fb fb fb fc fc fc fc fa fb fb fb
> fc fc fc fc
> [   58.825884]                          ^
> [   58.826169]  ffff8880019bca80: fb fb fb fb fc fc fc fc fb fb fb fb
> fc fc fc fc
> [   58.826790]  ffff8880019bcb00: fb fb fb fb fc fc fc fc fb fb fb fb
> fc fc fc fc
> [   58.827330] ==================================================================
>   HCI Command 0x2012 length 7
>   Test condition complete, 0 left
>
> Btw, it is a good idea to enable KSAN when testing, Im afraid this
> might be related to hdev_lock.

It was actually the sync removing entries inline, so the following
fixed the problem for me:

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 69ce640be465..9b31823ad1f0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1085,7 +1085,7 @@ static int hci_le_del_allow_list_sync(struct
hci_dev *hdev,
        bt_dev_dbg(hdev, "Remove %pMR (0x%x) from allow list", &cp.bdaddr,
                   cp.bdaddr_type);

-       return hci_le_del_resolve_list_sync(hdev, bdaddr, bdaddr_type);
+       return hci_le_del_resolve_list_sync(hdev, &cp.bdaddr, cp.bdaddr_type);
 }

 /* Adds connection to resolve list if needed.*/
@@ -1165,7 +1165,7 @@ static int hci_le_add_allow_list_sync(struct
hci_dev *hdev,
 static u8 hci_update_white_list_sync(struct hci_dev *hdev)
 {
        struct hci_conn_params *params;
-       struct bdaddr_list *b;
+       struct bdaddr_list *b, *t;
        u8 num_entries = 0;
        bool pend_conn, pend_report;
        /* We allow whitelisting even with RPAs in suspend. In the worst case,
@@ -1182,10 +1182,10 @@ static u8 hci_update_white_list_sync(struct
hci_dev *hdev)
        /* Go through the current white list programmed into the
         * controller one by one and check if that address is still
         * in the list of pending connections or list of devices to
-        * report. If not present in either list, then queue the
-        * command to remove it from the controller.
+        * report. If not present in either list, then remove it from
+        * the controller.
         */
-       list_for_each_entry(b, &hdev->le_white_list, list) {
+       list_for_each_entry_safe(b, t, &hdev->le_white_list, list) {
                pend_conn = hci_pend_le_action_lookup(&hdev->pend_le_conns,
                                                      &b->bdaddr,
                                                      b->bdaddr_type);

There might be other instances where we need to use
list_for_each_entry_safe since the command may generate an event that
frees the entries in place.

>
> >
> > However, it is not seen on the current bluetooth-next tree.
> >
> > Regards,
> >
> > Tedd
> >
>
>
> --
> Luiz Augusto von Dentz
diff mbox series

Patch

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 671f71413847..69ce640be465 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -85,7 +85,10 @@  u8 __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
 	uint8_t status;
 
 	skb = __hci_cmd_sync(hdev, opcode, plen, param, timeout);
-	if (IS_ERR(skb)) {
+	if (IS_ERR_OR_NULL(skb)) {
+		bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode,
+			   PTR_ERR(skb));
+
 		switch (PTR_ERR(skb)) {
 		case -ETIMEDOUT:
 			return HCI_ERROR_CONNECTION_TIMEOUT;
@@ -912,3 +915,604 @@  int hci_disable_advertising_sync(struct hci_dev *hdev)
 	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_ENABLE,
 				     sizeof(enable), &enable, HCI_CMD_TIMEOUT);
 }
+
+static int hci_le_set_ext_scan_enable_sync(struct hci_dev *hdev, u8 val,
+					   u8 filter_dup)
+{
+	struct hci_cp_le_set_ext_scan_enable cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.enable = val;
+	cp.filter_dup = filter_dup;
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_SCAN_ENABLE,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
+static int hci_le_set_scan_enable_sync(struct hci_dev *hdev, u8 val,
+				       u8 filter_dup)
+{
+	struct hci_cp_le_set_scan_enable cp;
+
+	if (use_ext_scan(hdev))
+		return hci_le_set_ext_scan_enable_sync(hdev, val, filter_dup);
+
+	memset(&cp, 0, sizeof(cp));
+	cp.enable = val;
+	cp.filter_dup = filter_dup;
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_SCAN_ENABLE,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
+static int hci_le_set_addr_resolution_enable_sync(struct hci_dev *hdev, u8 val)
+{
+	if (!use_ll_privacy(hdev) ||
+	    !hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY))
+		return 0;
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE,
+				     sizeof(val), &val, HCI_CMD_TIMEOUT);
+}
+
+int hci_scan_disable_sync(struct hci_dev *hdev, bool rpa_le_conn)
+{
+	int err;
+
+	/* If controller is not scanning we are done. */
+	if (!hci_dev_test_flag(hdev, HCI_LE_SCAN))
+		return 0;
+
+	if (hdev->scanning_paused) {
+		bt_dev_dbg(hdev, "Scanning is paused for suspend");
+		return 0;
+	}
+
+	if (hdev->suspended)
+		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
+
+	err = hci_le_set_scan_enable_sync(hdev, LE_SCAN_DISABLE, 0x00);
+	if (err) {
+		bt_dev_err(hdev, "Unable to disable scanning: %d", err);
+		return err;
+	}
+
+	if (rpa_le_conn) {
+		err = hci_le_set_addr_resolution_enable_sync(hdev, 0x00);
+		if (err)
+			bt_dev_err(hdev, "Unable to disable LL privacy: %d",
+				   err);
+	}
+
+	return err;
+}
+
+static bool scan_use_rpa(struct hci_dev *hdev)
+{
+	return hci_dev_test_flag(hdev, HCI_PRIVACY);
+}
+
+static void hci_start_interleave_scan(struct hci_dev *hdev)
+{
+	hdev->interleave_scan_state = INTERLEAVE_SCAN_NO_FILTER;
+	queue_delayed_work(hdev->req_workqueue,
+			   &hdev->interleave_scan, 0);
+}
+
+static bool is_interleave_scanning(struct hci_dev *hdev)
+{
+	return hdev->interleave_scan_state != INTERLEAVE_SCAN_NONE;
+}
+
+static void cancel_interleave_scan(struct hci_dev *hdev)
+{
+	bt_dev_dbg(hdev, "cancelling interleave scan");
+
+	cancel_delayed_work_sync(&hdev->interleave_scan);
+
+	hdev->interleave_scan_state = INTERLEAVE_SCAN_NONE;
+}
+
+/* Return true if interleave_scan wasn't started until exiting this function,
+ * otherwise, return false
+ */
+static bool hci_update_interleaved_scan_sync(struct hci_dev *hdev)
+{
+	/* Do interleaved scan only if all of the following are true:
+	 * - There is at least one ADV monitor
+	 * - At least one pending LE connection or one device to be scanned for
+	 * - Monitor offloading is not supported
+	 * If so, we should alternate between allowlist scan and one without
+	 * any filters to save power.
+	 */
+	bool use_interleaving = hci_is_adv_monitoring(hdev) &&
+				!(list_empty(&hdev->pend_le_conns) &&
+				  list_empty(&hdev->pend_le_reports)) &&
+				hci_get_adv_monitor_offload_ext(hdev) ==
+				    HCI_ADV_MONITOR_EXT_NONE;
+	bool is_interleaving = is_interleave_scanning(hdev);
+
+	if (use_interleaving && !is_interleaving) {
+		hci_start_interleave_scan(hdev);
+		bt_dev_dbg(hdev, "starting interleave scan");
+		return true;
+	}
+
+	if (!use_interleaving && is_interleaving)
+		cancel_interleave_scan(hdev);
+
+	return false;
+}
+
+/* Removes connection to resolve list if needed.*/
+static int hci_le_del_resolve_list_sync(struct hci_dev *hdev,
+					bdaddr_t *bdaddr, u8 bdaddr_type)
+{
+	struct hci_cp_le_del_from_resolv_list cp;
+	struct smp_irk *irk;
+
+	if (!use_ll_privacy(hdev) ||
+	    !hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY))
+		return 0;
+
+	irk = hci_find_irk_by_addr(hdev, bdaddr, bdaddr_type);
+	if (!irk)
+		return 0;
+
+	cp.bdaddr_type = bdaddr_type;
+	bacpy(&cp.bdaddr, bdaddr);
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
+static int hci_le_del_allow_list_sync(struct hci_dev *hdev,
+				      bdaddr_t *bdaddr, u8 bdaddr_type)
+{
+	struct hci_cp_le_del_from_white_list cp;
+	int err;
+
+	cp.bdaddr_type = bdaddr_type;
+	bacpy(&cp.bdaddr, bdaddr);
+
+	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_DEL_FROM_WHITE_LIST,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+	if (err) {
+		bt_dev_err(hdev, "Unable to remove from allow list: %d", err);
+		return err;
+	}
+
+	bt_dev_dbg(hdev, "Remove %pMR (0x%x) from allow list", &cp.bdaddr,
+		   cp.bdaddr_type);
+
+	return hci_le_del_resolve_list_sync(hdev, bdaddr, bdaddr_type);
+}
+
+/* Adds connection to resolve list if needed.*/
+static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
+					struct hci_conn_params *params)
+{
+	struct hci_cp_le_add_to_resolv_list cp;
+	struct smp_irk *irk;
+
+	if (!use_ll_privacy(hdev) ||
+	    !hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY))
+		return 0;
+
+	irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
+	if (!irk)
+		return 0;
+
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+	memcpy(cp.peer_irk, irk->val, 16);
+
+	if (hci_dev_test_flag(hdev, HCI_PRIVACY))
+		memcpy(cp.local_irk, hdev->irk, 16);
+	else
+		memset(cp.local_irk, 0, 16);
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_ADD_TO_RESOLV_LIST,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
+/* Adds connection to allow list if needed.*/
+static int hci_le_add_allow_list_sync(struct hci_dev *hdev,
+				      struct hci_conn_params *params,
+				      u8 *num_entries, bool allow_rpa)
+{
+	struct hci_cp_le_add_to_white_list cp;
+	int err;
+
+	/* Already in white list */
+	if (hci_bdaddr_list_lookup(&hdev->le_white_list, &params->addr,
+				   params->addr_type))
+		return 0;
+
+	/* Select filter policy to accept all advertising */
+	if (*num_entries >= hdev->le_white_list_size)
+		return -ENOSPC;
+
+	/* White list can not be used with RPAs */
+	if (!allow_rpa &&
+	    !hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY) &&
+	    hci_find_irk_by_addr(hdev, &params->addr, params->addr_type)) {
+		return -EINVAL;
+	}
+
+	/* During suspend, only wakeable devices can be in whitelist */
+	if (hdev->suspended && !hci_conn_test_flag(HCI_CONN_FLAG_REMOTE_WAKEUP,
+						   params->current_flags))
+		return 0;
+
+	*num_entries += 1;
+	cp.bdaddr_type = params->addr_type;
+	bacpy(&cp.bdaddr, &params->addr);
+
+	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_ADD_TO_WHITE_LIST,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+	if (err) {
+		bt_dev_err(hdev, "Unable to add to allow list: %d", err);
+		return err;
+	}
+
+	bt_dev_dbg(hdev, "Add %pMR (0x%x) to allow list", &cp.bdaddr,
+		   cp.bdaddr_type);
+
+	return hci_le_add_resolve_list_sync(hdev, params);
+}
+
+static u8 hci_update_white_list_sync(struct hci_dev *hdev)
+{
+	struct hci_conn_params *params;
+	struct bdaddr_list *b;
+	u8 num_entries = 0;
+	bool pend_conn, pend_report;
+	/* We allow whitelisting even with RPAs in suspend. In the worst case,
+	 * we won't be able to wake from devices that use the privacy1.2
+	 * features. Additionally, once we support privacy1.2 and IRK
+	 * offloading, we can update this to also check for those conditions.
+	 */
+	bool allow_rpa = hdev->suspended;
+
+	if (use_ll_privacy(hdev) &&
+	    hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY))
+		allow_rpa = true;
+
+	/* Go through the current white list programmed into the
+	 * controller one by one and check if that address is still
+	 * in the list of pending connections or list of devices to
+	 * report. If not present in either list, then queue the
+	 * command to remove it from the controller.
+	 */
+	list_for_each_entry(b, &hdev->le_white_list, list) {
+		pend_conn = hci_pend_le_action_lookup(&hdev->pend_le_conns,
+						      &b->bdaddr,
+						      b->bdaddr_type);
+		pend_report = hci_pend_le_action_lookup(&hdev->pend_le_reports,
+							&b->bdaddr,
+							b->bdaddr_type);
+
+		/* If the device is not likely to connect or report,
+		 * remove it from the whitelist.
+		 */
+		if (!pend_conn && !pend_report) {
+			hci_le_del_allow_list_sync(hdev, &b->bdaddr,
+						   b->bdaddr_type);
+			continue;
+		}
+
+		/* White list can not be used with RPAs */
+		if (!allow_rpa &&
+		    !hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY) &&
+		    hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
+			return 0x00;
+		}
+
+		num_entries++;
+	}
+
+	/* Since all no longer valid white list entries have been
+	 * removed, walk through the list of pending connections
+	 * and ensure that any new device gets programmed into
+	 * the controller.
+	 *
+	 * If the list of the devices is larger than the list of
+	 * available white list entries in the controller, then
+	 * just abort and return filer policy value to not use the
+	 * white list.
+	 */
+	list_for_each_entry(params, &hdev->pend_le_conns, action) {
+		if (hci_le_add_allow_list_sync(hdev, params, &num_entries,
+					       allow_rpa))
+			return 0x00;
+	}
+
+	/* After adding all new pending connections, walk through
+	 * the list of pending reports and also add these to the
+	 * white list if there is still space. Abort if space runs out.
+	 */
+	list_for_each_entry(params, &hdev->pend_le_reports, action) {
+		if (hci_le_add_allow_list_sync(hdev, params, &num_entries,
+					       allow_rpa))
+			return 0x00;
+	}
+
+	/* Use the allowlist unless the following conditions are all true:
+	 * - We are not currently suspending
+	 * - There are 1 or more ADV monitors registered and it's not offloaded
+	 * - Interleaved scanning is not currently using the allowlist
+	 */
+	if (!idr_is_empty(&hdev->adv_monitors_idr) && !hdev->suspended &&
+	    hci_get_adv_monitor_offload_ext(hdev) == HCI_ADV_MONITOR_EXT_NONE &&
+	    hdev->interleave_scan_state != INTERLEAVE_SCAN_ALLOWLIST)
+		return 0x00;
+
+	/* Select filter policy to use white list */
+	return 0x01;
+}
+
+/* Returns true if an le connection is in the scanning state */
+static inline bool hci_is_le_conn_scanning(struct hci_dev *hdev)
+{
+	struct hci_conn_hash *h = &hdev->conn_hash;
+	struct hci_conn  *c;
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(c, &h->list, list) {
+		if (c->type == LE_LINK && c->state == BT_CONNECT &&
+		    test_bit(HCI_CONN_SCANNING, &c->flags)) {
+			rcu_read_unlock();
+			return true;
+		}
+	}
+
+	rcu_read_unlock();
+
+	return false;
+}
+
+static int hci_le_set_ext_scan_param_sync(struct hci_dev *hdev, u8 type,
+					  u16 interval, u16 window,
+					  u8 own_addr_type, u8 filter_policy)
+{
+	struct hci_cp_le_set_ext_scan_params *cp;
+	struct hci_cp_le_scan_phy_params *phy;
+	u8 data[sizeof(*cp) + sizeof(*phy) * 2];
+	u8 num_phy = 0;
+
+	cp = (void *)data;
+	phy = (void *)cp->data;
+
+	memset(data, 0, sizeof(data));
+
+	cp->own_addr_type = own_addr_type;
+	cp->filter_policy = filter_policy;
+
+	if (scan_1m(hdev) || scan_2m(hdev)) {
+		cp->scanning_phys |= LE_SCAN_PHY_1M;
+
+		phy->type = type;
+		phy->interval = cpu_to_le16(interval);
+		phy->window = cpu_to_le16(window);
+
+		num_phy++;
+		phy++;
+	}
+
+	if (scan_coded(hdev)) {
+		cp->scanning_phys |= LE_SCAN_PHY_CODED;
+
+		phy->type = type;
+		phy->interval = cpu_to_le16(interval);
+		phy->window = cpu_to_le16(window);
+
+		num_phy++;
+		phy++;
+	}
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_SCAN_PARAMS,
+				     sizeof(*cp) + sizeof(*phy) * num_phy,
+				     data, HCI_CMD_TIMEOUT);
+}
+
+static int hci_le_set_scan_param_sync(struct hci_dev *hdev, u8 type,
+				       u16 interval, u16 window,
+				       u8 own_addr_type, u8 filter_policy)
+{
+	struct hci_cp_le_set_scan_param cp;
+
+	if (use_ext_scan(hdev))
+		return hci_le_set_ext_scan_param_sync(hdev, type, interval,
+						      window, own_addr_type,
+						      filter_policy);
+
+	memset(&cp, 0, sizeof(cp));
+	cp.type = type;
+	cp.interval = cpu_to_le16(interval);
+	cp.window = cpu_to_le16(window);
+	cp.own_address_type = own_addr_type;
+	cp.filter_policy = filter_policy;
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_SCAN_PARAM,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
+static int hci_start_scan_sync(struct hci_dev *hdev, u8 type, u16 interval,
+			       u16 window, u8 own_addr_type, u8 filter_policy,
+			       bool addr_resolv)
+{
+	int err;
+
+	if (hdev->scanning_paused) {
+		bt_dev_dbg(hdev, "Scanning is paused for suspend");
+		return 0;
+	}
+
+	if (addr_resolv) {
+		err = hci_le_set_addr_resolution_enable_sync(hdev, 0x01);
+		if (err)
+			return err;
+	}
+
+	err = hci_le_set_scan_param_sync(hdev, type, interval, window,
+					 own_addr_type, filter_policy);
+	if (err)
+		return err;
+
+	return hci_le_set_scan_enable_sync(hdev, LE_SCAN_ENABLE,
+					   LE_SCAN_FILTER_DUP_ENABLE);
+}
+
+/* Ensure to call hci_scan_disable_sync first to disable the controller based
+ * address resolution to be able to reconfigure resolving list.
+ */
+int hci_passive_scan_sync(struct hci_dev *hdev)
+{
+	u8 own_addr_type;
+	u8 filter_policy;
+	u16 window, interval;
+	/* Background scanning should run with address resolution */
+	bool addr_resolv = true;
+
+	if (hdev->scanning_paused) {
+		bt_dev_dbg(hdev, "Scanning is paused for suspend");
+		return 0;
+	}
+
+	/* Set require_privacy to false since no SCAN_REQ are send
+	 * during passive scanning. Not using an non-resolvable address
+	 * here is important so that peer devices using direct
+	 * advertising with our address will be correctly reported
+	 * by the controller.
+	 */
+	if (hci_update_random_address_sync(hdev, false, scan_use_rpa(hdev),
+					   &own_addr_type))
+		return 0;
+
+	if (hdev->enable_advmon_interleave_scan &&
+	    hci_update_interleaved_scan_sync(hdev))
+		return 0;
+
+	bt_dev_dbg(hdev, "interleave state %d", hdev->interleave_scan_state);
+	/* Adding or removing entries from the white list must
+	 * happen before enabling scanning. The controller does
+	 * not allow white list modification while scanning.
+	 */
+	filter_policy = hci_update_white_list_sync(hdev);
+
+	/* When the controller is using random resolvable addresses and
+	 * with that having LE privacy enabled, then controllers with
+	 * Extended Scanner Filter Policies support can now enable support
+	 * for handling directed advertising.
+	 *
+	 * So instead of using filter polices 0x00 (no whitelist)
+	 * and 0x01 (whitelist enabled) use the new filter policies
+	 * 0x02 (no whitelist) and 0x03 (whitelist enabled).
+	 */
+	if (hci_dev_test_flag(hdev, HCI_PRIVACY) &&
+	    (hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
+		filter_policy |= 0x02;
+
+	if (hdev->suspended) {
+		window = hdev->le_scan_window_suspend;
+		interval = hdev->le_scan_int_suspend;
+
+		set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);
+	} else if (hci_is_le_conn_scanning(hdev)) {
+		window = hdev->le_scan_window_connect;
+		interval = hdev->le_scan_int_connect;
+	} else if (hci_is_adv_monitoring(hdev)) {
+		window = hdev->le_scan_window_adv_monitor;
+		interval = hdev->le_scan_int_adv_monitor;
+	} else {
+		window = hdev->le_scan_window;
+		interval = hdev->le_scan_interval;
+	}
+
+	bt_dev_dbg(hdev, "LE passive scan with whitelist = %d", filter_policy);
+
+	return hci_start_scan_sync(hdev, LE_SCAN_PASSIVE, interval, window,
+				   own_addr_type, filter_policy, addr_resolv);
+}
+
+/* This function controls the background scanning based on hdev->pend_le_conns
+ * list. If there are pending LE connection we start the background scanning,
+ * otherwise we stop it.
+ */
+int hci_update_background_scan_sync(struct hci_dev *hdev)
+{
+	int err;
+
+	if (!test_bit(HCI_UP, &hdev->flags) ||
+	    test_bit(HCI_INIT, &hdev->flags) ||
+	    hci_dev_test_flag(hdev, HCI_SETUP) ||
+	    hci_dev_test_flag(hdev, HCI_CONFIG) ||
+	    hci_dev_test_flag(hdev, HCI_AUTO_OFF) ||
+	    hci_dev_test_flag(hdev, HCI_UNREGISTER))
+		return 0;
+
+	/* No point in doing scanning if LE support hasn't been enabled */
+	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
+		return 0;
+
+	/* If discovery is active don't interfere with it */
+	if (hdev->discovery.state != DISCOVERY_STOPPED)
+		return 0;
+
+	/* Reset RSSI and UUID filters when starting background scanning
+	 * since these filters are meant for service discovery only.
+	 *
+	 * The Start Discovery and Start Service Discovery operations
+	 * ensure to set proper values for RSSI threshold and UUID
+	 * filter list. So it is safe to just reset them here.
+	 */
+	hci_discovery_filter_clear(hdev);
+
+	bt_dev_dbg(hdev, "ADV monitoring is %s",
+		   hci_is_adv_monitoring(hdev) ? "on" : "off");
+
+	if (list_empty(&hdev->pend_le_conns) &&
+	    list_empty(&hdev->pend_le_reports) &&
+	    !hci_is_adv_monitoring(hdev)) {
+		/* If there is no pending LE connections or devices
+		 * to be scanned for or no ADV monitors, we should stop the
+		 * background scanning.
+		 */
+
+		bt_dev_dbg(hdev, "stopping background scanning");
+
+		err = hci_scan_disable_sync(hdev, false);
+		if (err)
+			bt_dev_err(hdev, "stop background scanning failed: %d",
+				   err);
+	} else {
+		/* If there is at least one pending LE connection, we should
+		 * keep the background scan running.
+		 */
+
+		/* If controller is connecting, we should not start scanning
+		 * since some controllers are not able to scan and connect at
+		 * the same time.
+		 */
+		if (hci_lookup_le_connect(hdev))
+			return 0;
+
+		err = hci_scan_disable_sync(hdev, false);
+		if (err) {
+			bt_dev_err(hdev, "stop background scanning failed: %d",
+				   err);
+			return err;
+		}
+
+		bt_dev_dbg(hdev, "start background scanning");
+
+		err = hci_passive_scan_sync(hdev);
+		if (err)
+			bt_dev_err(hdev, "start background scanning failed: %d",
+				   err);
+	}
+
+	return err;
+}
diff --git a/net/bluetooth/hci_sync.h b/net/bluetooth/hci_sync.h
index 5e4392a93607..5ef02b1cb8c1 100644
--- a/net/bluetooth/hci_sync.h
+++ b/net/bluetooth/hci_sync.h
@@ -29,3 +29,5 @@  int hci_remove_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance);
 void hci_clear_adv_instance_sync(struct hci_dev *hdev, struct sock *sk,
 				 u8 instance, bool force);
 int hci_disable_advertising_sync(struct hci_dev *hdev);
+
+int hci_update_background_scan_sync(struct hci_dev *hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 55f5bd96f2a9..26ff1375d69b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6698,6 +6698,11 @@  static void device_added(struct sock *sk, struct hci_dev *hdev,
 	mgmt_event(MGMT_EV_DEVICE_ADDED, hdev, &ev, sizeof(ev), sk);
 }
 
+static int add_device_sync(struct hci_dev *hdev, void *data)
+{
+	return hci_update_background_scan_sync(hdev);
+}
+
 static int add_device(struct sock *sk, struct hci_dev *hdev,
 		      void *data, u16 len)
 {
@@ -6780,7 +6785,9 @@  static int add_device(struct sock *sk, struct hci_dev *hdev,
 			current_flags = params->current_flags;
 	}
 
-	hci_update_background_scan(hdev);
+	err = hci_cmd_sync_queue(hdev, add_device_sync, NULL, NULL);
+	if (err < 0)
+		goto unlock;
 
 added:
 	device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
@@ -6807,6 +6814,11 @@  static void device_removed(struct sock *sk, struct hci_dev *hdev,
 	mgmt_event(MGMT_EV_DEVICE_REMOVED, hdev, &ev, sizeof(ev), sk);
 }
 
+static int remove_device_sync(struct hci_dev *hdev, void *data)
+{
+	return hci_update_background_scan_sync(hdev);
+}
+
 static int remove_device(struct sock *sk, struct hci_dev *hdev,
 			 void *data, u16 len)
 {
@@ -6886,7 +6898,6 @@  static int remove_device(struct sock *sk, struct hci_dev *hdev,
 		list_del(&params->action);
 		list_del(&params->list);
 		kfree(params);
-		hci_update_background_scan(hdev);
 
 		device_removed(sk, hdev, &cp->addr.bdaddr, cp->addr.type);
 	} else {
@@ -6923,10 +6934,10 @@  static int remove_device(struct sock *sk, struct hci_dev *hdev,
 		}
 
 		bt_dev_dbg(hdev, "All LE connection parameters were removed");
-
-		hci_update_background_scan(hdev);
 	}
 
+	hci_cmd_sync_queue(hdev, remove_device_sync, NULL, NULL);
+
 complete:
 	err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_DEVICE,
 				MGMT_STATUS_SUCCESS, &cp->addr,