diff mbox series

Bluetooth: msft: fix slab-use-after-free in msft_do_close()

Message ID 20240427051934.879051-1-iam@sung-woo.kim (mailing list archive)
State Superseded
Headers show
Series Bluetooth: msft: fix slab-use-after-free in msft_do_close() | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #100: Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 total: 0 errors, 1 warnings, 0 checks, 287 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13645561.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 19: B1 Line exceeds max length (90>80): "BUG: KASAN: slab-use-after-free in __mutex_lock_common kernel/locking/mutex.c:587 [inline]" 20: B1 Line exceeds max length (85>80): "BUG: KASAN: slab-use-after-free in __mutex_lock+0x8f/0xc30 kernel/locking/mutex.c:752"
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS
tedd_an/CheckSmatch fail CheckSparse: FAIL: Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139 make[4]: *** Deleting file 'net/bluetooth/hci_core.o' make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: net] Error 2 make[2]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139 make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o' make[4]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139 make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o' make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: drivers] Error 2 make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2 make: *** [Makefile:240: __sub-make] Error 2
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Sungwoo Kim April 27, 2024, 5:19 a.m. UTC
Add refcnt to safely destroy hdev->msft_data.

How msft is used after freed:

[use]
msft_do_close()
  if (!msft)                      ...(1)
    return;
  mutex_lock(&msft->filter_lock); ...(4) <- used after freed

[free]
msft_unregister()
  hdev->msft_data = NULL;         ...(2)
  kfree(msft);                    ...(3) <- msft is freed

==================================================================
BUG: KASAN: slab-use-after-free in __mutex_lock_common kernel/locking/mutex.c:587 [inline]
BUG: KASAN: slab-use-after-free in __mutex_lock+0x8f/0xc30 kernel/locking/mutex.c:752
Read of size 8 at addr ffff888106cbbca8 by task kworker/u5:2/309

CPU: 0 PID: 309 Comm: kworker/u5:2 Not tainted 6.9.0-rc5+ #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Workqueue: hci4 hci_error_reset
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xcd/0x140 lib/dump_stack.c:114
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x191/0x560 mm/kasan/report.c:488
 kasan_report+0xe2/0x120 mm/kasan/report.c:601
 __asan_report_load8_noabort+0x18/0x20 mm/kasan/report_generic.c:381
 __mutex_lock_common kernel/locking/mutex.c:587 [inline]
 __mutex_lock+0x8f/0xc30 kernel/locking/mutex.c:752
 mutex_lock_nested+0x1f/0x30 kernel/locking/mutex.c:804
 msft_do_close+0x292/0x700 net/bluetooth/msft.c:694
 hci_dev_close_sync+0x906/0xf10 net/bluetooth/hci_sync.c:5168
 hci_dev_do_close net/bluetooth/hci_core.c:554 [inline]
 hci_error_reset+0x152/0x410 net/bluetooth/hci_core.c:1091
 process_one_work kernel/workqueue.c:3254 [inline]
 process_scheduled_works+0x90f/0x1530 kernel/workqueue.c:3335
 worker_thread+0x926/0xe70 kernel/workqueue.c:3416
 kthread+0x2e3/0x380 kernel/kthread.c:388
 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
 </TASK>

