[PATCH] Bluetooth: Check inquiry status before sending one
From: Archie Pusaka
There is a possibility where HCI_INQUIRY flag is set but we still
send HCI_OP_INQUIRY anyway.
Such a case can be reproduced by connecting to an LE device while
active scanning. When the device is discovered, we initiate a
connection, stop LE Scan, and send Discovery MGMT with status
disabled, but we don't cancel the inquiry.
Signed-off-by: Archie Pusaka
Reviewed-by: Sonny Sasaka
---
net/bluetooth/hci_request.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 8ace5d34b01e..5a5ec7ed15ea 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -2952,6 +2952,9 @@ static int bredr_inquiry(struct hci_request *req,
unsigned long opt)
const u8 liac[3] = { 0x00, 0x8b, 0x9e };
struct hci_cp_inquiry cp;
+ if (test_bit(HCI_INQUIRY, >hdev->flags))
+ return 0;
+
bt_dev_dbg(req->hdev, "");
hci_dev_lock(req->hdev);
--
2.31.0.291.g576ba9dcdaf-goog
Re: [PATCH] Bluetooth: check for zapped sk before connecting
Hi Marcel,
Thanks for your suggestion. I implemented it in v2, please take another look.
On Mon, 22 Mar 2021 at 23:53, Marcel Holtmann wrote:
>
> Hi Archie,
>
> > There is a possibility of receiving a zapped sock on
> > l2cap_sock_connect(). This could lead to interesting crashes, one
> > such case is tearing down an already tore l2cap_sock as is happened
> > with this call trace:
> >
> > __dump_stack lib/dump_stack.c:15 [inline]
> > dump_stack+0xc4/0x118 lib/dump_stack.c:56
> > register_lock_class kernel/locking/lockdep.c:792 [inline]
> > register_lock_class+0x239/0x6f6 kernel/locking/lockdep.c:742
> > __lock_acquire+0x209/0x1e27 kernel/locking/lockdep.c:3105
> > lock_acquire+0x29c/0x2fb kernel/locking/lockdep.c:3599
> > __raw_spin_lock_bh include/linux/spinlock_api_smp.h:137 [inline]
> > _raw_spin_lock_bh+0x38/0x47 kernel/locking/spinlock.c:175
> > spin_lock_bh include/linux/spinlock.h:307 [inline]
> > lock_sock_nested+0x44/0xfa net/core/sock.c:2518
> > l2cap_sock_teardown_cb+0x88/0x2fb net/bluetooth/l2cap_sock.c:1345
> > l2cap_chan_del+0xa3/0x383 net/bluetooth/l2cap_core.c:598
> > l2cap_chan_close+0x537/0x5dd net/bluetooth/l2cap_core.c:756
> > l2cap_chan_timeout+0x104/0x17e net/bluetooth/l2cap_core.c:429
> > process_one_work+0x7e3/0xcb0 kernel/workqueue.c:2064
> > worker_thread+0x5a5/0x773 kernel/workqueue.c:2196
> > kthread+0x291/0x2a6 kernel/kthread.c:211
> > ret_from_fork+0x4e/0x80 arch/x86/entry/entry_64.S:604
> >
> > Signed-off-by: Archie Pusaka
> > Reported-by: syzbot+abfc0f5e668d4099a...@syzkaller.appspotmail.com
> > Reviewed-by: Alain Michaud
> > Reviewed-by: Abhishek Pandit-Subedi
> > Reviewed-by: Guenter Roeck
> > ---
> >
> > net/bluetooth/l2cap_sock.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index f1b1edd0b697..b86fd8cc4dc1 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -182,6 +182,13 @@ static int l2cap_sock_connect(struct socket *sock,
> > struct sockaddr *addr,
> >
> > BT_DBG("sk %p", sk);
> >
> > + lock_sock(sk);
> > + if (sock_flag(sk, SOCK_ZAPPED)) {
> > + release_sock(sk);
> > + return -EINVAL;
> > + }
> > + release_sock(sk);
> > +
>
> hmmm. I wonder if this would look better and easy to see that the locking is
> done correctly.
>
> lock_sock(sk);
> zapped = sock_flag(sk, SOCK_ZAPPED);
> release_sock(sk);
>
> if (zapped)
> return -EINVAL;
>
> Regards
>
> Marcel
>
[PATCH v2] Bluetooth: check for zapped sk before connecting
From: Archie Pusaka
There is a possibility of receiving a zapped sock on
l2cap_sock_connect(). This could lead to interesting crashes, one
such case is tearing down an already tore l2cap_sock as is happened
with this call trace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0xc4/0x118 lib/dump_stack.c:56
register_lock_class kernel/locking/lockdep.c:792 [inline]
register_lock_class+0x239/0x6f6 kernel/locking/lockdep.c:742
__lock_acquire+0x209/0x1e27 kernel/locking/lockdep.c:3105
lock_acquire+0x29c/0x2fb kernel/locking/lockdep.c:3599
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:137 [inline]
_raw_spin_lock_bh+0x38/0x47 kernel/locking/spinlock.c:175
spin_lock_bh include/linux/spinlock.h:307 [inline]
lock_sock_nested+0x44/0xfa net/core/sock.c:2518
l2cap_sock_teardown_cb+0x88/0x2fb net/bluetooth/l2cap_sock.c:1345
l2cap_chan_del+0xa3/0x383 net/bluetooth/l2cap_core.c:598
l2cap_chan_close+0x537/0x5dd net/bluetooth/l2cap_core.c:756
l2cap_chan_timeout+0x104/0x17e net/bluetooth/l2cap_core.c:429
process_one_work+0x7e3/0xcb0 kernel/workqueue.c:2064
worker_thread+0x5a5/0x773 kernel/workqueue.c:2196
kthread+0x291/0x2a6 kernel/kthread.c:211
ret_from_fork+0x4e/0x80 arch/x86/entry/entry_64.S:604
Signed-off-by: Archie Pusaka
Reported-by: syzbot+abfc0f5e668d4099a...@syzkaller.appspotmail.com
Reviewed-by: Alain Michaud
Reviewed-by: Abhishek Pandit-Subedi
Reviewed-by: Guenter Roeck
---
Changes in v2:
* Modify locking order for better visibility
net/bluetooth/l2cap_sock.c | 8
1 file changed, 8 insertions(+)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index f1b1edd0b697..c99d65ef13b1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -179,9 +179,17 @@ static int l2cap_sock_connect(struct socket *sock, struct
sockaddr *addr,
struct l2cap_chan *chan = l2cap_pi(sk)->chan;
struct sockaddr_l2 la;
int len, err = 0;
+ bool zapped;
BT_DBG("sk %p", sk);
+ lock_sock(sk);
+ zapped = sock_flag(sk, SOCK_ZAPPED);
+ release_sock(sk);
+
+ if (zapped)
+ return -EINVAL;
+
if (!addr || alen < offsetofend(struct sockaddr, sa_family) ||
addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
--
2.31.0.291.g576ba9dcdaf-goog
[PATCH] Bluetooth: verify AMP hci_chan before amp_destroy
From: Archie Pusaka
hci_chan can be created in 2 places: hci_loglink_complete_evt() if
it is an AMP hci_chan, or l2cap_conn_add() otherwise. In theory,
Only AMP hci_chan should be removed by a call to
hci_disconn_loglink_complete_evt(). However, the controller might mess
up, call that function, and destroy an hci_chan which is not initiated
by hci_loglink_complete_evt().
This patch adds a verification that the destroyed hci_chan must have
been init'd by hci_loglink_complete_evt().
Example crash call trace:
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xe3/0x144 lib/dump_stack.c:118
print_address_description+0x67/0x22a mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report mm/kasan/report.c:412 [inline]
kasan_report+0x251/0x28f mm/kasan/report.c:396
hci_send_acl+0x3b/0x56e net/bluetooth/hci_core.c:4072
l2cap_send_cmd+0x5af/0x5c2 net/bluetooth/l2cap_core.c:877
l2cap_send_move_chan_cfm_icid+0x8e/0xb1 net/bluetooth/l2cap_core.c:4661
l2cap_move_fail net/bluetooth/l2cap_core.c:5146 [inline]
l2cap_move_channel_rsp net/bluetooth/l2cap_core.c:5185 [inline]
l2cap_bredr_sig_cmd net/bluetooth/l2cap_core.c:5464 [inline]
l2cap_sig_channel net/bluetooth/l2cap_core.c:5799 [inline]
l2cap_recv_frame+0x1d12/0x51aa net/bluetooth/l2cap_core.c:7023
l2cap_recv_acldata+0x2ea/0x693 net/bluetooth/l2cap_core.c:7596
hci_acldata_packet net/bluetooth/hci_core.c:4606 [inline]
hci_rx_work+0x2bd/0x45e net/bluetooth/hci_core.c:4796
process_one_work+0x6f8/0xb50 kernel/workqueue.c:2175
worker_thread+0x4fc/0x670 kernel/workqueue.c:2321
kthread+0x2f0/0x304 kernel/kthread.c:253
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:415
Allocated by task 38:
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0x8d/0x9a mm/kasan/kasan.c:553
kmem_cache_alloc_trace+0x102/0x129 mm/slub.c:2787
kmalloc include/linux/slab.h:515 [inline]
kzalloc include/linux/slab.h:709 [inline]
hci_chan_create+0x86/0x26d net/bluetooth/hci_conn.c:1674
l2cap_conn_add.part.0+0x1c/0x814 net/bluetooth/l2cap_core.c:7062
l2cap_conn_add net/bluetooth/l2cap_core.c:7059 [inline]
l2cap_connect_cfm+0x134/0x852 net/bluetooth/l2cap_core.c:7381
hci_connect_cfm+0x9d/0x122 include/net/bluetooth/hci_core.h:1404
hci_remote_ext_features_evt net/bluetooth/hci_event.c:4161 [inline]
hci_event_packet+0x463f/0x72fa net/bluetooth/hci_event.c:5981
hci_rx_work+0x197/0x45e net/bluetooth/hci_core.c:4791
process_one_work+0x6f8/0xb50 kernel/workqueue.c:2175
worker_thread+0x4fc/0x670 kernel/workqueue.c:2321
kthread+0x2f0/0x304 kernel/kthread.c:253
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:415
Freed by task 1732:
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free mm/kasan/kasan.c:521 [inline]
__kasan_slab_free+0x106/0x128 mm/kasan/kasan.c:493
slab_free_hook mm/slub.c:1409 [inline]
slab_free_freelist_hook+0xaa/0xf6 mm/slub.c:1436
slab_free mm/slub.c:3009 [inline]
kfree+0x182/0x21e mm/slub.c:3972
hci_disconn_loglink_complete_evt net/bluetooth/hci_event.c:4891 [inline]
hci_event_packet+0x6a1c/0x72fa net/bluetooth/hci_event.c:6050
hci_rx_work+0x197/0x45e net/bluetooth/hci_core.c:4791
process_one_work+0x6f8/0xb50 kernel/workqueue.c:2175
worker_thread+0x4fc/0x670 kernel/workqueue.c:2321
kthread+0x2f0/0x304 kernel/kthread.c:253
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:415
The buggy address belongs to the object at 8881d7af9180
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 24 bytes inside of
128-byte region [8881d7af9180, 8881d7af9200)
The buggy address belongs to the page:
page:ea00075ebe40 count:1 mapcount:0 mapping:8881da403200 index:0x0
flags: 0x8200(slab)
raw: 8200 dead0100 dead0200 8881da403200
raw: 80150015 0001
page dumped because: kasan: bad access detected
Memory state around the buggy address:
8881d7af9080: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
8881d7af9100: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>8881d7af9180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
8881d7af9200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
8881d7af9280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
Signed-off-by: Archie Pusaka
Reported-by: syzbot+98228e7407314d2d4...@syzkaller.appspotmail.com
Reviewed-by: Alain Michaud
Reviewed-by: Abhishek Pandit-Subedi
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_event.c| 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebdd4afe30d2..ca4ac6603b9a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -704,6 +704,7 @@ struct hci_chan {
struct sk_buff_head data_q;
unsigned intsent;
__u8state;
+ boolamp;
};
str
[PATCH] Bluetooth: Set CONF_NOT_COMPLETE as l2cap_chan default
From: Archie Pusaka Currently l2cap_chan_set_defaults() reset chan->conf_state to zero. However, there is a flag CONF_NOT_COMPLETE which is set when creating the l2cap_chan. It is suggested that the flag should be cleared when l2cap_chan is ready, but when l2cap_chan_set_defaults() is called, l2cap_chan is not yet ready. Therefore, we must set this flag as the default. Example crash call trace: __dump_stack lib/dump_stack.c:15 [inline] dump_stack+0xc4/0x118 lib/dump_stack.c:56 panic+0x1c6/0x38b kernel/panic.c:117 __warn+0x170/0x1b9 kernel/panic.c:471 warn_slowpath_fmt+0xc7/0xf8 kernel/panic.c:494 debug_print_object+0x175/0x193 lib/debugobjects.c:260 debug_object_assert_init+0x171/0x1bf lib/debugobjects.c:614 debug_timer_assert_init kernel/time/timer.c:629 [inline] debug_assert_init kernel/time/timer.c:677 [inline] del_timer+0x7c/0x179 kernel/time/timer.c:1034 try_to_grab_pending+0x81/0x2e5 kernel/workqueue.c:1230 cancel_delayed_work+0x7c/0x1c4 kernel/workqueue.c:2929 l2cap_clear_timer+0x1e/0x41 include/net/bluetooth/l2cap.h:834 l2cap_chan_del+0x2d8/0x37e net/bluetooth/l2cap_core.c:640 l2cap_chan_close+0x532/0x5d8 net/bluetooth/l2cap_core.c:756 l2cap_sock_shutdown+0x806/0x969 net/bluetooth/l2cap_sock.c:1174 l2cap_sock_release+0x64/0x14d net/bluetooth/l2cap_sock.c:1217 __sock_release+0xda/0x217 net/socket.c:580 sock_close+0x1b/0x1f net/socket.c:1039 __fput+0x322/0x55c fs/file_table.c:208 fput+0x17/0x19 fs/file_table.c:244 task_work_run+0x19b/0x1d3 kernel/task_work.c:115 exit_task_work include/linux/task_work.h:21 [inline] do_exit+0xe4c/0x204a kernel/exit.c:766 do_group_exit+0x291/0x291 kernel/exit.c:891 get_signal+0x749/0x1093 kernel/signal.c:2396 do_signal+0xa5/0xcdb arch/x86/kernel/signal.c:737 exit_to_usermode_loop arch/x86/entry/common.c:243 [inline] prepare_exit_to_usermode+0xed/0x235 arch/x86/entry/common.c:277 syscall_return_slowpath+0x3a7/0x3b3 arch/x86/entry/common.c:348 int_ret_from_sys_call+0x25/0xa3 Signed-off-by: Archie Pusaka Reported-by: syzbot+338f014a98367a08a...@syzkaller.appspotmail.com Reviewed-by: Alain Michaud Reviewed-by: Abhishek Pandit-Subedi Reviewed-by: Guenter Roeck --- net/bluetooth/l2cap_core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 374cc32d7138..59ab9689b37d 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -516,7 +516,9 @@ void l2cap_chan_set_defaults(struct l2cap_chan *chan) chan->flush_to = L2CAP_DEFAULT_FLUSH_TO; chan->retrans_timeout = L2CAP_DEFAULT_RETRANS_TO; chan->monitor_timeout = L2CAP_DEFAULT_MONITOR_TO; + chan->conf_state = 0; + set_bit(CONF_NOT_COMPLETE, >conf_state); set_bit(FLAG_FORCE_ACTIVE, >flags); } -- 2.31.0.rc2.261.g7f71774620-goog
[PATCH] Bluetooth: check for zapped sk before connecting
From: Archie Pusaka
There is a possibility of receiving a zapped sock on
l2cap_sock_connect(). This could lead to interesting crashes, one
such case is tearing down an already tore l2cap_sock as is happened
with this call trace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0xc4/0x118 lib/dump_stack.c:56
register_lock_class kernel/locking/lockdep.c:792 [inline]
register_lock_class+0x239/0x6f6 kernel/locking/lockdep.c:742
__lock_acquire+0x209/0x1e27 kernel/locking/lockdep.c:3105
lock_acquire+0x29c/0x2fb kernel/locking/lockdep.c:3599
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:137 [inline]
_raw_spin_lock_bh+0x38/0x47 kernel/locking/spinlock.c:175
spin_lock_bh include/linux/spinlock.h:307 [inline]
lock_sock_nested+0x44/0xfa net/core/sock.c:2518
l2cap_sock_teardown_cb+0x88/0x2fb net/bluetooth/l2cap_sock.c:1345
l2cap_chan_del+0xa3/0x383 net/bluetooth/l2cap_core.c:598
l2cap_chan_close+0x537/0x5dd net/bluetooth/l2cap_core.c:756
l2cap_chan_timeout+0x104/0x17e net/bluetooth/l2cap_core.c:429
process_one_work+0x7e3/0xcb0 kernel/workqueue.c:2064
worker_thread+0x5a5/0x773 kernel/workqueue.c:2196
kthread+0x291/0x2a6 kernel/kthread.c:211
ret_from_fork+0x4e/0x80 arch/x86/entry/entry_64.S:604
Signed-off-by: Archie Pusaka
Reported-by: syzbot+abfc0f5e668d4099a...@syzkaller.appspotmail.com
Reviewed-by: Alain Michaud
Reviewed-by: Abhishek Pandit-Subedi
Reviewed-by: Guenter Roeck
---
net/bluetooth/l2cap_sock.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index f1b1edd0b697..b86fd8cc4dc1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -182,6 +182,13 @@ static int l2cap_sock_connect(struct socket *sock, struct
sockaddr *addr,
BT_DBG("sk %p", sk);
+ lock_sock(sk);
+ if (sock_flag(sk, SOCK_ZAPPED)) {
+ release_sock(sk);
+ return -EINVAL;
+ }
+ release_sock(sk);
+
if (!addr || alen < offsetofend(struct sockaddr, sa_family) ||
addr->sa_family != AF_BLUETOOTH)
return -EINVAL;
--
2.31.0.rc2.261.g7f71774620-goog
[PATCH v6 0/7] MSFT offloading support for advertisement monitor
From: Archie Pusaka Hi linux-bluetooth, This series of patches manages the hardware offloading part of MSFT extension API. The full documentation can be accessed by this link: https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events Only four of the HCI commands are planned to be implemented: HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), HCI_VS_MSFT_LE_Monitor_Advertisement, HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. These are the commands which would be used for advertisement monitor feature. Only if the controller supports the MSFT extension would these commands be sent. Otherwise, software-based monitoring would be performed in the user space instead. Thanks in advance for your feedback! Archie Changes in v6: * New patch "advmon offload MSFT interleave scanning integration" * New patch "disable advertisement filters during suspend" Changes in v5: * Discard struct flags on msft_data and use it's members directly Changes in v4: * Change the logic of merging add_adv_patterns_monitor with rssi * Aligning variable declaration on mgmt.h * Replacing the usage of BT_DBG with bt_dev_dbg Changes in v3: * Flips the order of rssi and pattern_count on mgmt struct * Fix return type of msft_remove_monitor Changes in v2: * Add a new opcode instead of modifying an existing one * Also implement the new MGMT opcode and merge the functionality with the old one. Archie Pusaka (6): Bluetooth: advmon offload MSFT add rssi support Bluetooth: advmon offload MSFT add monitor Bluetooth: advmon offload MSFT remove monitor Bluetooth: advmon offload MSFT handle controller reset Bluetooth: advmon offload MSFT handle filter enablement Bluetooth: advmon offload MSFT interleave scanning integration Howard Chung (1): Bluetooth: disable advertisement filters during suspend include/net/bluetooth/hci_core.h | 36 ++- include/net/bluetooth/mgmt.h | 16 ++ net/bluetooth/hci_core.c | 174 +--- net/bluetooth/hci_request.c | 49 +++- net/bluetooth/mgmt.c | 391 +++--- net/bluetooth/msft.c | 460 ++- net/bluetooth/msft.h | 30 ++ 7 files changed, 1015 insertions(+), 141 deletions(-) -- 2.30.0.280.ga3ce27912f-goog
[PATCH v6 3/7] Bluetooth: advmon offload MSFT remove monitor
From: Archie Pusaka
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
Reported-by: kernel test robot
Reported-by: Dan Carpenter
---
(no changes since v3)
Changes in v3:
* Fix return type of msft_remove_monitor
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++--
net/bluetooth/mgmt.c | 110 +-
net/bluetooth/msft.c | 127 ++-
net/bluetooth/msft.h | 9 +++
5 files changed, 323 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8
instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 625298f64a20..b0a63f643a07 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(>adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, >patterns, list)
+ list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+ list_del(>list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(>adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3122,39 +3140,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function
[PATCH v6 2/7] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v4)
Changes in v4:
* Replacing the usage of BT_DBG with bt_dev_dbg
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 55 +++--
net/bluetooth/mgmt.c | 114 +-
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 356 insertions(+), 43 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..625298f64a20 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,56 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI
[PATCH v6 1/7] Bluetooth: advmon offload MSFT add rssi support
From: Archie Pusaka
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt. This adds a new opcode
to add advertisement monitor with rssi parameters.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v4)
Changes in v4:
* Change the logic of merging add_adv_patterns_monitor with rssi
* Aligning variable declaration on mgmt.h
Changes in v3:
* Flips the order of rssi and pattern_count on mgmt struct
Changes in v2:
* Add a new opcode instead of modifying an existing one
include/net/bluetooth/hci_core.h | 9 ++
include/net/bluetooth/mgmt.h | 16 +++
net/bluetooth/mgmt.c | 225 +--
3 files changed, 178 insertions(+), 72 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 677a8c50b2ad..8b7cf3620938 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,8 +250,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
boolactive;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f9a6638e20b3..839a2028009e 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
__u8instance;
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8high_threshold;
+ __le16 high_threshold_timeout;
+ __s8low_threshold;
+ __le16 low_threshold_timeout;
+ __u8sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+ struct mgmt_adv_rssi_thresholds rssi;
+ __u8pattern_count;
+ struct mgmt_adv_pattern patterns[];
+} __packed;
+#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE8
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 608dda5403b7..72d37c80e071 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_REMOVE_ADV_MONITOR,
MGMT_OP_ADD_EXT_ADV_PARAMS,
MGMT_OP_ADD_EXT_ADV_DATA,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
};
static const u16 mgmt_events[] = {
@@ -4225,75 +4226,15 @@ static int read_adv_mon_features(struct sock *sk,
struct hci_dev *hdev,
return err;
}
-static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ struct adv_monitor *m, u8 status, u16 op)
{
- struct mgmt_cp_add_adv_patterns_monitor *cp = data;
struct mgmt_rp_add_adv_patterns_monitor rp;
- struct adv_monitor *m = NULL;
- struct adv_pattern *p = NULL;
- unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
- __u8 cp_ofst = 0, cp_len = 0;
- int err, i;
-
- BT_DBG("request for %s", hdev->name);
-
- if (len <= sizeof(*cp) || cp->pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
- m = kmalloc(sizeof(*m), GFP_KERNEL);
- if (!m) {
- err = -ENOMEM;
- goto failed;
- }
-
- INIT_LIST_HEAD(>patterns);
- m->active = false;
-
- for (i = 0; i < cp->pattern_count; i++) {
- if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
- cp_ofst = cp->patterns[i].offset;
- cp_len = cp->patterns[i].length;
- if (cp_ofst >= HCI_MAX_AD_LENGTH ||
- cp_len > HCI_MAX_AD_LENGTH ||
- (cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- go
[PATCH v6 6/7] Bluetooth: advmon offload MSFT interleave scanning integration
From: Archie Pusaka
When MSFT extension is supported, we don't have to interleave the scan
as we could just do allowlist scan.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
---
Changes in v6:
* New patch "advmon offload MSFT interleave scanning integration"
net/bluetooth/hci_request.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 5aa7bd5030a2..d29a44d77b4e 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -404,13 +404,18 @@ static void cancel_interleave_scan(struct hci_dev *hdev)
*/
static bool __hci_update_interleaved_scan(struct hci_dev *hdev)
{
- /* If there is at least one ADV monitors and one pending LE connection
-* or one device to be scanned for, we should alternate between
-* allowlist scan and one without any filters to save power.
+ /* 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(>pend_le_conns) &&
- list_empty(>pend_le_reports));
+ list_empty(>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) {
@@ -899,14 +904,11 @@ static u8 update_white_list(struct hci_request *req)
/* Use the allowlist unless the following conditions are all true:
* - We are not currently suspending
-* - There are 1 or more ADV monitors registered
+* - There are 1 or more ADV monitors registered and it's not offloaded
* - Interleaved scanning is not currently using the allowlist
-*
-* Once the controller offloading of advertisement monitor is in place,
-* the above condition should include the support of MSFT extension
-* support.
*/
if (!idr_is_empty(>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;
--
2.30.0.280.ga3ce27912f-goog
[PATCH v6 7/7] Bluetooth: disable advertisement filters during suspend
From: Howard Chung
This adds logic to disable and reenable advertisement filters during
suspend and resume. After this patch, we would only receive packets from
devices in allow list during suspend.
Signed-off-by: Howard Chung
Reviewed-by: Abhishek Pandit-Subedi
---
Changes in v6:
* New patch "disable advertisement filters during suspend"
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_request.c | 29 +
net/bluetooth/msft.c | 17 -
net/bluetooth/msft.h | 3 +++
4 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 29cfc6a2d689..fd1d10fe2f11 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -105,6 +105,8 @@ enum suspend_tasks {
SUSPEND_POWERING_DOWN,
SUSPEND_PREPARE_NOTIFIER,
+
+ SUSPEND_SET_ADV_FILTER,
__SUSPEND_NUM_TASKS
};
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index d29a44d77b4e..e55976db4403 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -29,6 +29,7 @@
#include "smp.h"
#include "hci_request.h"
+#include "msft.h"
#define HCI_REQ_DONE 0
#define HCI_REQ_PEND 1
@@ -1242,6 +1243,29 @@ static void suspend_req_complete(struct hci_dev *hdev,
u8 status, u16 opcode)
clear_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
wake_up(>suspend_wait_q);
}
+
+ if (test_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks)) {
+ clear_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
+ wake_up(>suspend_wait_q);
+ }
+}
+
+static void hci_req_add_set_adv_filter_enable(struct hci_request *req,
+ bool enable)
+{
+ struct hci_dev *hdev = req->hdev;
+
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_MSFT:
+ msft_req_add_set_filter_enable(req, enable);
+ break;
+ default:
+ return;
+ }
+
+ /* No need to block when enabling since it's on resume path */
+ if (hdev->suspended && !enable)
+ set_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
}
/* Call with hci_dev_lock */
@@ -1301,6 +1325,9 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum
suspended_state next)
hci_req_add_le_scan_disable(, false);
}
+ /* Disable advertisement filters */
+ hci_req_add_set_adv_filter_enable(, false);
+
/* Mark task needing completion */
set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
@@ -1340,6 +1367,8 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum
suspended_state next)
hci_req_clear_event_filter();
/* Reset passive/background scanning to normal */
__hci_update_background_scan();
+ /* Enable all of the advertisement filters */
+ hci_req_add_set_adv_filter_enable(, true);
/* Unpause directed advertising */
hdev->advertising_paused = false;
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index b2ef654b1d3d..47b104f318e9 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -579,9 +579,19 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
-int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+void msft_req_add_set_filter_enable(struct hci_request *req, bool enable)
{
+ struct hci_dev *hdev = req->hdev;
struct msft_cp_le_set_advertisement_filter_enable cp;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_add(req, hdev->msft_opcode, sizeof(cp), );
+}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
struct hci_request req;
struct msft_data *msft = hdev->msft_data;
int err;
@@ -589,11 +599,8 @@ int msft_set_filter_enable(struct hci_dev *hdev, bool
enable)
if (!msft)
return -EOPNOTSUPP;
- cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
- cp.enable = enable;
-
hci_req_init(, hdev);
- hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ msft_req_add_set_filter_enable(, enable);
err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
return err;
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index f8e4d3a6d641..88ed613dfa08 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+void
[PATCH v6 5/7] Bluetooth: advmon offload MSFT handle filter enablement
From: Archie Pusaka
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 67
net/bluetooth/msft.h | 6
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index d25c6936daa4..b2ef654b1d3d 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -83,6 +94,7 @@ struct msft_data {
__u16 pending_add_handle;
__u16 pending_remove_handle;
__u8 reregistering;
+ __u8 filter_enabled;
};
static int __msft_add_monitor_pattern(struct hci_dev *hdev,
@@ -190,6 +202,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -395,6 +408,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct
hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+* already set to whatever we were trying to set.
+* Although the default state should be disabled, some controller set
+* the initial value to enabled. Because there is no way to know the
+* actual initial value before sending this command, here we also treat
+* error 0x0C as success.
+*/
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = >rssi;
@@ -531,3 +578,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(, hdev);
+ hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 6f126a1f1688..f8e4d3a6d641 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline int msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.30.0.280.ga3ce27912f-goog
[PATCH v6 4/7] Bluetooth: advmon offload MSFT handle controller reset
From: Archie Pusaka
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v5)
Changes in v5:
* Discard struct flags on msft_data and use it's members directly
net/bluetooth/msft.c | 76 +---
1 file changed, 71 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..d25c6936daa4 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,12 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+ __u8 reregistering;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +138,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(>adv_monitors_idr, );
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +187,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(>handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +208,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, >handle_map, list) {
+ monitor = idr_find(>adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(_data->list);
kfree(handle_data);
}
@@ -282,9 +327,15 @@ static void msft_le_monitor_advertisement_cb(struct
hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +425,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor
*monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +439,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +483,20 @@ int msft_add_monitor_pattern(struct hci_dev
Re: [PATCH v5 0/5] MSFT offloading support for advertisement monitor
Hi maintainers, Gentle ping to review this patch, thanks~! Regards, Archie On Tue, 22 Dec 2020 at 18:26, Archie Pusaka wrote: > > From: Archie Pusaka > > > Hi linux-bluetooth, > > This series of patches manages the hardware offloading part of MSFT > extension API. The full documentation can be accessed by this link: > https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events > > Only four of the HCI commands are planned to be implemented: > HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), > HCI_VS_MSFT_LE_Monitor_Advertisement, > HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and > HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. > These are the commands which would be used for advertisement monitor > feature. Only if the controller supports the MSFT extension would > these commands be sent. Otherwise, software-based monitoring would be > performed in the user space instead. > > Thanks in advance for your feedback! > > Archie > > Changes in v5: > * Discard struct flags on msft_data and use it's members directly > > Changes in v4: > * Change the logic of merging add_adv_patterns_monitor with rssi > * Aligning variable declaration on mgmt.h > * Replacing the usage of BT_DBG with bt_dev_dbg > > Changes in v3: > * Flips the order of rssi and pattern_count on mgmt struct > * Fix return type of msft_remove_monitor > > Changes in v2: > * Add a new opcode instead of modifying an existing one > * Also implement the new MGMT opcode and merge the functionality with > the old one. > > Archie Pusaka (5): > Bluetooth: advmon offload MSFT add rssi support > Bluetooth: advmon offload MSFT add monitor > Bluetooth: advmon offload MSFT remove monitor > Bluetooth: advmon offload MSFT handle controller reset > Bluetooth: advmon offload MSFT handle filter enablement > > include/net/bluetooth/hci_core.h | 34 ++- > include/net/bluetooth/mgmt.h | 16 ++ > net/bluetooth/hci_core.c | 174 +--- > net/bluetooth/mgmt.c | 391 +++--- > net/bluetooth/msft.c | 453 ++- > net/bluetooth/msft.h | 27 ++ > 6 files changed, 963 insertions(+), 132 deletions(-) > > -- > 2.29.2.729.g45daf8777d-goog >
Re: [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
Hi Marcel,
I've sent a new v5 patch to address this issue.
Thanks,
Archie
On Tue, 22 Dec 2020 at 18:03, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>> When the controller is powered off, the registered advertising monitor
> >>> is removed from the controller. This patch handles the re-registration
> >>> of those monitors when the power is on.
> >>>
> >>> Signed-off-by: Archie Pusaka
> >>> Reviewed-by: Miao-chen Chou
> >>> Reviewed-by: Yun-Hao Chung
> >>>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>> net/bluetooth/msft.c | 79 +---
> >>> 1 file changed, 74 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> >>> index f5aa0e3b1b9b..7e33a85c3f1c 100644
> >>> --- a/net/bluetooth/msft.c
> >>> +++ b/net/bluetooth/msft.c
> >>> @@ -82,8 +82,15 @@ struct msft_data {
> >>> struct list_head handle_map;
> >>> __u16 pending_add_handle;
> >>> __u16 pending_remove_handle;
> >>> +
> >>> + struct {
> >>> + u8 reregistering:1;
> >>> + } flags;
> >>> };
> >>
> >> hmmm. Do you have bigger plans with this struct? I would just skip it.
> >>
> > This struct is also used in patch 5/5 to store the "enabled" status of
> > the filter.
> > Suspend/resume would need to enable/disable the filter, but it is not
> > yet implemented in this patch series.
>
> just do it without the nested structs. I think you are overdoing it here.
>
> Regards
>
> Marcel
>
[PATCH v5 5/5] Bluetooth: advmon offload MSFT handle filter enablement
From: Archie Pusaka
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 67
net/bluetooth/msft.h | 6
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index d25c6936daa4..b2ef654b1d3d 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -83,6 +94,7 @@ struct msft_data {
__u16 pending_add_handle;
__u16 pending_remove_handle;
__u8 reregistering;
+ __u8 filter_enabled;
};
static int __msft_add_monitor_pattern(struct hci_dev *hdev,
@@ -190,6 +202,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -395,6 +408,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct
hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+* already set to whatever we were trying to set.
+* Although the default state should be disabled, some controller set
+* the initial value to enabled. Because there is no way to know the
+* actual initial value before sending this command, here we also treat
+* error 0x0C as success.
+*/
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = >rssi;
@@ -531,3 +578,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(, hdev);
+ hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 6f126a1f1688..f8e4d3a6d641 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline int msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.729.g45daf8777d-goog
[PATCH v5 4/5] Bluetooth: advmon offload MSFT handle controller reset
From: Archie Pusaka
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
Changes in v5:
* Discard struct flags on msft_data and use it's members directly
net/bluetooth/msft.c | 76 +---
1 file changed, 71 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..d25c6936daa4 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,12 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+ __u8 reregistering;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +138,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(>adv_monitors_idr, );
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +187,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(>handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +208,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, >handle_map, list) {
+ monitor = idr_find(>adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(_data->list);
kfree(handle_data);
}
@@ -282,9 +327,15 @@ static void msft_le_monitor_advertisement_cb(struct
hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +425,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor
*monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +439,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +483,20 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_
[PATCH v5 2/5] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v4)
Changes in v4:
* Replacing the usage of BT_DBG with bt_dev_dbg
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 55 +++--
net/bluetooth/mgmt.c | 114 +-
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 356 insertions(+), 43 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..625298f64a20 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,56 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI
[PATCH v5 3/5] Bluetooth: advmon offload MSFT remove monitor
From: Archie Pusaka
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
Reported-by: kernel test robot
Reported-by: Dan Carpenter
---
(no changes since v3)
Changes in v3:
* Fix return type of msft_remove_monitor
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++--
net/bluetooth/mgmt.c | 110 +-
net/bluetooth/msft.c | 127 ++-
net/bluetooth/msft.h | 9 +++
5 files changed, 323 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8
instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 625298f64a20..b0a63f643a07 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(>adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, >patterns, list)
+ list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+ list_del(>list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(>adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3122,39 +3140,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function
[PATCH v5 0/5] MSFT offloading support for advertisement monitor
From: Archie Pusaka Hi linux-bluetooth, This series of patches manages the hardware offloading part of MSFT extension API. The full documentation can be accessed by this link: https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events Only four of the HCI commands are planned to be implemented: HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), HCI_VS_MSFT_LE_Monitor_Advertisement, HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. These are the commands which would be used for advertisement monitor feature. Only if the controller supports the MSFT extension would these commands be sent. Otherwise, software-based monitoring would be performed in the user space instead. Thanks in advance for your feedback! Archie Changes in v5: * Discard struct flags on msft_data and use it's members directly Changes in v4: * Change the logic of merging add_adv_patterns_monitor with rssi * Aligning variable declaration on mgmt.h * Replacing the usage of BT_DBG with bt_dev_dbg Changes in v3: * Flips the order of rssi and pattern_count on mgmt struct * Fix return type of msft_remove_monitor Changes in v2: * Add a new opcode instead of modifying an existing one * Also implement the new MGMT opcode and merge the functionality with the old one. Archie Pusaka (5): Bluetooth: advmon offload MSFT add rssi support Bluetooth: advmon offload MSFT add monitor Bluetooth: advmon offload MSFT remove monitor Bluetooth: advmon offload MSFT handle controller reset Bluetooth: advmon offload MSFT handle filter enablement include/net/bluetooth/hci_core.h | 34 ++- include/net/bluetooth/mgmt.h | 16 ++ net/bluetooth/hci_core.c | 174 +--- net/bluetooth/mgmt.c | 391 +++--- net/bluetooth/msft.c | 453 ++- net/bluetooth/msft.h | 27 ++ 6 files changed, 963 insertions(+), 132 deletions(-) -- 2.29.2.729.g45daf8777d-goog
[PATCH v5 1/5] Bluetooth: advmon offload MSFT add rssi support
From: Archie Pusaka
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt. This adds a new opcode
to add advertisement monitor with rssi parameters.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v4)
Changes in v4:
* Change the logic of merging add_adv_patterns_monitor with rssi
* Aligning variable declaration on mgmt.h
Changes in v3:
* Flips the order of rssi and pattern_count on mgmt struct
Changes in v2:
* Add a new opcode instead of modifying an existing one
include/net/bluetooth/hci_core.h | 9 ++
include/net/bluetooth/mgmt.h | 16 +++
net/bluetooth/mgmt.c | 225 +--
3 files changed, 178 insertions(+), 72 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 677a8c50b2ad..8b7cf3620938 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,8 +250,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
boolactive;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f9a6638e20b3..839a2028009e 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
__u8instance;
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8high_threshold;
+ __le16 high_threshold_timeout;
+ __s8low_threshold;
+ __le16 low_threshold_timeout;
+ __u8sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+ struct mgmt_adv_rssi_thresholds rssi;
+ __u8pattern_count;
+ struct mgmt_adv_pattern patterns[];
+} __packed;
+#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE8
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 608dda5403b7..72d37c80e071 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_REMOVE_ADV_MONITOR,
MGMT_OP_ADD_EXT_ADV_PARAMS,
MGMT_OP_ADD_EXT_ADV_DATA,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
};
static const u16 mgmt_events[] = {
@@ -4225,75 +4226,15 @@ static int read_adv_mon_features(struct sock *sk,
struct hci_dev *hdev,
return err;
}
-static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ struct adv_monitor *m, u8 status, u16 op)
{
- struct mgmt_cp_add_adv_patterns_monitor *cp = data;
struct mgmt_rp_add_adv_patterns_monitor rp;
- struct adv_monitor *m = NULL;
- struct adv_pattern *p = NULL;
- unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
- __u8 cp_ofst = 0, cp_len = 0;
- int err, i;
-
- BT_DBG("request for %s", hdev->name);
-
- if (len <= sizeof(*cp) || cp->pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
- m = kmalloc(sizeof(*m), GFP_KERNEL);
- if (!m) {
- err = -ENOMEM;
- goto failed;
- }
-
- INIT_LIST_HEAD(>patterns);
- m->active = false;
-
- for (i = 0; i < cp->pattern_count; i++) {
- if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
- cp_ofst = cp->patterns[i].offset;
- cp_len = cp->patterns[i].length;
- if (cp_ofst >= HCI_MAX_AD_LENGTH ||
- cp_len > HCI_MAX_AD_LENGTH ||
- (cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- go
[PATCH v4 5/5] Bluetooth: advmon offload MSFT handle filter enablement
From: Archie Pusaka
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 67
net/bluetooth/msft.h | 6
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 7e33a85c3f1c..055cc5a260df 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -85,6 +96,7 @@ struct msft_data {
struct {
u8 reregistering:1;
+ u8 filter_enabled:1;
} flags;
};
@@ -193,6 +205,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->flags.reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -398,6 +411,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct
hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+* already set to whatever we were trying to set.
+* Although the default state should be disabled, some controller set
+* the initial value to enabled. Because there is no way to know the
+* actual initial value before sending this command, here we also treat
+* error 0x0C as success.
+*/
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->flags.filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = >rssi;
@@ -534,3 +581,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(, hdev);
+ hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 6f126a1f1688..f8e4d3a6d641 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline int msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.729.g45daf8777d-goog
[PATCH v4 4/5] Bluetooth: advmon offload MSFT handle controller reset
From: Archie Pusaka
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 79 +---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..7e33a85c3f1c 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,15 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+
+ struct {
+ u8 reregistering:1;
+ } flags;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +141,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(>adv_monitors_idr, );
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->flags.reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +190,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(>handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->flags.reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +211,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, >handle_map, list) {
+ monitor = idr_find(>adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(_data->list);
kfree(handle_data);
}
@@ -282,9 +330,15 @@ static void msft_le_monitor_advertisement_cb(struct
hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->flags.reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->flags.reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +428,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor
*monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +442,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +486,20 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, s
[PATCH v4 3/5] Bluetooth: advmon offload MSFT remove monitor
From: Archie Pusaka
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
Reported-by: kernel test robot
Reported-by: Dan Carpenter
---
(no changes since v3)
Changes in v3:
* Fix return type of msft_remove_monitor
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++--
net/bluetooth/mgmt.c | 110 +-
net/bluetooth/msft.c | 127 ++-
net/bluetooth/msft.h | 9 +++
5 files changed, 323 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8
instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 625298f64a20..b0a63f643a07 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(>adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, >patterns, list)
+ list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+ list_del(>list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(>adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3122,39 +3140,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function
[PATCH v4 1/5] Bluetooth: advmon offload MSFT add rssi support
From: Archie Pusaka
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt. This adds a new opcode
to add advertisement monitor with rssi parameters.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
Changes in v4:
* Change the logic of merging add_adv_patterns_monitor with rssi
* Aligning variable declaration on mgmt.h
Changes in v3:
* Flips the order of rssi and pattern_count on mgmt struct
Changes in v2:
* Add a new opcode instead of modifying an existing one
include/net/bluetooth/hci_core.h | 9 ++
include/net/bluetooth/mgmt.h | 16 +++
net/bluetooth/mgmt.c | 225 +--
3 files changed, 178 insertions(+), 72 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 677a8c50b2ad..8b7cf3620938 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,8 +250,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
boolactive;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f9a6638e20b3..839a2028009e 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
__u8instance;
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8high_threshold;
+ __le16 high_threshold_timeout;
+ __s8low_threshold;
+ __le16 low_threshold_timeout;
+ __u8sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+ struct mgmt_adv_rssi_thresholds rssi;
+ __u8pattern_count;
+ struct mgmt_adv_pattern patterns[];
+} __packed;
+#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE8
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 608dda5403b7..72d37c80e071 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_REMOVE_ADV_MONITOR,
MGMT_OP_ADD_EXT_ADV_PARAMS,
MGMT_OP_ADD_EXT_ADV_DATA,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
};
static const u16 mgmt_events[] = {
@@ -4225,75 +4226,15 @@ static int read_adv_mon_features(struct sock *sk,
struct hci_dev *hdev,
return err;
}
-static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ struct adv_monitor *m, u8 status, u16 op)
{
- struct mgmt_cp_add_adv_patterns_monitor *cp = data;
struct mgmt_rp_add_adv_patterns_monitor rp;
- struct adv_monitor *m = NULL;
- struct adv_pattern *p = NULL;
- unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
- __u8 cp_ofst = 0, cp_len = 0;
- int err, i;
-
- BT_DBG("request for %s", hdev->name);
-
- if (len <= sizeof(*cp) || cp->pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
- m = kmalloc(sizeof(*m), GFP_KERNEL);
- if (!m) {
- err = -ENOMEM;
- goto failed;
- }
-
- INIT_LIST_HEAD(>patterns);
- m->active = false;
-
- for (i = 0; i < cp->pattern_count; i++) {
- if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
- cp_ofst = cp->patterns[i].offset;
- cp_len = cp->patterns[i].length;
- if (cp_ofst >= HCI_MAX_AD_LENGTH ||
- cp_len > HCI_MAX_AD_LENGTH ||
- (cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
-
[PATCH v4 2/5] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
Changes in v4:
* Replacing the usage of BT_DBG with bt_dev_dbg
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 55 +++--
net/bluetooth/mgmt.c | 114 +-
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 356 insertions(+), 43 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..625298f64a20 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,56 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE:
+ hci_update
[PATCH v4 0/5] MSFT offloading support for advertisement monitor
From: Archie Pusaka Hi linux-bluetooth, This series of patches manages the hardware offloading part of MSFT extension API. The full documentation can be accessed by this link: https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events Only four of the HCI commands are planned to be implemented: HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), HCI_VS_MSFT_LE_Monitor_Advertisement, HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. These are the commands which would be used for advertisement monitor feature. Only if the controller supports the MSFT extension would these commands be sent. Otherwise, software-based monitoring would be performed in the user space instead. Thanks in advance for your feedback! Archie Changes in v4: * Change the logic of merging add_adv_patterns_monitor with rssi * Aligning variable declaration on mgmt.h * Replacing the usage of BT_DBG with bt_dev_dbg Changes in v3: * Flips the order of rssi and pattern_count on mgmt struct * Fix return type of msft_remove_monitor Changes in v2: * Add a new opcode instead of modifying an existing one * Also implement the new MGMT opcode and merge the functionality with the old one. Archie Pusaka (5): Bluetooth: advmon offload MSFT add rssi support Bluetooth: advmon offload MSFT add monitor Bluetooth: advmon offload MSFT remove monitor Bluetooth: advmon offload MSFT handle controller reset Bluetooth: advmon offload MSFT handle filter enablement include/net/bluetooth/hci_core.h | 34 ++- include/net/bluetooth/mgmt.h | 16 ++ net/bluetooth/hci_core.c | 174 +--- net/bluetooth/mgmt.c | 391 +++--- net/bluetooth/msft.c | 456 ++- net/bluetooth/msft.h | 27 ++ 6 files changed, 966 insertions(+), 132 deletions(-) -- 2.29.2.729.g45daf8777d-goog
Re: [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
Hi Marcel,
On Mon, 21 Dec 2020 at 17:12, Marcel Holtmann wrote:
>
> Hi Archie,
>
> > When the controller is powered off, the registered advertising monitor
> > is removed from the controller. This patch handles the re-registration
> > of those monitors when the power is on.
> >
> > Signed-off-by: Archie Pusaka
> > Reviewed-by: Miao-chen Chou
> > Reviewed-by: Yun-Hao Chung
> >
> > ---
> >
> > (no changes since v1)
> >
> > net/bluetooth/msft.c | 79 +---
> > 1 file changed, 74 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> > index f5aa0e3b1b9b..7e33a85c3f1c 100644
> > --- a/net/bluetooth/msft.c
> > +++ b/net/bluetooth/msft.c
> > @@ -82,8 +82,15 @@ struct msft_data {
> > struct list_head handle_map;
> > __u16 pending_add_handle;
> > __u16 pending_remove_handle;
> > +
> > + struct {
> > + u8 reregistering:1;
> > + } flags;
> > };
>
> hmmm. Do you have bigger plans with this struct? I would just skip it.
>
This struct is also used in patch 5/5 to store the "enabled" status of
the filter.
Suspend/resume would need to enable/disable the filter, but it is not
yet implemented in this patch series.
Thanks,
Archie
[PATCH v3 3/5] Bluetooth: advmon offload MSFT remove monitor
From: Archie Pusaka
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
Reported-by: kernel test robot
Reported-by: Dan Carpenter
---
Changes in v3:
* Fix return type of msft_remove_monitor
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++--
net/bluetooth/mgmt.c | 110 +-
net/bluetooth/msft.c | 127 ++-
net/bluetooth/msft.h | 9 +++
5 files changed, 323 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8
instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fa13e35f775d..782b05ca0d0c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(>adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, >patterns, list)
+ list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+ list_del(>list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(>adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3121,39 +3139,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller
[PATCH v3 5/5] Bluetooth: advmon offload MSFT handle filter enablement
From: Archie Pusaka
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 67
net/bluetooth/msft.h | 6
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 7e33a85c3f1c..055cc5a260df 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -85,6 +96,7 @@ struct msft_data {
struct {
u8 reregistering:1;
+ u8 filter_enabled:1;
} flags;
};
@@ -193,6 +205,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->flags.reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -398,6 +411,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct
hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+* already set to whatever we were trying to set.
+* Although the default state should be disabled, some controller set
+* the initial value to enabled. Because there is no way to know the
+* actual initial value before sending this command, here we also treat
+* error 0x0C as success.
+*/
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->flags.filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = >rssi;
@@ -534,3 +581,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(, hdev);
+ hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 6f126a1f1688..f8e4d3a6d641 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline int msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.684.gfbc64c5ab5-goog
[PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v2)
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 54 +++--
net/bluetooth/mgmt.c | 144 ++
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 367 insertions(+), 61 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..fa13e35f775d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE:
+ hci_update_background_scan(hdev);
+
[PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
From: Archie Pusaka
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 79 +---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..7e33a85c3f1c 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,15 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+
+ struct {
+ u8 reregistering:1;
+ } flags;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +141,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(>adv_monitors_idr, );
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->flags.reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +190,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(>handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->flags.reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +211,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, >handle_map, list) {
+ monitor = idr_find(>adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(_data->list);
kfree(handle_data);
}
@@ -282,9 +330,15 @@ static void msft_le_monitor_advertisement_cb(struct
hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->flags.reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->flags.reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +428,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor
*monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +442,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +486,20 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, s
[PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support
From: Archie Pusaka
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt. This adds a new opcode
to add advertisement monitor with rssi parameters.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
Changes in v3:
* Flips the order of rssi and pattern_count on mgmt struct
Changes in v2:
* Add a new opcode instead of modifying an existing one
include/net/bluetooth/hci_core.h | 9 +++
include/net/bluetooth/mgmt.h | 16 ++
net/bluetooth/mgmt.c | 99
3 files changed, 101 insertions(+), 23 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 677a8c50b2ad..8b7cf3620938 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,8 +250,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
boolactive;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f9a6638e20b3..9917b911a4fb 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
__u8instance;
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8 high_threshold;
+ __le16 high_threshold_timeout;
+ __s8 low_threshold;
+ __le16 low_threshold_timeout;
+ __u8 sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+ struct mgmt_adv_rssi_thresholds rssi;
+ __u8 pattern_count;
+ struct mgmt_adv_pattern patterns[];
+} __packed;
+#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE8
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fa0f7a4a1d2f..cd574054aa39 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_REMOVE_ADV_MONITOR,
MGMT_OP_ADD_EXT_ADV_PARAMS,
MGMT_OP_ADD_EXT_ADV_DATA,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
};
static const u16 mgmt_events[] = {
@@ -4225,22 +4226,40 @@ static int read_adv_mon_features(struct sock *sk,
struct hci_dev *hdev,
return err;
}
-static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len, u16 op)
{
- struct mgmt_cp_add_adv_patterns_monitor *cp = data;
+ struct mgmt_cp_add_adv_patterns_monitor *cp = NULL;
+ struct mgmt_cp_add_adv_patterns_monitor_rssi *cp_rssi = NULL;
struct mgmt_rp_add_adv_patterns_monitor rp;
+ struct mgmt_adv_rssi_thresholds *rssi = NULL;
+ struct mgmt_adv_pattern *patterns = NULL;
struct adv_monitor *m = NULL;
struct adv_pattern *p = NULL;
unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
__u8 cp_ofst = 0, cp_len = 0;
int err, i;
+ u8 pattern_count;
+ u16 expected_len;
BT_DBG("request for %s", hdev->name);
- if (len <= sizeof(*cp) || cp->pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ if (op == MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI) {
+ cp_rssi = data;
+ pattern_count = cp_rssi->pattern_count;
+ rssi = _rssi->rssi;
+ patterns = cp_rssi->patterns;
+ expected_len = sizeof(*cp_rssi) +
+ pattern_count * sizeof(*patterns);
+ } else {
+ cp = data;
+ pattern_count = cp->pattern_count;
+ patterns = cp->patterns;
+ expected_len = sizeof(*cp) + pattern_count * sizeof(*patterns);
+ }
+
+ if (len != expected_len || pattern_count == 0) {
+ err = mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_INVALID_PARAMS);
goto failed;
}
@@ -4254,21 +4273,40 @@ static int add_adv_patterns_monitor(struct sock *sk,
struct hci_dev *hdev,
INIT_LIST_HEAD(>patterns);
m->active = false;
- for (i = 0; i < cp->pattern_count; i++) {
+ if (rssi) {
+ m->rssi.low_threshold = rssi->lo
[PATCH v3 0/5] MSFT offloading support for advertisement monitor
From: Archie Pusaka Hi linux-bluetooth, This series of patches manages the hardware offloading part of MSFT extension API. The full documentation can be accessed by this link: https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events Only four of the HCI commands are planned to be implemented: HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), HCI_VS_MSFT_LE_Monitor_Advertisement, HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. These are the commands which would be used for advertisement monitor feature. Only if the controller supports the MSFT extension would these commands be sent. Otherwise, software-based monitoring would be performed in the user space instead. Thanks in advance for your feedback! Archie Changes in v3: * Flips the order of rssi and pattern_count on mgmt struct * Fix return type of msft_remove_monitor Changes in v2: * Add a new opcode instead of modifying an existing one * Also implement the new MGMT opcode and merge the functionality with the old one. Archie Pusaka (5): Bluetooth: advmon offload MSFT add rssi support Bluetooth: advmon offload MSFT add monitor Bluetooth: advmon offload MSFT remove monitor Bluetooth: advmon offload MSFT handle controller reset Bluetooth: advmon offload MSFT handle filter enablement include/net/bluetooth/hci_core.h | 34 ++- include/net/bluetooth/mgmt.h | 16 ++ net/bluetooth/hci_core.c | 173 +--- net/bluetooth/mgmt.c | 333 -- net/bluetooth/msft.c | 456 ++- net/bluetooth/msft.h | 27 ++ 6 files changed, 919 insertions(+), 120 deletions(-) -- 2.29.2.684.gfbc64c5ab5-goog
Re: [PATCH v2 3/5] Bluetooth: advmon offload MSFT remove monitor
Hi Dan,
On Wed, 16 Dec 2020 at 03:06, Dan Carpenter wrote:
>
> Hi Archie,
>
> url:
> https://github.com/0day-ci/linux/commits/Archie-Pusaka/MSFT-offloading-support-for-advertisement-monitor/20201215-163858
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
> master
> config: i386-randconfig-m021-20201215 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot
> Reported-by: Dan Carpenter
>
> smatch warnings:
> net/bluetooth/msft.h:45 msft_remove_monitor() warn: signedness bug returning
> '(-95)'
>
> vim +45 net/bluetooth/msft.h
>
> d8ca4cbc63c57c8 Archie Pusaka 2020-12-15 41 static inline bool
> msft_remove_monitor(struct hci_dev *hdev,
>
> d8ca4cbc63c57c8 Archie Pusaka 2020-12-15 42
> struct adv_monitor *monitor,
> d8ca4cbc63c57c8 Archie Pusaka 2020-12-15 43
> u16 handle)
> d8ca4cbc63c57c8 Archie Pusaka 2020-12-15 44 {
> d8ca4cbc63c57c8 Archie Pusaka 2020-12-15 @45 return -EOPNOTSUPP;
> ^^^^^^^
> change this to return false?
>
Thanks. Will fix.
> d8ca4cbc63c57c8 Archie Pusaka 2020-12-15 46 }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Regards,
Archie
[PATCH v2 2/5] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 54 +++--
net/bluetooth/mgmt.c | 144 ++
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 367 insertions(+), 61 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..fa13e35f775d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE:
+ hci_update_background_scan(hdev);
+ BT_DBG("%s ad
[PATCH v2 4/5] Bluetooth: advmon offload MSFT handle controller reset
From: Archie Pusaka
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 79 +---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..7e33a85c3f1c 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,15 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+
+ struct {
+ u8 reregistering:1;
+ } flags;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +141,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(>adv_monitors_idr, );
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->flags.reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +190,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(>handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->flags.reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +211,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, >handle_map, list) {
+ monitor = idr_find(>adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(_data->list);
kfree(handle_data);
}
@@ -282,9 +330,15 @@ static void msft_le_monitor_advertisement_cb(struct
hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->flags.reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->flags.reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +428,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor
*monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +442,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +486,20 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, s
[PATCH v2 5/5] Bluetooth: advmon offload MSFT handle filter enablement
From: Archie Pusaka
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
net/bluetooth/msft.c | 67
net/bluetooth/msft.h | 6
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 7e33a85c3f1c..055cc5a260df 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -85,6 +96,7 @@ struct msft_data {
struct {
u8 reregistering:1;
+ u8 filter_enabled:1;
} flags;
};
@@ -193,6 +205,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->flags.reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -398,6 +411,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct
hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+* already set to whatever we were trying to set.
+* Although the default state should be disabled, some controller set
+* the initial value to enabled. Because there is no way to know the
+* actual initial value before sending this command, here we also treat
+* error 0x0C as success.
+*/
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->flags.filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = >rssi;
@@ -534,3 +581,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(, hdev);
+ hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 9f9a11f90b0c..44bee705c16d 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline bool msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.684.gfbc64c5ab5-goog
[PATCH v2 3/5] Bluetooth: advmon offload MSFT remove monitor
From: Archie Pusaka
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
(no changes since v1)
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++--
net/bluetooth/mgmt.c | 110 +-
net/bluetooth/msft.c | 127 ++-
net/bluetooth/msft.h | 9 +++
5 files changed, 323 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8
instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fa13e35f775d..782b05ca0d0c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(>adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, >patterns, list)
+ list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+ list_del(>list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(>adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3121,39 +3139,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+static bool hci_remove_adv_monitor(struct hci_dev *hdev,
+
[PATCH v2 1/5] Bluetooth: advmon offload MSFT add rssi support
From: Archie Pusaka
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt. This adds a new opcode
to add advertisement monitor with rssi parameters.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
Changes in v2:
* Add a new opcode instead of modifying an existing one
include/net/bluetooth/hci_core.h | 9 +++
include/net/bluetooth/mgmt.h | 16 ++
net/bluetooth/mgmt.c | 99
3 files changed, 101 insertions(+), 23 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 677a8c50b2ad..8b7cf3620938 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,8 +250,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
boolactive;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f9a6638e20b3..f3b1460b463d 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
__u8instance;
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8 high_threshold;
+ __le16 high_threshold_timeout;
+ __s8 low_threshold;
+ __le16 low_threshold_timeout;
+ __u8 sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+ __u8 pattern_count;
+ struct mgmt_adv_rssi_thresholds rssi;
+ struct mgmt_adv_pattern patterns[];
+} __packed;
+#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE8
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fa0f7a4a1d2f..cd574054aa39 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_REMOVE_ADV_MONITOR,
MGMT_OP_ADD_EXT_ADV_PARAMS,
MGMT_OP_ADD_EXT_ADV_DATA,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
};
static const u16 mgmt_events[] = {
@@ -4225,22 +4226,40 @@ static int read_adv_mon_features(struct sock *sk,
struct hci_dev *hdev,
return err;
}
-static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len, u16 op)
{
- struct mgmt_cp_add_adv_patterns_monitor *cp = data;
+ struct mgmt_cp_add_adv_patterns_monitor *cp = NULL;
+ struct mgmt_cp_add_adv_patterns_monitor_rssi *cp_rssi = NULL;
struct mgmt_rp_add_adv_patterns_monitor rp;
+ struct mgmt_adv_rssi_thresholds *rssi = NULL;
+ struct mgmt_adv_pattern *patterns = NULL;
struct adv_monitor *m = NULL;
struct adv_pattern *p = NULL;
unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
__u8 cp_ofst = 0, cp_len = 0;
int err, i;
+ u8 pattern_count;
+ u16 expected_len;
BT_DBG("request for %s", hdev->name);
- if (len <= sizeof(*cp) || cp->pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ if (op == MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI) {
+ cp_rssi = data;
+ pattern_count = cp_rssi->pattern_count;
+ rssi = _rssi->rssi;
+ patterns = cp_rssi->patterns;
+ expected_len = sizeof(*cp_rssi) +
+ pattern_count * sizeof(*patterns);
+ } else {
+ cp = data;
+ pattern_count = cp->pattern_count;
+ patterns = cp->patterns;
+ expected_len = sizeof(*cp) + pattern_count * sizeof(*patterns);
+ }
+
+ if (len != expected_len || pattern_count == 0) {
+ err = mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_INVALID_PARAMS);
goto failed;
}
@@ -4254,21 +4273,40 @@ static int add_adv_patterns_monitor(struct sock *sk,
struct hci_dev *hdev,
INIT_LIST_HEAD(>patterns);
m->active = false;
- for (i = 0; i < cp->pattern_count; i++) {
+ if (rssi) {
+ m->rssi.low_threshold = rssi->low_threshold;
+ m->rssi.low_threshold_timeout =
+
[PATCH v2 0/5] MSFT offloading support for advertisement monitor
From: Archie Pusaka Hi linux-bluetooth, This series of patches manages the hardware offloading part of MSFT extension API. The full documentation can be accessed by this link: https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events Only four of the HCI commands are planned to be implemented: HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), HCI_VS_MSFT_LE_Monitor_Advertisement, HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. These are the commands which would be used for advertisement monitor feature. Only if the controller supports the MSFT extension would these commands be sent. Otherwise, software-based monitoring would be performed in the user space instead. Thanks in advance for your feedback! Archie Changes in v2: * Add a new opcode instead of modifying an existing one * Also implement the new MGMT opcode and merge the functionality with the old one. Archie Pusaka (5): Bluetooth: advmon offload MSFT add rssi support Bluetooth: advmon offload MSFT add monitor Bluetooth: advmon offload MSFT remove monitor Bluetooth: advmon offload MSFT handle controller reset Bluetooth: advmon offload MSFT handle filter enablement include/net/bluetooth/hci_core.h | 34 ++- include/net/bluetooth/mgmt.h | 16 ++ net/bluetooth/hci_core.c | 173 +--- net/bluetooth/mgmt.c | 333 -- net/bluetooth/msft.c | 456 ++- net/bluetooth/msft.h | 27 ++ 6 files changed, 919 insertions(+), 120 deletions(-) -- 2.29.2.684.gfbc64c5ab5-goog
Re: [PATCH v1 1/5] Bluetooth: advmon offload MSFT add rssi support
Hi Marcel,
On Mon, 7 Dec 2020 at 17:57, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>>>> MSFT needs rssi parameter for monitoring advertisement packet,
> >>>>> therefore we should supply them from mgmt.
> >>>>>
> >>>>> Signed-off-by: Archie Pusaka
> >>>>> Reviewed-by: Miao-chen Chou
> >>>>> Reviewed-by: Yun-Hao Chung
> >>>>
> >>>> I don’t need any Reviewed-by if they are not catching an obvious user
> >>>> API breakage.
> >>>>
> >>>>> ---
> >>>>>
> >>>>> include/net/bluetooth/hci_core.h | 9 +
> >>>>> include/net/bluetooth/mgmt.h | 9 +
> >>>>> net/bluetooth/mgmt.c | 8
> >>>>> 3 files changed, 26 insertions(+)
> >>>>>
> >>>>> diff --git a/include/net/bluetooth/hci_core.h
> >>>>> b/include/net/bluetooth/hci_core.h
> >>>>> index 9873e1c8cd16..42d446417817 100644
> >>>>> --- a/include/net/bluetooth/hci_core.h
> >>>>> +++ b/include/net/bluetooth/hci_core.h
> >>>>> @@ -246,8 +246,17 @@ struct adv_pattern {
> >>>>> __u8 value[HCI_MAX_AD_LENGTH];
> >>>>> };
> >>>>>
> >>>>> +struct adv_rssi_thresholds {
> >>>>> + __s8 low_threshold;
> >>>>> + __s8 high_threshold;
> >>>>> + __u16 low_threshold_timeout;
> >>>>> + __u16 high_threshold_timeout;
> >>>>> + __u8 sampling_period;
> >>>>> +};
> >>>>> +
> >>>>> struct adv_monitor {
> >>>>> struct list_head patterns;
> >>>>> + struct adv_rssi_thresholds rssi;
> >>>>> boolactive;
> >>>>> __u16 handle;
> >>>>> };
> >>>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> >>>>> index d8367850e8cd..dc534837be0e 100644
> >>>>> --- a/include/net/bluetooth/mgmt.h
> >>>>> +++ b/include/net/bluetooth/mgmt.h
> >>>>> @@ -763,9 +763,18 @@ struct mgmt_adv_pattern {
> >>>>> __u8 value[31];
> >>>>> } __packed;
> >>>>>
> >>>>> +struct mgmt_adv_rssi_thresholds {
> >>>>> + __s8 high_threshold;
> >>>>> + __le16 high_threshold_timeout;
> >>>>> + __s8 low_threshold;
> >>>>> + __le16 low_threshold_timeout;
> >>>>> + __u8 sampling_period;
> >>>>> +} __packed;
> >>>>> +
> >>>>> #define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052
> >>>>> struct mgmt_cp_add_adv_patterns_monitor {
> >>>>> __u8 pattern_count;
> >>>>> + struct mgmt_adv_rssi_thresholds rssi;
> >>>>> struct mgmt_adv_pattern patterns[];
> >>>>> } __packed;
> >>>>
> >>>> This is something we can not do. It breaks an userspace facing API. Is
> >>>> the mgmt opcode 0x0052 in an already released kernel?
> >>>
> >>> Yes, the opcode does exist in an already released kernel.
> >>>
> >>> The DBus method which accesses this API is put behind the experimental
> >>> flag, therefore we expect they are flexible enough to support changes.
> >>> Previously, we already had a discussion in an email thread with the
> >>> title "Offload RSSI tracking to controller", and the outcome supports
> >>> this change.
> >>>
> >>> Here is an excerpt of the discussion.
> >>
> >> it doesn’t matter. This is fixed API now and so we can not just change it.
> >> The argument above is void. What matters if it is in already released
> >> kernel.
> >
> > If that is the case, do you have a suggestion to allow RSSI to be
> > considered when monitoring advertisement? Would a new MGMT opcode with
> > these parameters suffice?
>
> its the only way.
I will make the necessary changes. Thanks for the confirmation.
Regards,
Archie
>
> Regards
>
> Marcel
>
Re: [PATCH v1 1/5] Bluetooth: advmon offload MSFT add rssi support
Hi Marcel,
On Fri, 4 Dec 2020 at 17:51, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>> MSFT needs rssi parameter for monitoring advertisement packet,
> >>> therefore we should supply them from mgmt.
> >>>
> >>> Signed-off-by: Archie Pusaka
> >>> Reviewed-by: Miao-chen Chou
> >>> Reviewed-by: Yun-Hao Chung
> >>
> >> I don’t need any Reviewed-by if they are not catching an obvious user API
> >> breakage.
> >>
> >>> ---
> >>>
> >>> include/net/bluetooth/hci_core.h | 9 +
> >>> include/net/bluetooth/mgmt.h | 9 +
> >>> net/bluetooth/mgmt.c | 8
> >>> 3 files changed, 26 insertions(+)
> >>>
> >>> diff --git a/include/net/bluetooth/hci_core.h
> >>> b/include/net/bluetooth/hci_core.h
> >>> index 9873e1c8cd16..42d446417817 100644
> >>> --- a/include/net/bluetooth/hci_core.h
> >>> +++ b/include/net/bluetooth/hci_core.h
> >>> @@ -246,8 +246,17 @@ struct adv_pattern {
> >>> __u8 value[HCI_MAX_AD_LENGTH];
> >>> };
> >>>
> >>> +struct adv_rssi_thresholds {
> >>> + __s8 low_threshold;
> >>> + __s8 high_threshold;
> >>> + __u16 low_threshold_timeout;
> >>> + __u16 high_threshold_timeout;
> >>> + __u8 sampling_period;
> >>> +};
> >>> +
> >>> struct adv_monitor {
> >>> struct list_head patterns;
> >>> + struct adv_rssi_thresholds rssi;
> >>> boolactive;
> >>> __u16 handle;
> >>> };
> >>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> >>> index d8367850e8cd..dc534837be0e 100644
> >>> --- a/include/net/bluetooth/mgmt.h
> >>> +++ b/include/net/bluetooth/mgmt.h
> >>> @@ -763,9 +763,18 @@ struct mgmt_adv_pattern {
> >>> __u8 value[31];
> >>> } __packed;
> >>>
> >>> +struct mgmt_adv_rssi_thresholds {
> >>> + __s8 high_threshold;
> >>> + __le16 high_threshold_timeout;
> >>> + __s8 low_threshold;
> >>> + __le16 low_threshold_timeout;
> >>> + __u8 sampling_period;
> >>> +} __packed;
> >>> +
> >>> #define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052
> >>> struct mgmt_cp_add_adv_patterns_monitor {
> >>> __u8 pattern_count;
> >>> + struct mgmt_adv_rssi_thresholds rssi;
> >>> struct mgmt_adv_pattern patterns[];
> >>> } __packed;
> >>
> >> This is something we can not do. It breaks an userspace facing API. Is the
> >> mgmt opcode 0x0052 in an already released kernel?
> >
> > Yes, the opcode does exist in an already released kernel.
> >
> > The DBus method which accesses this API is put behind the experimental
> > flag, therefore we expect they are flexible enough to support changes.
> > Previously, we already had a discussion in an email thread with the
> > title "Offload RSSI tracking to controller", and the outcome supports
> > this change.
> >
> > Here is an excerpt of the discussion.
>
> it doesn’t matter. This is fixed API now and so we can not just change it.
> The argument above is void. What matters if it is in already released kernel.
If that is the case, do you have a suggestion to allow RSSI to be
considered when monitoring advertisement? Would a new MGMT opcode with
these parameters suffice?
#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x005?
struct mgmt_cp_add_adv_patterns_monitor {
__u8 pattern_count;
struct mgmt_adv_rssi_thresholds rssi;
struct mgmt_adv_pattern patterns[];
} __packed;
Thanks,
Archie
Re: [PATCH v1 1/5] Bluetooth: advmon offload MSFT add rssi support
Hi Marcel,
On Thu, 3 Dec 2020 at 22:03, Marcel Holtmann wrote:
>
> Hi Archie,
>
> > MSFT needs rssi parameter for monitoring advertisement packet,
> > therefore we should supply them from mgmt.
> >
> > Signed-off-by: Archie Pusaka
> > Reviewed-by: Miao-chen Chou
> > Reviewed-by: Yun-Hao Chung
>
> I don’t need any Reviewed-by if they are not catching an obvious user API
> breakage.
>
> > ---
> >
> > include/net/bluetooth/hci_core.h | 9 +
> > include/net/bluetooth/mgmt.h | 9 +
> > net/bluetooth/mgmt.c | 8
> > 3 files changed, 26 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h
> > b/include/net/bluetooth/hci_core.h
> > index 9873e1c8cd16..42d446417817 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -246,8 +246,17 @@ struct adv_pattern {
> > __u8 value[HCI_MAX_AD_LENGTH];
> > };
> >
> > +struct adv_rssi_thresholds {
> > + __s8 low_threshold;
> > + __s8 high_threshold;
> > + __u16 low_threshold_timeout;
> > + __u16 high_threshold_timeout;
> > + __u8 sampling_period;
> > +};
> > +
> > struct adv_monitor {
> > struct list_head patterns;
> > + struct adv_rssi_thresholds rssi;
> > boolactive;
> > __u16 handle;
> > };
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index d8367850e8cd..dc534837be0e 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -763,9 +763,18 @@ struct mgmt_adv_pattern {
> > __u8 value[31];
> > } __packed;
> >
> > +struct mgmt_adv_rssi_thresholds {
> > + __s8 high_threshold;
> > + __le16 high_threshold_timeout;
> > + __s8 low_threshold;
> > + __le16 low_threshold_timeout;
> > + __u8 sampling_period;
> > +} __packed;
> > +
> > #define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052
> > struct mgmt_cp_add_adv_patterns_monitor {
> > __u8 pattern_count;
> > + struct mgmt_adv_rssi_thresholds rssi;
> > struct mgmt_adv_pattern patterns[];
> > } __packed;
>
> This is something we can not do. It breaks an userspace facing API. Is the
> mgmt opcode 0x0052 in an already released kernel?
Yes, the opcode does exist in an already released kernel.
The DBus method which accesses this API is put behind the experimental
flag, therefore we expect they are flexible enough to support changes.
Previously, we already had a discussion in an email thread with the
title "Offload RSSI tracking to controller", and the outcome supports
this change.
Here is an excerpt of the discussion.
On Thu, 1 Oct 2020 at 05:58, Miao-chen Chou wrote:
>
> Hi Luiz,
>
> Yes, the RSSI is included as a part of the Adv monitor API, and the RSSI
> tracking is currently implemented (the patch series is still under review) in
> bluetoothd and used by bluetoothctl (submenu advmon). As mentioned, we are
> planning to offload the RSSI tracking to the controller as well, so there
> will be changes to the corresponding MGMT commands.
> Thanks for your quick feedback!
>
> Regards,
> Miao
>
> On Wed, Sep 30, 2020 at 2:00 PM Von Dentz, Luiz
> wrote:
>>
>> Hi Miao,
>>
>> I do recall seeing these at D-Bus level, or perhaps it was in use by
>> bluetoothctl commands? Anyway since these are still experimental it should
>> be fine to change them.
>>
>> From: Miao-chen Chou
>> Sent: Wednesday, September 30, 2020 12:51 PM
>> To: Holtmann, Marcel ; Von Dentz, Luiz
>>
>> Cc: Alain Michaud ; Yun-hao Chung
>> ; Manish Mandlik ; Archie
>> Pusaka
>> Subject: Offload RSSI tracking to controller.
>>
>> Hi Luiz and Marcel,
>>
>> Going forward to 2020 Q4, we will be working on offloading the content
>> filtering to the controllers based on controll's support of MSFT HCI
>> extension. In the meantime, we are planning to change the existing MGMT
>> commands of Adv monitoring to allow the offloading of RSSI tracking shortly.
>> Here is a snippet of potential changes.
>>
>> +struct mgmt_adv_rssi_thresholds {
>> + __s8 high_rssi_threshold;
>> + u16 high_rssi_threshold_timeout;
>> + __s8 low_rssi_threshold;
>> + u16 high_rssi_threshold_timeout;
>> +} __packed;
>>
>> struct mgmt_cp_add_adv_patterns_monitor {
>> u8 pattern_count;
>> +struct mgmt_adv_rssi_thresholds rssi_thresholds;
>> struct mgmt_adv_pattern patterns[];
>> } __packed;
>>
>> Note that as suggested by you, the D-Bus Adv monitor API which accesses
>> these MGMT commands is currently hidden behind the experimental flag, so
>> they are still mutable. We'd like to hear your early feedback on changing
>> the corresponding MGMT commands.
>>
>> Thanks,
>> Miao
Thanks,
Archie
[PATCH v1 5/5] Bluetooth: advmon offload MSFT handle filter enablement
From: Archie Pusaka
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
net/bluetooth/msft.c | 67
net/bluetooth/msft.h | 6
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 7e33a85c3f1c..055cc5a260df 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -85,6 +96,7 @@ struct msft_data {
struct {
u8 reregistering:1;
+ u8 filter_enabled:1;
} flags;
};
@@ -193,6 +205,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->flags.reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -398,6 +411,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct
hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+* already set to whatever we were trying to set.
+* Although the default state should be disabled, some controller set
+* the initial value to enabled. Because there is no way to know the
+* actual initial value before sending this command, here we also treat
+* error 0x0C as success.
+*/
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->flags.filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = >rssi;
@@ -534,3 +581,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(, hdev);
+ hci_req_add(, hdev->msft_opcode, sizeof(cp), );
+ err = hci_req_run_skb(, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 9f9a11f90b0c..44bee705c16d 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor
*monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline bool msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.454.gaff20da3a2-goog
[PATCH v1 2/5] Bluetooth: advmon offload MSFT add monitor
From: Archie Pusaka
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka
Reviewed-by: Manish Mandlik
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 54 +++--
net/bluetooth/mgmt.c | 150 +++
net/bluetooth/msft.c | 201 ++-
net/bluetooth/msft.h | 12 ++
5 files changed, 369 insertions(+), 65 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 42d446417817..275061a1b670 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -257,13 +257,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- boolactive;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1305,9 +1312,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev
*hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1783,6 +1793,7 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index c4aa2cbb9269..7f70812dde15 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3031,27 +3031,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(>adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE:
+ hci_update_background_scan(hdev);
+ BT_DBG("%s add monitor status %d", hdev->name, *err);
+ /* Message was not forw
[PATCH v1 4/5] Bluetooth: advmon offload MSFT handle controller reset
From: Archie Pusaka
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
net/bluetooth/msft.c | 79 +---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..7e33a85c3f1c 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,15 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+
+ struct {
+ u8 reregistering:1;
+ } flags;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +141,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(>adv_monitors_idr, );
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->flags.reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +190,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(>handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->flags.reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +211,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, >handle_map, list) {
+ monitor = idr_find(>adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(_data->list);
kfree(handle_data);
}
@@ -282,9 +330,15 @@ static void msft_le_monitor_advertisement_cb(struct
hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->flags.reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->flags.reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +428,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor
*monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +442,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +486,20 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct
adv_monitor *monitor)
[PATCH v1 3/5] Bluetooth: advmon offload MSFT remove monitor
From: Archie Pusaka
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++--
net/bluetooth/mgmt.c | 109 +-
net/bluetooth/msft.c | 127 ++-
net/bluetooth/msft.h | 9 +++
5 files changed, 322 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 275061a1b670..0a211cd52fdb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1311,11 +1311,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8
instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1792,8 +1794,10 @@ void mgmt_advertising_added(struct sock *sk, struct
hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7f70812dde15..70bb2a11d9b1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3012,12 +3012,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(>adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(>adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3025,8 +3028,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, >patterns, list)
+ list_for_each_entry_safe(pattern, tmp, >patterns, list) {
+ list_del(>list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(>adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3036,6 +3049,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev
*hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3082,39 +3100,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct
adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+static bool hci_remove_adv_monitor(struct hci_dev *hdev,
+
[PATCH v1 1/5] Bluetooth: advmon offload MSFT add rssi support
From: Archie Pusaka
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt.
Signed-off-by: Archie Pusaka
Reviewed-by: Miao-chen Chou
Reviewed-by: Yun-Hao Chung
---
include/net/bluetooth/hci_core.h | 9 +
include/net/bluetooth/mgmt.h | 9 +
net/bluetooth/mgmt.c | 8
3 files changed, 26 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9873e1c8cd16..42d446417817 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -246,8 +246,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
boolactive;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index d8367850e8cd..dc534837be0e 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -763,9 +763,18 @@ struct mgmt_adv_pattern {
__u8 value[31];
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8 high_threshold;
+ __le16 high_threshold_timeout;
+ __s8 low_threshold;
+ __le16 low_threshold_timeout;
+ __u8 sampling_period;
+} __packed;
+
#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052
struct mgmt_cp_add_adv_patterns_monitor {
__u8 pattern_count;
+ struct mgmt_adv_rssi_thresholds rssi;
struct mgmt_adv_pattern patterns[];
} __packed;
#define MGMT_ADD_ADV_PATTERNS_MONITOR_SIZE 1
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3dfed4efa078..5f0f03dcd81d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4237,6 +4237,14 @@ static int add_adv_patterns_monitor(struct sock *sk,
struct hci_dev *hdev,
INIT_LIST_HEAD(>patterns);
m->active = false;
+ m->rssi.low_threshold = cp->rssi.low_threshold;
+ m->rssi.low_threshold_timeout =
+ __le16_to_cpu(cp->rssi.low_threshold_timeout);
+ m->rssi.high_threshold = cp->rssi.high_threshold;
+ m->rssi.high_threshold_timeout =
+ __le16_to_cpu(cp->rssi.high_threshold_timeout);
+ m->rssi.sampling_period = cp->rssi.sampling_period;
+
for (i = 0; i < cp->pattern_count; i++) {
if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
err = mgmt_cmd_status(sk, hdev->id,
--
2.29.2.454.gaff20da3a2-goog
[PATCH v1 0/5] MSFT offloading support for advertisement monitor
From: Archie Pusaka Hi linux-bluetooth, This series of patches manages the hardware offloading part of MSFT extension API. The full documentation can be accessed by this link: https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events Only four of the HCI commands are planned to be implemented: HCI_VS_MSFT_Read_Supported_Features (implemented in previous patch), HCI_VS_MSFT_LE_Monitor_Advertisement, HCI_VS_MSFT_LE_Cancel_Monitor_Advertisement, and HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable. These are the commands which would be used for advertisement monitor feature. Only if the controller supports the MSFT extension would these commands be sent. Otherwise, software-based monitoring would be performed in the user space instead. Thanks in advance for your feedback! Archie Archie Pusaka (5): Bluetooth: advmon offload MSFT add rssi support Bluetooth: advmon offload MSFT add monitor Bluetooth: advmon offload MSFT remove monitor Bluetooth: advmon offload MSFT handle controller reset Bluetooth: advmon offload MSFT handle filter enablement include/net/bluetooth/hci_core.h | 34 ++- include/net/bluetooth/mgmt.h | 9 + net/bluetooth/hci_core.c | 173 +--- net/bluetooth/mgmt.c | 263 +- net/bluetooth/msft.c | 456 ++- net/bluetooth/msft.h | 27 ++ 6 files changed, 853 insertions(+), 109 deletions(-) -- 2.29.2.454.gaff20da3a2-goog
[PATCH v2] Bluetooth: Enforce key size of 16 bytes on FIPS level
From: Archie Pusaka According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: Device in security mode 4 level 4 shall enforce: 128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened) This patch rejects connection with key size below 16 for FIPS level services. Signed-off-by: Archie Pusaka Reviewed-by: Alain Michaud --- Sorry for the long delay. This patch fell out of my radar. Changes in v2: * Add comment on enforcing 16 bytes key size net/bluetooth/l2cap_core.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 1ab27b90ddcb..5817f5c2ec18 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1515,8 +1515,14 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) * that have no key size requirements. Ensure that the link is * actually encrypted before enforcing a key size. */ + int min_key_size = hcon->hdev->min_enc_key_size; + + /* On FIPS security level, key size must be 16 bytes */ + if (hcon->sec_level == BT_SECURITY_FIPS) + min_key_size = 16; + return (!test_bit(HCI_CONN_ENCRYPT, >flags) || - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); + hcon->enc_key_size >= min_key_size); } static void l2cap_do_start(struct l2cap_chan *chan) -- 2.29.2.299.gdc1121823c-goog
Re: [PATCH v3] Bluetooth: Check for encryption key size on connect
[Re-sending in plain text]
Hi Marcel,
I tried Alex's patch and it works for me as well.
Thanks,
Archie
On Thu, 1 Oct 2020 at 15:14, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>>>>>>>> When receiving connection, we only check whether the link has been
> >>>>>>>>> encrypted, but not the encryption key size of the link.
> >>>>>>>>>
> >>>>>>>>> This patch adds check for encryption key size, and reject L2CAP
> >>>>>>>>> connection which size is below the specified threshold (default 7)
> >>>>>>>>> with security block.
> >>>>>>>>>
> >>>>>>>>> Here is some btmon trace.
> >>>>>>>>> @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0]
> >>>>>>>>> 5.847722
> >>>>>>>>>Store hint: No (0x00)
> >>>>>>>>>BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> >>>>>>>>>Key type: Unauthenticated Combination key from P-192 (0x04)
> >>>>>>>>>Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> >>>>>>>>>PIN length: 0
> >>>>>>>>>> HCI Event: Encryption Change (0x08) plen 4#29 [hci0]
> >>>>>>>>>> 5.871537
> >>>>>>>>>Status: Success (0x00)
> >>>>>>>>>Handle: 256
> >>>>>>>>>Encryption: Enabled with E0 (0x01)
> >>>>>>>>> < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0]
> >>>>>>>>> 5.871609
> >>>>>>>>>Handle: 256
> >>>>>>>>>> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0]
> >>>>>>>>>> 5.872524
> >>>>>>>>> Read Encryption Key Size (0x05|0x0008) ncmd 1
> >>>>>>>>>Status: Success (0x00)
> >>>>>>>>>Handle: 256
> >>>>>>>>>Key size: 3
> >>>>>>>>>
> >>>>>>>>> // WITHOUT PATCH //
> >>>>>>>>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0]
> >>>>>>>>>> 5.895023
> >>>>>>>>> L2CAP: Connection Request (0x02) ident 3 len 4
> >>>>>>>>>PSM: 4097 (0x1001)
> >>>>>>>>>Source CID: 64
> >>>>>>>>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0]
> >>>>>>>>> 5.895213
> >>>>>>>>> L2CAP: Connection Response (0x03) ident 3 len 8
> >>>>>>>>>Destination CID: 64
> >>>>>>>>>Source CID: 64
> >>>>>>>>>Result: Connection successful (0x)
> >>>>>>>>>Status: No further information available (0x)
> >>>>>>>>>
> >>>>>>>>> // WITH PATCH //
> >>>>>>>>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0]
> >>>>>>>>>> 4.887024
> >>>>>>>>> L2CAP: Connection Request (0x02) ident 3 len 4
> >>>>>>>>>PSM: 4097 (0x1001)
> >>>>>>>>>Source CID: 64
> >>>>>>>>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0]
> >>>>>>>>> 4.887127
> >>>>>>>>> L2CAP: Connection Response (0x03) ident 3 len 8
> >>>>>>>>>Destination CID: 0
> >>>>>>>>>Source CID: 64
> >>>>>>>>>Result: Connection refused - security block (0x0003)
> >>>>>>>>>Status: No further information available (0x)
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Archie Pusaka
> >>>>>>>>>
> >>>>>>>>> ---
> >>>>>>>>>
> >>>>>>>>> Changes in v3:
> >>>>>>>>> * Move the check to hci_conn_check_link_mode()
> >>>>>>>>>
> >>>>>>>>> Cha
Re: [PATCH v3] Bluetooth: Check for encryption key size on connect
Hi Marcel,
On Tue, 29 Sep 2020 at 14:21, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>>>>>> When receiving connection, we only check whether the link has been
> >>>>>>> encrypted, but not the encryption key size of the link.
> >>>>>>>
> >>>>>>> This patch adds check for encryption key size, and reject L2CAP
> >>>>>>> connection which size is below the specified threshold (default 7)
> >>>>>>> with security block.
> >>>>>>>
> >>>>>>> Here is some btmon trace.
> >>>>>>> @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0]
> >>>>>>> 5.847722
> >>>>>>> Store hint: No (0x00)
> >>>>>>> BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> >>>>>>> Key type: Unauthenticated Combination key from P-192 (0x04)
> >>>>>>> Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> >>>>>>> PIN length: 0
> >>>>>>>> HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
> >>>>>>> Status: Success (0x00)
> >>>>>>> Handle: 256
> >>>>>>> Encryption: Enabled with E0 (0x01)
> >>>>>>> < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0]
> >>>>>>> 5.871609
> >>>>>>> Handle: 256
> >>>>>>>> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
> >>>>>>> Read Encryption Key Size (0x05|0x0008) ncmd 1
> >>>>>>> Status: Success (0x00)
> >>>>>>> Handle: 256
> >>>>>>> Key size: 3
> >>>>>>>
> >>>>>>> // WITHOUT PATCH //
> >>>>>>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
> >>>>>>> L2CAP: Connection Request (0x02) ident 3 len 4
> >>>>>>> PSM: 4097 (0x1001)
> >>>>>>> Source CID: 64
> >>>>>>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0]
> >>>>>>> 5.895213
> >>>>>>> L2CAP: Connection Response (0x03) ident 3 len 8
> >>>>>>> Destination CID: 64
> >>>>>>> Source CID: 64
> >>>>>>> Result: Connection successful (0x)
> >>>>>>> Status: No further information available (0x)
> >>>>>>>
> >>>>>>> // WITH PATCH //
> >>>>>>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
> >>>>>>> L2CAP: Connection Request (0x02) ident 3 len 4
> >>>>>>> PSM: 4097 (0x1001)
> >>>>>>> Source CID: 64
> >>>>>>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0]
> >>>>>>> 4.887127
> >>>>>>> L2CAP: Connection Response (0x03) ident 3 len 8
> >>>>>>> Destination CID: 0
> >>>>>>> Source CID: 64
> >>>>>>> Result: Connection refused - security block (0x0003)
> >>>>>>> Status: No further information available (0x)
> >>>>>>>
> >>>>>>> Signed-off-by: Archie Pusaka
> >>>>>>>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> Changes in v3:
> >>>>>>> * Move the check to hci_conn_check_link_mode()
> >>>>>>>
> >>>>>>> Changes in v2:
> >>>>>>> * Add btmon trace to the commit message
> >>>>>>>
> >>>>>>> net/bluetooth/hci_conn.c | 4
> >>>>>>> 1 file changed, 4 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >>>>>>> index 9832f8445d43..89085fac797c 100644
> >>>>>>> --- a/net/bluetooth/hci_conn.c
> >>>>>>> +++ b/net/bluetooth/hci_conn.c
> >>>>>>> @@ -1348,6 +1348,10 @@ int hci_conn_check_link_mode(struct hci_conn
> >>>>>>> *conn)
> >>>>&
Re: [PATCH v3] Bluetooth: Check for encryption key size on connect
Hi Marcel,
On Tue, 29 Sep 2020 at 00:43, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>>>> When receiving connection, we only check whether the link has been
> >>>>> encrypted, but not the encryption key size of the link.
> >>>>>
> >>>>> This patch adds check for encryption key size, and reject L2CAP
> >>>>> connection which size is below the specified threshold (default 7)
> >>>>> with security block.
> >>>>>
> >>>>> Here is some btmon trace.
> >>>>> @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
> >>>>> Store hint: No (0x00)
> >>>>> BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> >>>>> Key type: Unauthenticated Combination key from P-192 (0x04)
> >>>>> Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> >>>>> PIN length: 0
> >>>>>> HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
> >>>>> Status: Success (0x00)
> >>>>> Handle: 256
> >>>>> Encryption: Enabled with E0 (0x01)
> >>>>> < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
> >>>>> Handle: 256
> >>>>>> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
> >>>>>Read Encryption Key Size (0x05|0x0008) ncmd 1
> >>>>> Status: Success (0x00)
> >>>>> Handle: 256
> >>>>> Key size: 3
> >>>>>
> >>>>> // WITHOUT PATCH //
> >>>>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
> >>>>>L2CAP: Connection Request (0x02) ident 3 len 4
> >>>>> PSM: 4097 (0x1001)
> >>>>> Source CID: 64
> >>>>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
> >>>>>L2CAP: Connection Response (0x03) ident 3 len 8
> >>>>> Destination CID: 64
> >>>>> Source CID: 64
> >>>>> Result: Connection successful (0x0000)
> >>>>> Status: No further information available (0x)
> >>>>>
> >>>>> // WITH PATCH //
> >>>>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
> >>>>>L2CAP: Connection Request (0x02) ident 3 len 4
> >>>>> PSM: 4097 (0x1001)
> >>>>> Source CID: 64
> >>>>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 4.887127
> >>>>>L2CAP: Connection Response (0x03) ident 3 len 8
> >>>>> Destination CID: 0
> >>>>> Source CID: 64
> >>>>> Result: Connection refused - security block (0x0003)
> >>>>> Status: No further information available (0x)
> >>>>>
> >>>>> Signed-off-by: Archie Pusaka
> >>>>>
> >>>>> ---
> >>>>>
> >>>>> Changes in v3:
> >>>>> * Move the check to hci_conn_check_link_mode()
> >>>>>
> >>>>> Changes in v2:
> >>>>> * Add btmon trace to the commit message
> >>>>>
> >>>>> net/bluetooth/hci_conn.c | 4
> >>>>> 1 file changed, 4 insertions(+)
> >>>>>
> >>>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >>>>> index 9832f8445d43..89085fac797c 100644
> >>>>> --- a/net/bluetooth/hci_conn.c
> >>>>> +++ b/net/bluetooth/hci_conn.c
> >>>>> @@ -1348,6 +1348,10 @@ int hci_conn_check_link_mode(struct hci_conn
> >>>>> *conn)
> >>>>> !test_bit(HCI_CONN_ENCRYPT, >flags))
> >>>>> return 0;
> >>>>>
> >>>>> + if (test_bit(HCI_CONN_ENCRYPT, >flags) &&
> >>>>> + conn->enc_key_size < conn->hdev->min_enc_key_size)
> >>>>> + return 0;
> >>>>> +
> >>>>> return 1;
> >>>>> }
> >>>>
> >>>> I am a bit concerned since we had that check and I on purpose moved it.
> >>>> See commit 693cd8ce3f88 for the change where I removed and commit
>
Re: [PATCH v3] Bluetooth: Check for encryption key size on connect
Hi Marcel,
On Sun, 27 Sep 2020 at 20:09, Marcel Holtmann wrote:
>
> Hi Archie,
>
> >>> When receiving connection, we only check whether the link has been
> >>> encrypted, but not the encryption key size of the link.
> >>>
> >>> This patch adds check for encryption key size, and reject L2CAP
> >>> connection which size is below the specified threshold (default 7)
> >>> with security block.
> >>>
> >>> Here is some btmon trace.
> >>> @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
> >>> Store hint: No (0x00)
> >>> BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> >>> Key type: Unauthenticated Combination key from P-192 (0x04)
> >>> Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> >>> PIN length: 0
> >>>> HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
> >>> Status: Success (0x00)
> >>> Handle: 256
> >>> Encryption: Enabled with E0 (0x01)
> >>> < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
> >>> Handle: 256
> >>>> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
> >>> Read Encryption Key Size (0x05|0x0008) ncmd 1
> >>> Status: Success (0x00)
> >>> Handle: 256
> >>> Key size: 3
> >>>
> >>> // WITHOUT PATCH //
> >>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
> >>> L2CAP: Connection Request (0x02) ident 3 len 4
> >>> PSM: 4097 (0x1001)
> >>> Source CID: 64
> >>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
> >>> L2CAP: Connection Response (0x03) ident 3 len 8
> >>> Destination CID: 64
> >>> Source CID: 64
> >>> Result: Connection successful (0x)
> >>> Status: No further information available (0x)
> >>>
> >>> // WITH PATCH //
> >>>> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
> >>> L2CAP: Connection Request (0x02) ident 3 len 4
> >>> PSM: 4097 (0x1001)
> >>> Source CID: 64
> >>> < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 4.887127
> >>> L2CAP: Connection Response (0x03) ident 3 len 8
> >>> Destination CID: 0
> >>> Source CID: 64
> >>> Result: Connection refused - security block (0x0003)
> >>> Status: No further information available (0x)
> >>>
> >>> Signed-off-by: Archie Pusaka
> >>>
> >>> ---
> >>>
> >>> Changes in v3:
> >>> * Move the check to hci_conn_check_link_mode()
> >>>
> >>> Changes in v2:
> >>> * Add btmon trace to the commit message
> >>>
> >>> net/bluetooth/hci_conn.c | 4
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >>> index 9832f8445d43..89085fac797c 100644
> >>> --- a/net/bluetooth/hci_conn.c
> >>> +++ b/net/bluetooth/hci_conn.c
> >>> @@ -1348,6 +1348,10 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
> >>> !test_bit(HCI_CONN_ENCRYPT, >flags))
> >>> return 0;
> >>>
> >>> + if (test_bit(HCI_CONN_ENCRYPT, >flags) &&
> >>> + conn->enc_key_size < conn->hdev->min_enc_key_size)
> >>> + return 0;
> >>> +
> >>> return 1;
> >>> }
> >>
> >> I am a bit concerned since we had that check and I on purpose moved it.
> >> See commit 693cd8ce3f88 for the change where I removed and commit
> >> d5bb334a8e17 where I initially added it.
> >>
> >> Naively adding the check in that location caused a major regression with
> >> Bluetooth 2.0 devices. This makes me a bit reluctant to re-add it here
> >> since I restructured the whole change to check the key size a different
> >> location.
> >
> > I have tried this patch (both v2 and v3) to connect with a Bluetooth
> > 2.0 device, it doesn't have any connection problem.
> > I suppose because in the original patch (d5bb334a8e17), there is no
> > check for the HCI_CONN_ENCRYPT flag.
>
Re: [PATCH v3] Bluetooth: Check for encryption key size on connect
Hi Marcel,
On Sat, 26 Sep 2020 at 00:37, Marcel Holtmann wrote:
>
> Hi Archie,
>
> > When receiving connection, we only check whether the link has been
> > encrypted, but not the encryption key size of the link.
> >
> > This patch adds check for encryption key size, and reject L2CAP
> > connection which size is below the specified threshold (default 7)
> > with security block.
> >
> > Here is some btmon trace.
> > @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
> >Store hint: No (0x00)
> >BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> >Key type: Unauthenticated Combination key from P-192 (0x04)
> >Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> >PIN length: 0
> >> HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
> >Status: Success (0x00)
> >Handle: 256
> >Encryption: Enabled with E0 (0x01)
> > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
> >Handle: 256
> >> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
> > Read Encryption Key Size (0x05|0x0008) ncmd 1
> >Status: Success (0x00)
> >Handle: 256
> >Key size: 3
> >
> > // WITHOUT PATCH //
> >> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
> > L2CAP: Connection Request (0x02) ident 3 len 4
> >PSM: 4097 (0x1001)
> >Source CID: 64
> > < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
> > L2CAP: Connection Response (0x03) ident 3 len 8
> >Destination CID: 64
> >Source CID: 64
> >Result: Connection successful (0x)
> >Status: No further information available (0x)
> >
> > // WITH PATCH //
> >> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
> > L2CAP: Connection Request (0x02) ident 3 len 4
> >PSM: 4097 (0x1001)
> >Source CID: 64
> > < ACL Data TX: Handle 256 flags 0x00 dlen 16 #43 [hci0] 4.887127
> > L2CAP: Connection Response (0x03) ident 3 len 8
> >Destination CID: 0
> >Source CID: 64
> >Result: Connection refused - security block (0x0003)
> >Status: No further information available (0x)
> >
> > Signed-off-by: Archie Pusaka
> >
> > ---
> >
> > Changes in v3:
> > * Move the check to hci_conn_check_link_mode()
> >
> > Changes in v2:
> > * Add btmon trace to the commit message
> >
> > net/bluetooth/hci_conn.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 9832f8445d43..89085fac797c 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -1348,6 +1348,10 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
> > !test_bit(HCI_CONN_ENCRYPT, >flags))
> > return 0;
> >
> > + if (test_bit(HCI_CONN_ENCRYPT, >flags) &&
> > + conn->enc_key_size < conn->hdev->min_enc_key_size)
> > + return 0;
> > +
> > return 1;
> > }
>
> I am a bit concerned since we had that check and I on purpose moved it. See
> commit 693cd8ce3f88 for the change where I removed and commit d5bb334a8e17
> where I initially added it.
>
> Naively adding the check in that location caused a major regression with
> Bluetooth 2.0 devices. This makes me a bit reluctant to re-add it here since
> I restructured the whole change to check the key size a different location.
I have tried this patch (both v2 and v3) to connect with a Bluetooth
2.0 device, it doesn't have any connection problem.
I suppose because in the original patch (d5bb334a8e17), there is no
check for the HCI_CONN_ENCRYPT flag.
>
> Now I have to ask, are you running an upstream kernel with both commits above
> that address KNOB vulnerability?
Actually no, I haven't heard of KNOB vulnerability before.
This patch is written for qualification purposes, specifically to pass
GAP/SEC/SEM/BI-05-C to BI-08-C.
However, it sounds like it could also prevent some KNOB vulnerability
as a bonus.
>
> Regards
>
> Marcel
>
Thanks,
Archie
[PATCH v2] Bluetooth: send proper config param to unknown config request
From: Archie Pusaka
When receiving an L2CAP_CONFIGURATION_REQ with an unknown config
type, currently we will reply with L2CAP_CONFIGURATION_RSP with
a list of unknown types as the config param. However, this is not
a correct format of config param.
As described in the bluetooth spec v5.2, Vol 3, Part A, Sec 5,
the config param should consists of type, length, and optionally
data.
This patch copies the length and data from the received
L2CAP_CONFIGURATION_REQ and also appends them to the config param
of the corresponding L2CAP_CONFIGURATION_RSP to match the format
of the config param according to the spec.
Here's some btmon traces.
//--- Without Patch ---//
> ACL Data RX: Handle 256 flags 0x02 dlen 24 #58 [hci0] 21.570741
L2CAP: Configure Request (0x04) ident 5 len 16
Destination CID: 64
Flags: 0x
Option: Unknown (0x10) [mandatory]
10 00 11 02 11 00 12 02 12 00..
< ACL Data TX: Handle 256 flags 0x00 dlen 17 #59 [hci0] 21.570892
L2CAP: Configure Response (0x05) ident 5 len 9
Source CID: 64
Flags: 0x
Result: Failure - unknown options (0x0003)
Option: Unknown (0x10) [mandatory]
12
// Btmon parses it wrong - we sent 10 11 12 instead of just 12.
//--- With Patch ---//
> ACL Data RX: Handle 256 flags 0x02 dlen 24 #58 [hci0] 22.188308
L2CAP: Configure Request (0x04) ident 9 len 16
Destination CID: 64
Flags: 0x
Option: Unknown (0x10) [mandatory]
10 00 11 02 11 00 12 02 12 00..
< ACL Data TX: Handle 256 flags 0x00 dlen 26 #59 [hci0] 22.188516
L2CAP: Configure Response (0x05) ident 9 len 18
Source CID: 64
Flags: 0x
Result: Failure - unknown options (0x0003)
Option: Unknown (0x10) [mandatory]
10 00 11 02 11 00 12 02 12 00..
Signed-off-by: Archie Pusaka
Reviewed-by: Alain Michaud
---
Changes in v2:
* Add btmon traces in the commit message
net/bluetooth/l2cap_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1ab27b90ddcb..4e65854b2f1c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3627,7 +3627,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan,
void *data, size_t data
if (hint)
break;
result = L2CAP_CONF_UNKNOWN;
- *((u8 *) ptr++) = type;
+ l2cap_add_conf_opt(, type, olen, val,
+ endptr - ptr);
break;
}
}
@@ -3658,7 +3659,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan,
void *data, size_t data
}
done:
- if (chan->mode != rfc.mode) {
+ if (chan->mode != rfc.mode && result != L2CAP_CONF_UNKNOWN) {
result = L2CAP_CONF_UNACCEPT;
rfc.mode = chan->mode;
--
2.28.0.681.g6f77f65b4e-goog
Re: [PATCH v2] Bluetooth: Check for encryption key size on connect
Hi Luiz,
On Wed, 23 Sep 2020 at 01:03, Luiz Augusto von Dentz
wrote:
>
> Hi Archie,
>
> On Tue, Sep 22, 2020 at 12:48 AM Archie Pusaka wrote:
> >
> > Hi Luiz,
> >
> > On Tue, 22 Sep 2020 at 01:15, Luiz Augusto von Dentz
> > wrote:
> > >
> > > Hi Archie,
> > >
> > >
> > > On Mon, Sep 21, 2020 at 12:56 AM Archie Pusaka wrote:
> > > >
> > > > From: Archie Pusaka
> > > >
> > > > When receiving connection, we only check whether the link has been
> > > > encrypted, but not the encryption key size of the link.
> > > >
> > > > This patch adds check for encryption key size, and reject L2CAP
> > > > connection which size is below the specified threshold (default 7)
> > > > with security block.
> > > >
> > > > Here is some btmon trace.
> > > > @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
> > > > Store hint: No (0x00)
> > > > BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> > > > Key type: Unauthenticated Combination key from P-192 (0x04)
> > > > Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> > > > PIN length: 0
> > > > > HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
> > > > Status: Success (0x00)
> > > > Handle: 256
> > > > Encryption: Enabled with E0 (0x01)
> > > > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
> > > > Handle: 256
> > > > > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
> > > > Read Encryption Key Size (0x05|0x0008) ncmd 1
> > > > Status: Success (0x00)
> > > > Handle: 256
> > > > Key size: 3
> > > >
> > > > // WITHOUT PATCH //
> > > > > ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
> > > > L2CAP: Connection Request (0x02) ident 3 len 4
> > > > PSM: 4097 (0x1001)
> > > > Source CID: 64
> > > > < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
> > > > L2CAP: Connection Response (0x03) ident 3 len 8
> > > > Destination CID: 64
> > > > Source CID: 64
> > > > Result: Connection successful (0x)
> > > > Status: No further information available (0x)
> > > >
> > > > // WITH PATCH //
> > > > > ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
> > > > L2CAP: Connection Request (0x02) ident 3 len 4
> > > > PSM: 4097 (0x1001)
> > > > Source CID: 64
> > > > < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 4.887127
> > > > L2CAP: Connection Response (0x03) ident 3 len 8
> > > > Destination CID: 0
> > > > Source CID: 64
> > > > Result: Connection refused - security block (0x0003)
> > > > Status: No further information available (0x)
> > > >
> > > > Signed-off-by: Archie Pusaka
> > > > Reviewed-by: Alain Michaud
> > > >
> > > > ---
> > > > Btw, it looks like the patch sent by Alex Lu with the title
> > > > [PATCH] Bluetooth: Fix the vulnerable issue on enc key size
> > > > also solves the exact same issue.
> > > >
> > > > Changes in v2:
> > > > * Add btmon trace to the commit message
> > > >
> > > > net/bluetooth/l2cap_core.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > > > index ade83e224567..b4fc0ad38aaa 100644
> > > > --- a/net/bluetooth/l2cap_core.c
> > > > +++ b/net/bluetooth/l2cap_core.c
> > > > @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct
> > > > l2cap_conn *conn,
> > > >
> > > > /* Check if the ACL is secure enough (if not SDP) */
> > > > if (psm != cpu_to_le16(L2CAP_PSM_SDP) &&
> > > > - !hci_conn_check_link_mode(conn->hcon)) {
> > > > + (!hci_conn_check_link_mode(conn->hcon) ||
> > > > + !l2cap_check_enc_key_size(conn->hcon))) {
> > >
[PATCH v1] Bluetooth: send proper config param to unknown config request
From: Archie Pusaka
When receiving an L2CAP_CONFIGURATION_REQ with an unknown config
type, currently we will reply with L2CAP_CONFIGURATION_RSP with
a list of unknown types as the config param. However, this is not
a correct format of config param.
As described in the bluetooth spec v5.2, Vol 3, Part A, Sec 5,
the config param should consists of type, length, and optionally
data.
This patch copies the length and data from the received
L2CAP_CONFIGURATION_REQ and also appends them to the config param
of the corresponding L2CAP_CONFIGURATION_RSP to match the format
of the config param according to the spec.
Signed-off-by: Archie Pusaka
Reviewed-by: Alain Michaud
---
net/bluetooth/l2cap_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ade83e224567..2f3ddd4f0f4c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3627,7 +3627,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan,
void *data, size_t data
if (hint)
break;
result = L2CAP_CONF_UNKNOWN;
- *((u8 *) ptr++) = type;
+ l2cap_add_conf_opt(, type, olen, val,
+ endptr - ptr);
break;
}
}
@@ -3658,7 +3659,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan,
void *data, size_t data
}
done:
- if (chan->mode != rfc.mode) {
+ if (chan->mode != rfc.mode && result != L2CAP_CONF_UNKNOWN) {
result = L2CAP_CONF_UNACCEPT;
rfc.mode = chan->mode;
--
2.28.0.681.g6f77f65b4e-goog
[PATCH v3] Bluetooth: Check for encryption key size on connect
From: Archie Pusaka
When receiving connection, we only check whether the link has been
encrypted, but not the encryption key size of the link.
This patch adds check for encryption key size, and reject L2CAP
connection which size is below the specified threshold (default 7)
with security block.
Here is some btmon trace.
@ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
Store hint: No (0x00)
BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
Key type: Unauthenticated Combination key from P-192 (0x04)
Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
PIN length: 0
> HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
Status: Success (0x00)
Handle: 256
Encryption: Enabled with E0 (0x01)
< HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
Handle: 256
> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
Read Encryption Key Size (0x05|0x0008) ncmd 1
Status: Success (0x00)
Handle: 256
Key size: 3
// WITHOUT PATCH //
> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
L2CAP: Connection Request (0x02) ident 3 len 4
PSM: 4097 (0x1001)
Source CID: 64
< ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
L2CAP: Connection Response (0x03) ident 3 len 8
Destination CID: 64
Source CID: 64
Result: Connection successful (0x)
Status: No further information available (0x)
// WITH PATCH //
> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
L2CAP: Connection Request (0x02) ident 3 len 4
PSM: 4097 (0x1001)
Source CID: 64
< ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 4.887127
L2CAP: Connection Response (0x03) ident 3 len 8
Destination CID: 0
Source CID: 64
Result: Connection refused - security block (0x0003)
Status: No further information available (0x0000)
Signed-off-by: Archie Pusaka
---
Changes in v3:
* Move the check to hci_conn_check_link_mode()
Changes in v2:
* Add btmon trace to the commit message
net/bluetooth/hci_conn.c | 4
1 file changed, 4 insertions(+)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9832f8445d43..89085fac797c 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1348,6 +1348,10 @@ int hci_conn_check_link_mode(struct hci_conn *conn)
!test_bit(HCI_CONN_ENCRYPT, >flags))
return 0;
+ if (test_bit(HCI_CONN_ENCRYPT, >flags) &&
+ conn->enc_key_size < conn->hdev->min_enc_key_size)
+ return 0;
+
return 1;
}
--
2.28.0.681.g6f77f65b4e-goog
Re: [PATCH v2] Bluetooth: Check for encryption key size on connect
Hi Luiz,
On Tue, 22 Sep 2020 at 01:15, Luiz Augusto von Dentz
wrote:
>
> Hi Archie,
>
>
> On Mon, Sep 21, 2020 at 12:56 AM Archie Pusaka wrote:
> >
> > From: Archie Pusaka
> >
> > When receiving connection, we only check whether the link has been
> > encrypted, but not the encryption key size of the link.
> >
> > This patch adds check for encryption key size, and reject L2CAP
> > connection which size is below the specified threshold (default 7)
> > with security block.
> >
> > Here is some btmon trace.
> > @ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
> > Store hint: No (0x00)
> > BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
> > Key type: Unauthenticated Combination key from P-192 (0x04)
> > Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
> > PIN length: 0
> > > HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
> > Status: Success (0x00)
> > Handle: 256
> > Encryption: Enabled with E0 (0x01)
> > < HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
> > Handle: 256
> > > HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
> > Read Encryption Key Size (0x05|0x0008) ncmd 1
> > Status: Success (0x00)
> > Handle: 256
> > Key size: 3
> >
> > // WITHOUT PATCH //
> > > ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
> > L2CAP: Connection Request (0x02) ident 3 len 4
> > PSM: 4097 (0x1001)
> > Source CID: 64
> > < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
> > L2CAP: Connection Response (0x03) ident 3 len 8
> > Destination CID: 64
> > Source CID: 64
> > Result: Connection successful (0x)
> > Status: No further information available (0x)
> >
> > // WITH PATCH //
> > > ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
> > L2CAP: Connection Request (0x02) ident 3 len 4
> > PSM: 4097 (0x1001)
> > Source CID: 64
> > < ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 4.887127
> > L2CAP: Connection Response (0x03) ident 3 len 8
> > Destination CID: 0
> > Source CID: 64
> > Result: Connection refused - security block (0x0003)
> > Status: No further information available (0x)
> >
> > Signed-off-by: Archie Pusaka
> > Reviewed-by: Alain Michaud
> >
> > ---
> > Btw, it looks like the patch sent by Alex Lu with the title
> > [PATCH] Bluetooth: Fix the vulnerable issue on enc key size
> > also solves the exact same issue.
> >
> > Changes in v2:
> > * Add btmon trace to the commit message
> >
> > net/bluetooth/l2cap_core.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index ade83e224567..b4fc0ad38aaa 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct
> > l2cap_conn *conn,
> >
> > /* Check if the ACL is secure enough (if not SDP) */
> > if (psm != cpu_to_le16(L2CAP_PSM_SDP) &&
> > - !hci_conn_check_link_mode(conn->hcon)) {
> > + (!hci_conn_check_link_mode(conn->hcon) ||
> > + !l2cap_check_enc_key_size(conn->hcon))) {
>
> I wonder if we couldn't incorporate the check of key size into
> hci_conn_check_link_mode, like I said in the first patch checking the
> enc key size should not be specific to L2CAP.
Yes, I could move the check into hci_conn_check_link_mode.
At first look, this function is also called by AMP which I am not
familiar with. In addition, I found this patch which moves this check
outside hci_conn, so I have my doubts there.
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=693cd8ce3f882524a5d06f7800dd8492411877b3
>
> > conn->disc_reason = HCI_ERROR_AUTH_FAILURE;
> > result = L2CAP_CR_SEC_BLOCK;
> > goto response;
> > --
> > 2.28.0.681.g6f77f65b4e-goog
> >
>
>
> --
> Luiz Augusto von Dentz
Thanks,
Archie
Re: [PATCH v1] Bluetooth: Enforce key size of 16 bytes on FIPS level
Hi Luiz, On Tue, 22 Sep 2020 at 01:13, Luiz Augusto von Dentz wrote: > > Hi Archie, > > On Mon, Sep 21, 2020 at 1:31 AM Archie Pusaka wrote: > > > > From: Archie Pusaka > > > > According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: > > Device in security mode 4 level 4 shall enforce: > > 128-bit equivalent strength for link and encryption keys required > > using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, > > and P-192 not allowed; encryption key not shortened) > > > > This patch rejects connection with key size below 16 for FIPS level > > services. > > > > Signed-off-by: Archie Pusaka > > Reviewed-by: Alain Michaud > > > > --- > > > > net/bluetooth/l2cap_core.c | 7 ++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index ade83e224567..306616ec26e6 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn > > *hcon) > > * that have no key size requirements. Ensure that the link is > > * actually encrypted before enforcing a key size. > > */ > > + int min_key_size = hcon->hdev->min_enc_key_size; > > + > > + if (hcon->sec_level == BT_SECURITY_FIPS) > > + min_key_size = 16; > > + > > return (!test_bit(HCI_CONN_ENCRYPT, >flags) || > > - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); > > + hcon->enc_key_size >= min_key_size); > > While this looks fine to me, it looks like this should be placed > elsewhere since it takes an hci_conn and it is not L2CAP specific. >From what I understood, it is permissible to use AES-CCM P-256 encryption with key length < 16 when encrypting the link, but such a connection does not satisfy security level 4, and therefore must not be given access to level 4 services. However, I think it is permissible to give them access to level 3 services or below. Should I use l2cap chan->sec_level for this purpose? I'm kind of lost on the difference between hcon->sec_level and chan->sec_level. > > > } > > > > static void l2cap_do_start(struct l2cap_chan *chan) > > -- > > 2.28.0.681.g6f77f65b4e-goog > > > > > -- > Luiz Augusto von Dentz
[PATCH v1] Bluetooth: Enforce key size of 16 bytes on FIPS level
From: Archie Pusaka According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: Device in security mode 4 level 4 shall enforce: 128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened) This patch rejects connection with key size below 16 for FIPS level services. Signed-off-by: Archie Pusaka Reviewed-by: Alain Michaud --- net/bluetooth/l2cap_core.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index ade83e224567..306616ec26e6 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) * that have no key size requirements. Ensure that the link is * actually encrypted before enforcing a key size. */ + int min_key_size = hcon->hdev->min_enc_key_size; + + if (hcon->sec_level == BT_SECURITY_FIPS) + min_key_size = 16; + return (!test_bit(HCI_CONN_ENCRYPT, >flags) || - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); + hcon->enc_key_size >= min_key_size); } static void l2cap_do_start(struct l2cap_chan *chan) -- 2.28.0.681.g6f77f65b4e-goog
[PATCH v2] Bluetooth: Check for encryption key size on connect
From: Archie Pusaka
When receiving connection, we only check whether the link has been
encrypted, but not the encryption key size of the link.
This patch adds check for encryption key size, and reject L2CAP
connection which size is below the specified threshold (default 7)
with security block.
Here is some btmon trace.
@ MGMT Event: New Link Key (0x0009) plen 26{0x0001} [hci0] 5.847722
Store hint: No (0x00)
BR/EDR Address: 38:00:25:F7:F1:B0 (OUI 38-00-25)
Key type: Unauthenticated Combination key from P-192 (0x04)
Link key: 7bf2f68c81305d63a6b0ee2c5a7a34bc
PIN length: 0
> HCI Event: Encryption Change (0x08) plen 4#29 [hci0] 5.871537
Status: Success (0x00)
Handle: 256
Encryption: Enabled with E0 (0x01)
< HCI Command: Read Encryp... (0x05|0x0008) plen 2 #30 [hci0] 5.871609
Handle: 256
> HCI Event: Command Complete (0x0e) plen 7 #31 [hci0] 5.872524
Read Encryption Key Size (0x05|0x0008) ncmd 1
Status: Success (0x00)
Handle: 256
Key size: 3
// WITHOUT PATCH //
> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 5.895023
L2CAP: Connection Request (0x02) ident 3 len 4
PSM: 4097 (0x1001)
Source CID: 64
< ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 5.895213
L2CAP: Connection Response (0x03) ident 3 len 8
Destination CID: 64
Source CID: 64
Result: Connection successful (0x)
Status: No further information available (0x)
// WITH PATCH //
> ACL Data RX: Handle 256 flags 0x02 dlen 12#42 [hci0] 4.887024
L2CAP: Connection Request (0x02) ident 3 len 4
PSM: 4097 (0x1001)
Source CID: 64
< ACL Data TX: Handle 256 flags 0x00 dlen 16#43 [hci0] 4.887127
L2CAP: Connection Response (0x03) ident 3 len 8
Destination CID: 0
Source CID: 64
Result: Connection refused - security block (0x0003)
Status: No further information available (0x0000)
Signed-off-by: Archie Pusaka
Reviewed-by: Alain Michaud
---
Btw, it looks like the patch sent by Alex Lu with the title
[PATCH] Bluetooth: Fix the vulnerable issue on enc key size
also solves the exact same issue.
Changes in v2:
* Add btmon trace to the commit message
net/bluetooth/l2cap_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ade83e224567..b4fc0ad38aaa 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn
*conn,
/* Check if the ACL is secure enough (if not SDP) */
if (psm != cpu_to_le16(L2CAP_PSM_SDP) &&
- !hci_conn_check_link_mode(conn->hcon)) {
+ (!hci_conn_check_link_mode(conn->hcon) ||
+ !l2cap_check_enc_key_size(conn->hcon))) {
conn->disc_reason = HCI_ERROR_AUTH_FAILURE;
result = L2CAP_CR_SEC_BLOCK;
goto response;
--
2.28.0.681.g6f77f65b4e-goog
[PATCH v1] Bluetooth: Check for encryption key size on connect
From: Archie Pusaka
When receiving connection, we only check whether the link has been
encrypted, but not the encryption key size of the link.
This patch adds check for encryption key size, and reject L2CAP
connection which size is below the specified threshold (default 7)
with security block.
Signed-off-by: Archie Pusaka
Reviewed-by: Alain Michaud
---
net/bluetooth/l2cap_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ade83e224567..b4fc0ad38aaa 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4101,7 +4101,8 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn
*conn,
/* Check if the ACL is secure enough (if not SDP) */
if (psm != cpu_to_le16(L2CAP_PSM_SDP) &&
- !hci_conn_check_link_mode(conn->hcon)) {
+ (!hci_conn_check_link_mode(conn->hcon) ||
+ !l2cap_check_enc_key_size(conn->hcon))) {
conn->disc_reason = HCI_ERROR_AUTH_FAILURE;
result = L2CAP_CR_SEC_BLOCK;
goto response;
--
2.28.0.681.g6f77f65b4e-goog
Re: [RFC PATCH v1 1/2] Bluetooth: queue ACL packets if no handle is found
Hi Marcel,
On Mon, 29 Jun 2020 at 14:40, Marcel Holtmann wrote:
>
> Hi Archie,
>
> > There is a possibility that an ACL packet is received before we
> > receive the HCI connect event for the corresponding handle. If this
> > happens, we discard the ACL packet.
> >
> > Rather than just ignoring them, this patch provides a queue for
> > incoming ACL packet without a handle. The queue is processed when
> > receiving a HCI connection event. If 2 seconds elapsed without
> > receiving the HCI connection event, assume something bad happened
> > and discard the queued packet.
> >
> > Signed-off-by: Archie Pusaka
> > Reviewed-by: Abhishek Pandit-Subedi
>
> so two things up front. I want to hide this behind a
> HCI_QUIRK_OUT_OF_ORDER_ACL that a transport driver has to set first. Frankly
> if this kind of out-of-order happens on UART or SDIO transports, then
> something is obviously going wrong. I have no plan to fix up after a fully
> serialized transport.
>
> Secondly, if a transport sets HCI_QUIRK_OUT_OF_ORDER_ACL, then I want this
> off by default. You can enable it via an experimental setting. The reason
> here is that we have to make it really hard and fail as often as possible so
> that hardware manufactures and spec writers realize that something is
> fundamentally broken here.
>
> I have no problem in running the code and complaining loudly in case the
> quirk has been set. Just injecting the packets can only happen if bluetoothd
> explicitly enabled it.
Got it.
>
>
> >
> > ---
> >
> > include/net/bluetooth/hci_core.h | 8 +++
> > net/bluetooth/hci_core.c | 84 +---
> > net/bluetooth/hci_event.c| 2 +
> > 3 files changed, 88 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h
> > b/include/net/bluetooth/hci_core.h
> > index 836dc997ff94..b69ecdd0d15a 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -270,6 +270,9 @@ struct adv_monitor {
> > /* Default authenticated payload timeout 30s */
> > #define DEFAULT_AUTH_PAYLOAD_TIMEOUT 0x0bb8
> >
> > +/* Time to keep ACL packets without a corresponding handle queued (2s) */
> > +#define PENDING_ACL_TIMEOUT msecs_to_jiffies(2000)
> > +
>
> Do we have some btmon traces with timestamps. Isn’t a second enough? Actually
> 2 seconds is an awful long time.
When this happens in the test lab, the HCI connect event is about
0.002 second behind the first ACL packet. We can change this if
required.
>
> > struct amp_assoc {
> > __u16 len;
> > __u16 offset;
> > @@ -538,6 +541,9 @@ struct hci_dev {
> > struct delayed_work rpa_expired;
> > bdaddr_trpa;
> >
> > + struct delayed_work remove_pending_acl;
> > + struct sk_buff_head pending_acl_q;
> > +
>
> can we name this ooo_q and move it to the other queues in this struct. Unless
> we want to add a Kconfig option around it, we don’t need to keep it here.
Ack.
>
> > #if IS_ENABLED(CONFIG_BT_LEDS)
> > struct led_trigger *power_led;
> > #endif
> > @@ -1773,6 +1779,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16
> > ediv, __le64 rand,
> > void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> > u8 *bdaddr_type);
> >
> > +void hci_process_pending_acl(struct hci_dev *hdev, struct hci_conn *conn);
> > +
> > #define SCO_AIRMODE_MASK 0x0003
> > #define SCO_AIRMODE_CVSD 0x
> > #define SCO_AIRMODE_TRANSP 0x0003
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 7959b851cc63..30780242c267 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -1786,6 +1786,7 @@ int hci_dev_do_close(struct hci_dev *hdev)
> > skb_queue_purge(>rx_q);
> > skb_queue_purge(>cmd_q);
> > skb_queue_purge(>raw_q);
> > + skb_queue_purge(>pending_acl_q);
> >
> > /* Drop last sent command */
> > if (hdev->sent_cmd) {
> > @@ -3518,6 +3519,78 @@ static int hci_suspend_notifier(struct
> > notifier_block *nb, unsigned long action,
> > return NOTIFY_STOP;
> > }
> >
> > +static void hci_add_pending_acl(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > + skb_queue_tail(>pending_acl_q, skb);
> > +
> > + queue_delayed_work(hdev->workqueue, >remove_pending_acl,
> > +
[RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
From: Archie Pusaka
It is possible to receive an L2CAP conn req for an encrypted
connection, before actually receiving the HCI change encryption
event. If this happened, the received L2CAP packet will be ignored.
This patch queues the L2CAP packet and process them after the
expected HCI event is received. If after 2 seconds we still don't
receive it, then we assume something bad happened and discard the
queued packets.
Signed-off-by: Archie Pusaka
Reviewed-by: Abhishek Pandit-Subedi
---
include/net/bluetooth/bluetooth.h | 6 +++
include/net/bluetooth/l2cap.h | 6 +++
net/bluetooth/hci_event.c | 3 ++
net/bluetooth/l2cap_core.c| 87 +++
4 files changed, 91 insertions(+), 11 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h
b/include/net/bluetooth/bluetooth.h
index 7ee8041af803..e64278401084 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -335,7 +335,11 @@ struct l2cap_ctrl {
u16 reqseq;
u16 txseq;
u8 retries;
+ u8 rsp_code;
+ u8 amp_id;
+ __u8ident;
__le16 psm;
+ __le16 scid;
bdaddr_t bdaddr;
struct l2cap_chan *chan;
};
@@ -374,6 +378,8 @@ struct bt_skb_cb {
struct hci_ctrl hci;
};
};
+static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff *)0)->cb));
+
#define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
#define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 8f1e6a7a2df8..f8f6dec96f12 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -58,6 +58,7 @@
#define L2CAP_MOVE_ERTX_TIMEOUTmsecs_to_jiffies(6)
#define L2CAP_WAIT_ACK_POLL_PERIOD msecs_to_jiffies(200)
#define L2CAP_WAIT_ACK_TIMEOUT msecs_to_jiffies(1)
+#define L2CAP_PEND_ENC_CONN_TIMEOUTmsecs_to_jiffies(2000)
#define L2CAP_A2MP_DEFAULT_MTU 670
@@ -700,6 +701,9 @@ struct l2cap_conn {
struct mutexchan_lock;
struct kref ref;
struct list_headusers;
+
+ struct delayed_work remove_pending_encrypt_conn;
+ struct sk_buff_head pending_conn_q;
};
struct l2cap_user {
@@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user);
+void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
+
#endif /* __L2CAP_H */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 108c6c102a6a..8cefc51a5ca4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev,
struct sk_buff *skb)
unlock:
hci_dev_unlock(hdev);
+
+ if (conn && !ev->status && ev->encrypt)
+ l2cap_process_pending_encrypt_conn(conn);
}
static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 35d2bc569a2d..fc6fe2c80c46 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -62,6 +62,10 @@ static void l2cap_send_disconn_req(struct l2cap_chan *chan,
int err);
static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
struct sk_buff_head *skbs, u8 event);
+static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
+ u8 ident, u8 *data, u8 rsp_code,
+ u8 amp_id, bool queue_if_fail);
+
static inline u8 bdaddr_type(u8 link_type, u8 bdaddr_type)
{
if (link_type == LE_LINK) {
@@ -1902,6 +1906,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
cancel_delayed_work_sync(>info_timer);
+ cancel_delayed_work_sync(>remove_pending_encrypt_conn);
+
hcon->l2cap_data = NULL;
conn->hchan = NULL;
l2cap_conn_put(conn);
@@ -2023,6 +2029,55 @@ static void l2cap_retrans_timeout(struct work_struct
*work)
l2cap_chan_put(chan);
}
+static void l2cap_add_pending_encrypt_conn(struct l2cap_conn *conn,
+ struct l2cap_conn_req *req,
+ u8 ident, u8 rsp_code, u8 amp_id)
+{
+ struct sk_buff *skb = bt_skb_alloc(0, GFP_KERNEL);
+
+ bt_cb(skb)->l2cap.psm = req->psm;
+ bt_cb(skb)->l2cap.scid = req->scid;
+ bt_cb(skb)->l2cap.ident = ident;
+ bt_cb(skb)->l2cap.rsp_code = rsp_code;
+ bt_cb(skb)->l2cap.amp_id = amp_id;
+
+ skb_queue_tail(>
[RFC PATCH v1 0/2] handle USB endpoint race condition
From: Archie Pusaka On platforms with USB transport, events and data are transferred via different endpoints. This could potentially cause ordering problems, where the order of processed information is different than the actual order. One such a case is we receive a ACL packet before receiving the connect complete event. This could be frequently observed on ChromeOS auto test setup. > ACL Data RX: Handle 256 flags 0x02 dlen 12 #6 [hci0] 9.314240 L2CAP: Connection Request (0x02) ident 1 len 4 PSM: 1 (0x0001) Source CID: 1536 > HCI Event: Connect Complete (0x03) plen 11 #7 [hci0] 9.333236 Status: Success (0x00) Handle: 256 Address: 70:BF:92:CF:95:B7 (OUI 70-BF-92) Link type: ACL (0x01) Encryption: Disabled (0x00) If this happens, we will simply discard the ACL packet, since no corresponding handle is registered yet. The situation where the ACL packet is not replied by us is handled differently by each peripheral: some will resend the ACL packet (which will be successful, since the handle can be found now), but some will get upset and disconnect. To prevent this from happening, we propose to introduce a queue. So, instead of discarding those packets without handle, we put them in a queue. Then, we will process this queue when we receive a connect completion event for the matching handle. The queued packets will be cleared if after 2 seconds we still don't receive the corresponding connect complete event. Similarly, there is a chance that the incoming L2CAP connection request comes before HCI encryption change event. When this happens, the incoming connection will be refused. > ACL Data RX: Handle 256 flags 0x02 dlen 12 #46 [hci0] 9.275297 L2CAP: Connection Request (0x02) ident 4 len 4 PSM: 3 (0x0003) Source CID: 256 < ACL Data TX: Handle 256 flags 0x00 dlen 16 #47 [hci0] 9.275393 L2CAP: Connection Response (0x03) ident 4 len 8 Destination CID: 0 Source CID: 256 Result: Connection refused - security block (0x0003) Status: No further information available (0x) > HCI Event: Encryption Change (0x08) plen 4 #51 [hci0] 9.314109 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) We propose to handle this case with a similar solution: queuing the L2CAP connection request until we receive HCI encryption change. The proposed patch is tested with a special Intel firmware which allows us to purposely delay the "connect complete" event. We plan to merge this change to ChromeOS auto test setup to see whether this could fix the problem there, but we also would like to have your input on this in the meantime. Thanks, Archie Archie Pusaka (2): Bluetooth: queue ACL packets if no handle is found Bluetooth: queue L2CAP conn req if encryption is needed include/net/bluetooth/bluetooth.h | 6 +++ include/net/bluetooth/hci_core.h | 8 +++ include/net/bluetooth/l2cap.h | 6 +++ net/bluetooth/hci_core.c | 84 ++--- net/bluetooth/hci_event.c | 5 ++ net/bluetooth/l2cap_core.c| 87 +++ 6 files changed, 179 insertions(+), 17 deletions(-) -- 2.27.0.212.ge8ba1cc988-goog
[RFC PATCH v1 1/2] Bluetooth: queue ACL packets if no handle is found
From: Archie Pusaka
There is a possibility that an ACL packet is received before we
receive the HCI connect event for the corresponding handle. If this
happens, we discard the ACL packet.
Rather than just ignoring them, this patch provides a queue for
incoming ACL packet without a handle. The queue is processed when
receiving a HCI connection event. If 2 seconds elapsed without
receiving the HCI connection event, assume something bad happened
and discard the queued packet.
Signed-off-by: Archie Pusaka
Reviewed-by: Abhishek Pandit-Subedi
---
include/net/bluetooth/hci_core.h | 8 +++
net/bluetooth/hci_core.c | 84 +---
net/bluetooth/hci_event.c| 2 +
3 files changed, 88 insertions(+), 6 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 836dc997ff94..b69ecdd0d15a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -270,6 +270,9 @@ struct adv_monitor {
/* Default authenticated payload timeout 30s */
#define DEFAULT_AUTH_PAYLOAD_TIMEOUT 0x0bb8
+/* Time to keep ACL packets without a corresponding handle queued (2s) */
+#define PENDING_ACL_TIMEOUTmsecs_to_jiffies(2000)
+
struct amp_assoc {
__u16 len;
__u16 offset;
@@ -538,6 +541,9 @@ struct hci_dev {
struct delayed_work rpa_expired;
bdaddr_trpa;
+ struct delayed_work remove_pending_acl;
+ struct sk_buff_head pending_acl_q;
+
#if IS_ENABLED(CONFIG_BT_LEDS)
struct led_trigger *power_led;
#endif
@@ -1773,6 +1779,8 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv,
__le64 rand,
void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
u8 *bdaddr_type);
+void hci_process_pending_acl(struct hci_dev *hdev, struct hci_conn *conn);
+
#define SCO_AIRMODE_MASK 0x0003
#define SCO_AIRMODE_CVSD 0x
#define SCO_AIRMODE_TRANSP 0x0003
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7959b851cc63..30780242c267 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1786,6 +1786,7 @@ int hci_dev_do_close(struct hci_dev *hdev)
skb_queue_purge(>rx_q);
skb_queue_purge(>cmd_q);
skb_queue_purge(>raw_q);
+ skb_queue_purge(>pending_acl_q);
/* Drop last sent command */
if (hdev->sent_cmd) {
@@ -3518,6 +3519,78 @@ static int hci_suspend_notifier(struct notifier_block
*nb, unsigned long action,
return NOTIFY_STOP;
}
+static void hci_add_pending_acl(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ skb_queue_tail(>pending_acl_q, skb);
+
+ queue_delayed_work(hdev->workqueue, >remove_pending_acl,
+ PENDING_ACL_TIMEOUT);
+}
+
+void hci_process_pending_acl(struct hci_dev *hdev, struct hci_conn *conn)
+{
+ struct sk_buff *skb, *tmp;
+ struct hci_acl_hdr *hdr;
+ u16 handle, flags;
+ bool reset_timer = false;
+
+ skb_queue_walk_safe(>pending_acl_q, skb, tmp) {
+ hdr = (struct hci_acl_hdr *)skb->data;
+ handle = __le16_to_cpu(hdr->handle);
+ flags = hci_flags(handle);
+ handle = hci_handle(handle);
+
+ if (handle != conn->handle)
+ continue;
+
+ __skb_unlink(skb, >pending_acl_q);
+ skb_pull(skb, HCI_ACL_HDR_SIZE);
+
+ l2cap_recv_acldata(conn, skb, flags);
+ reset_timer = true;
+ }
+
+ if (reset_timer)
+ mod_delayed_work(hdev->workqueue, >remove_pending_acl,
+PENDING_ACL_TIMEOUT);
+}
+
+/* Remove the oldest pending ACL, and all pending ACLs with the same handle */
+static void hci_remove_pending_acl(struct work_struct *work)
+{
+ struct hci_dev *hdev;
+ struct sk_buff *skb, *tmp;
+ struct hci_acl_hdr *hdr;
+ u16 handle, oldest_handle;
+
+ hdev = container_of(work, struct hci_dev, remove_pending_acl.work);
+ skb = skb_dequeue(>pending_acl_q);
+
+ if (!skb)
+ return;
+
+ hdr = (struct hci_acl_hdr *)skb->data;
+ oldest_handle = hci_handle(__le16_to_cpu(hdr->handle));
+ kfree_skb(skb);
+
+ bt_dev_err(hdev, "ACL packet for unknown connection handle %d",
+ oldest_handle);
+
+ skb_queue_walk_safe(>pending_acl_q, skb, tmp) {
+ hdr = (struct hci_acl_hdr *)skb->data;
+ handle = hci_handle(__le16_to_cpu(hdr->handle));
+
+ if (handle == oldest_handle) {
+ __skb_unlink(skb, >pending_acl_q);
+ kfree_skb(skb);
+ }
+ }
+
+ if (!skb_queue_empty(>pending_acl_q))
+ queue_delayed_work(hdev->workqueue, >remove_pending_ac
