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 |
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 |
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
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 --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; }