Allocated by task 7328:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x30/0x70 mm/kasan/common.c:68
 kasan_save_alloc_info+0x3c/0x50 mm/kasan/generic.c:565
 poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
 __kasan_kmalloc+0xa2/0xc0 mm/kasan/common.c:387
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 kmalloc_trace+0x20c/0x3e0 mm/slub.c:3997
 kmalloc include/linux/slab.h:628 [inline]
 kzalloc include/linux/slab.h:749 [inline]
 msft_register+0x66/0x1d0 net/bluetooth/msft.c:760
 hci_register_dev+0x85e/0x9a0 net/bluetooth/hci_core.c:2737
 __vhci_create_device drivers/bluetooth/hci_vhci.c:438 [inline]
 vhci_create_device+0x390/0x720 drivers/bluetooth/hci_vhci.c:480
 vhci_get_user drivers/bluetooth/hci_vhci.c:537 [inline]
 vhci_write+0x39b/0x460 drivers/bluetooth/hci_vhci.c:617
 call_write_iter include/linux/fs.h:2110 [inline]
 new_sync_write fs/read_write.c:497 [inline]
 vfs_write+0x8eb/0xb50 fs/read_write.c:590
 ksys_write+0x106/0x1f0 fs/read_write.c:643
 __do_sys_write fs/read_write.c:655 [inline]
 __se_sys_write fs/read_write.c:652 [inline]
 __x64_sys_write+0x84/0xa0 fs/read_write.c:652
 x64_sys_call+0x271a/0x2ce0 arch/x86/include/generated/asm/syscalls_64.h:2
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x9c/0x130 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Freed by task 7332:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x30/0x70 mm/kasan/common.c:68
 kasan_save_free_info+0x44/0x50 mm/kasan/generic.c:579
 poison_slab_object+0x11a/0x190 mm/kasan/common.c:240
 __kasan_slab_free+0x3b/0x60 mm/kasan/common.c:256
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2106 [inline]
 slab_free mm/slub.c:4280 [inline]
 kfree+0x13c/0x330 mm/slub.c:4390
 msft_unregister+0x9d/0x120 net/bluetooth/msft.c:785
 hci_unregister_dev+0x1d9/0x520 net/bluetooth/hci_core.c:2771
 vhci_release+0x8c/0xe0 drivers/bluetooth/hci_vhci.c:674
 __fput+0x36f/0x750 fs/file_table.c:422
 ____fput+0x1e/0x30 fs/file_table.c:450
 task_work_run+0x1da/0x280 kernel/task_work.c:180
 exit_task_work include/linux/task_work.h:38 [inline]
 do_exit+0x856/0x2210 kernel/exit.c:878
 do_group_exit+0x201/0x2c0 kernel/exit.c:1027
 get_signal+0x12ff/0x1380 kernel/signal.c:2911
 arch_do_signal_or_restart+0x3b/0x650 arch/x86/kernel/signal.c:310
 exit_to_user_mode_loop kernel/entry/common.c:111 [inline]
 exit_to_user_mode_prepare include/linux/entry-common.h:328 [inline]
 __syscall_exit_to_user_mode_work kernel/entry/common.c:207 [inline]
 syscall_exit_to_user_mode+0xcc/0x2a0 kernel/entry/common.c:218
 do_syscall_64+0xa8/0x130 arch/x86/entry/common.c:89
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

The buggy address belongs to the object at ffff888106cbbc00
 which belongs to the cache kmalloc-256 of size 256
The buggy address is located 168 bytes inside of
 freed 256-byte region [ffff888106cbbc00, ffff888106cbbd00)

The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x106cba
head: order:1 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0x17ffffe0000840(slab|head|node=0|zone=2|lastcpupid=0x3fffff)
page_type: 0xffffffff()
raw: 0017ffffe0000840 ffff888100042040 ffffea00042de590 ffffea00041b3e10
raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
head: 0017ffffe0000840 ffff888100042040 ffffea00042de590 ffffea00041b3e10
head: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
head: 0017ffffe0000001 ffffea00041b2e81 dead000000000122 00000000ffffffff
head: 0000000200000000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff888106cbbb80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888106cbbc00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888106cbbc80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                  ^
 ffff888106cbbd00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff888106cbbd80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================

Fixes: 9e14606d8f38 ("Bluetooth: disable advertisement filters during suspend")
Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
 net/bluetooth/msft.c | 124 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 99 insertions(+), 25 deletions(-)

Comments

bluez.test.bot@gmail.com April 27, 2024, 5:53 a.m. UTC | #1
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=848460

---Test result---

Test Summary:
CheckPatch                    FAIL      1.00 seconds
GitLint                       FAIL      0.48 seconds
SubjectPrefix                 PASS      0.10 seconds
BuildKernel                   PASS      30.37 seconds
CheckAllWarning               PASS      32.62 seconds
CheckSparse                   PASS      38.84 seconds
CheckSmatch                   FAIL      36.55 seconds
BuildKernel32                 PASS      29.93 seconds
TestRunnerSetup               PASS      521.44 seconds
TestRunner_l2cap-tester       FAIL      4.60 seconds
TestRunner_iso-tester         FAIL      4.63 seconds
TestRunner_bnep-tester        FAIL      4.54 seconds
TestRunner_mgmt-tester        FAIL      4.67 seconds
TestRunner_rfcomm-tester      FAIL      4.60 seconds
TestRunner_sco-tester         FAIL      4.57 seconds
TestRunner_ioctl-tester       FAIL      4.65 seconds
TestRunner_mesh-tester        FAIL      4.60 seconds
TestRunner_smp-tester         FAIL      5.68 seconds
TestRunner_userchan-tester    FAIL      4.65 seconds
IncrementalBuild              PASS      28.36 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: msft: fix slab-use-after-free in msft_do_close()
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#100: 
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014

total: 0 errors, 1 warnings, 0 checks, 287 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13645561.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: msft: fix slab-use-after-free in msft_do_close()

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
19: B1 Line exceeds max length (90>80): "BUG: KASAN: slab-use-after-free in __mutex_lock_common kernel/locking/mutex.c:587 [inline]"
20: B1 Line exceeds max length (85>80): "BUG: KASAN: slab-use-after-free in __mutex_lock+0x8f/0xc30 kernel/locking/mutex.c:752"
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o'
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_bnep-tester - FAIL
Desc: Run bnep-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_rfcomm-tester - FAIL
Desc: Run rfcomm-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_smp-tester - FAIL
Desc: Run smp-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_userchan-tester - FAIL
Desc: Run userchan-tester with test-runner
Output:
No test result found


---
Regards,
Linux Bluetooth
Sungwoo Kim April 28, 2024, 3:47 a.m. UTC | #2
Hello,

Could you not apply this? I found an error.

On Sat, Apr 27, 2024 at 1:19 AM Sungwoo Kim <iam@sung-woo.kim> wrote:
>
> +
> +static struct msft_data *msft_data_hold_unless_zero(struct msft_data *msft)
> +{
> +       BT_DBG("msft %p orig refcnt %u", msft, kref_read(&msft->kref));

Here, msft could be NULL. &msft->kref causes a null-ptr-deref error.
I already sent a v2 patch fixing this.

> +
> +       if (!msft)
> +               return NULL;
> +
> +       if (!kref_get_unless_zero(&msft->kref))
> +               return NULL;
> +
> +       return msft;
> +}
> +

Thanks,
Sungwoo.
diff mbox series

Patch

diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 9612c5d1b..d42150cf9 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -132,10 +132,45 @@  struct msft_data {
 	__u8 filter_enabled;
 	/* To synchronize add/remove address filter and monitor device event.*/
 	struct mutex filter_lock;
+	struct kref	kref;
 };
 
+static void msft_data_free(struct kref *kref);
+
+static struct msft_data *msft_data_hold_unless_zero(struct msft_data *msft)
+{
+	BT_DBG("msft %p orig refcnt %u", msft, kref_read(&msft->kref));
+
+	if (!msft)
+		return NULL;
+
+	if (!kref_get_unless_zero(&msft->kref))
+		return NULL;
+
+	return msft;
+}
+
+static void msft_data_put(struct msft_data *msft)
+{
+	BT_DBG("msft %p orig refcnt %u", msft, kref_read(&msft->kref));
+
+	kref_put(&msft->kref, msft_data_free);
+}
+
+static void msft_data_free(struct kref *kref)
+{
+	struct msft_data *msft = container_of(kref, struct msft_data, kref);
+
+	BT_DBG("msft %p", msft);
+
+	kfree(msft->evt_prefix);
+	mutex_destroy(&msft->filter_lock);
+	kfree(msft);
+}
+
 bool msft_monitor_supported(struct hci_dev *hdev)
 {
+	/* msft_get_features() holds and put hdev->msft_data */
 	return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
 }
 
@@ -449,12 +484,17 @@  static int msft_remove_monitor_sync(struct hci_dev *hdev,
 /* This function requires the caller holds hci_req_sync_lock */
 int msft_suspend_sync(struct hci_dev *hdev)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
 	struct adv_monitor *monitor;
 	int handle = 0;
 
-	if (!msft || !msft_monitor_supported(hdev))
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
+	if (!msft)
 		return 0;
+	if (!msft_monitor_supported(hdev)) {
+		msft_data_put(msft);
+		return 0;
+	}
 
 	msft->suspending = true;
 
@@ -471,6 +511,7 @@  int msft_suspend_sync(struct hci_dev *hdev)
 	/* All monitors have been removed */
 	msft->suspending = false;
 
+	msft_data_put(msft);
 	return 0;
 }
 
@@ -608,11 +649,17 @@  static void reregister_monitor(struct hci_dev *hdev)
 /* This function requires the caller holds hci_req_sync_lock */
 int msft_resume_sync(struct hci_dev *hdev)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
 
-	if (!msft || !msft_monitor_supported(hdev))
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
+	if (!msft)
 		return 0;
 
+	if (!msft_monitor_supported(hdev)) {
+		msft_data_put(msft);
+		return 0;
+	}
+
 	hci_dev_lock(hdev);
 
 	/* Clear already tracked devices on resume. Once the monitors are
@@ -625,17 +672,19 @@  int msft_resume_sync(struct hci_dev *hdev)
 
 	reregister_monitor(hdev);
 
+	msft_data_put(msft);
 	return 0;
 }
 
 /* This function requires the caller holds hci_req_sync_lock */
 void msft_do_open(struct hci_dev *hdev)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
 
 	if (hdev->msft_opcode == HCI_OP_NOP)
 		return;
 
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
 	if (!msft) {
 		bt_dev_err(hdev, "MSFT extension not registered");
 		return;
@@ -650,8 +699,7 @@  void msft_do_open(struct hci_dev *hdev)
 	msft->features = 0;
 
 	if (!read_supported_features(hdev, msft)) {
-		hdev->msft_data = NULL;
-		kfree(msft);
+		msft_data_put(msft);
 		return;
 	}
 
@@ -663,15 +711,17 @@  void msft_do_open(struct hci_dev *hdev)
 		 */
 		reregister_monitor(hdev);
 	}
+	msft_data_put(msft);
 }
 
 void msft_do_close(struct hci_dev *hdev)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
 	struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
 	struct msft_monitor_addr_filter_data *address_filter, *n;
 	struct adv_monitor *monitor;
 
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
 	if (!msft)
 		return;
 
@@ -705,6 +755,8 @@  void msft_do_close(struct hci_dev *hdev)
 	hdev->advmon_pend_notify = false;
 	msft_monitor_device_del(hdev, 0, NULL, 0, true);
 
+	msft_data_put(msft);
+
 	hci_dev_unlock(hdev);
 }
 
@@ -767,6 +819,7 @@  void msft_register(struct hci_dev *hdev)
 	INIT_LIST_HEAD(&msft->address_filters);
 	hdev->msft_data = msft;
 	mutex_init(&msft->filter_lock);
+	kref_init(&msft->kref);
 }
 
 void msft_unregister(struct hci_dev *hdev)
@@ -779,10 +832,7 @@  void msft_unregister(struct hci_dev *hdev)
 	bt_dev_dbg(hdev, "Unregister MSFT extension");
 
 	hdev->msft_data = NULL;
-
-	kfree(msft->evt_prefix);
-	mutex_destroy(&msft->filter_lock);
-	kfree(msft);
+	msft_data_put(msft);
 }
 
 /* This function requires the caller holds hdev->lock */
@@ -1068,10 +1118,11 @@  static void msft_monitor_device_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
 	u8 *evt_prefix;
 	u8 *evt;
 
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
 	if (!msft)
 		return;
 
@@ -1081,21 +1132,21 @@  void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 	if (msft->evt_prefix_len > 0) {
 		evt_prefix = msft_skb_pull(hdev, skb, 0, msft->evt_prefix_len);
 		if (!evt_prefix)
-			return;
+			goto done;
 
 		if (memcmp(evt_prefix, msft->evt_prefix, msft->evt_prefix_len))
-			return;
+			goto done;
 	}
 
 	/* Every event starts at least with an event code and the rest of
 	 * the data is variable and depends on the event code.
 	 */
 	if (skb->len < 1)
-		return;
+		goto done;
 
 	evt = msft_skb_pull(hdev, skb, 0, sizeof(*evt));
 	if (!evt)
-		return;
+		goto done;
 
 	hci_dev_lock(hdev);
 
@@ -1112,13 +1163,24 @@  void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 	}
 
 	hci_dev_unlock(hdev);
+
+done:
+	msft_data_put(msft);
 }
 
 __u64 msft_get_features(struct hci_dev *hdev)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
+	unsigned long long features;
+
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
+	if (!msft)
+		return 0;
+
+	features = msft->features;
 
-	return msft ? msft->features : 0;
+	msft_data_put(msft);
+	return features;
 }
 
 static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
@@ -1152,37 +1214,48 @@  static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
 /* This function requires the caller holds hci_req_sync_lock */
 int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
+	int err;
 
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
 	if (!msft)
 		return -EOPNOTSUPP;
 
-	if (msft->resuming || msft->suspending)
+	if (msft->resuming || msft->suspending) {
+		msft_data_put(msft);
 		return -EBUSY;
+	}
 
-	return msft_add_monitor_sync(hdev, monitor);
+	err = msft_add_monitor_sync(hdev, monitor);
+	msft_data_put(msft);
+	return err;
 }
 
 /* This function requires the caller holds hci_req_sync_lock */
 int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
 {
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
+	int err;
 
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
 	if (!msft)
 		return -EOPNOTSUPP;
 
 	if (msft->resuming || msft->suspending)
 		return -EBUSY;
 
-	return msft_remove_monitor_sync(hdev, monitor);
+	err = msft_remove_monitor_sync(hdev, monitor);
+	msft_data_put(msft);
+	return err;
 }
 
 int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
 {
 	struct msft_cp_le_set_advertisement_filter_enable cp;
-	struct msft_data *msft = hdev->msft_data;
+	struct msft_data *msft;
 	int err;
 
+	msft = msft_data_hold_unless_zero(hdev->msft_data);
 	if (!msft)
 		return -EOPNOTSUPP;
 
@@ -1193,6 +1266,7 @@  int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
 
 	msft_le_set_advertisement_filter_enable_cb(hdev, &cp, err);
 
+	msft_data_put(msft);
 	return 0;
 }