[PATCH 1/1] Bluetooth: msft: fix vendor event use-after-free during open
Ren Wei <[email protected]> Tue, 28 Jul 2026 01:16:04 +0800
| Newsgroups | org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <0e31ed963082a6fdabbbb10b913d35d6be5e7a19.1784880004.git.edragain@163.com> |
From: Yong Wang <[email protected]> Commit 5031ffcc79b8 ("Bluetooth: Keep MSFT ext info throughout a hci_dev's life cycle") changed msft_do_open() to reuse the live hdev->msft_data object across power cycles. That makes msft_do_open() free and replace msft->evt_prefix while the MSFT extension is already published through hdev->msft_data. On failure it can also clear hdev->msft_data and free the whole msft object. At the same time, msft_vendor_evt() reads hdev->msft_data and checks msft->evt_prefix_len / msft->evt_prefix before taking hci_dev_lock(). Since HCI vendor events may still be processed while HCI_INIT is set, vendor event handling can race with msft_do_open() and hit a use-after- free on the old prefix buffer or the msft_data object itself. Fix this by reading the supported feature data into a temporary object first, then updating the live MSFT state only while holding hci_dev_lock(). Also make msft_vendor_evt() take hci_dev_lock() before reading hdev->msft_data and the event prefix state. This keeps the published MSFT state stable across the open path and serializes it against vendor event processing. Fixes: 5031ffcc79b8 ("Bluetooth: Keep MSFT ext info throughout a hci_dev's life cycle") Cc: [email protected] Reported-by: Vega <[email protected]> Assisted-by: Codex:GPT-5.4 Signed-off-by: Yong Wang <[email protected]> Signed-off-by: Ren Wei <[email protected]> --- net/bluetooth/msft.c | 70 +++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c index d7badce8746c..ff8629638299 100644 --- a/net/bluetooth/msft.c +++ b/net/bluetooth/msft.c @@ -133,13 +133,19 @@ struct msft_data { struct mutex filter_lock; }; +struct msft_supported_features { + __u64 features; + __u8 evt_prefix_len; + __u8 *evt_prefix; +}; + bool msft_monitor_supported(struct hci_dev *hdev) { return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR); } static bool read_supported_features(struct hci_dev *hdev, - struct msft_data *msft) + struct msft_supported_features *supported) { struct msft_cp_read_supported_features cp; struct msft_rp_read_supported_features *rp; @@ -166,17 +172,14 @@ static bool read_supported_features(struct hci_dev *hdev, goto failed; if (rp->evt_prefix_len > 0) { - msft->evt_prefix = kmemdup(rp->evt_prefix, rp->evt_prefix_len, - GFP_KERNEL); - if (!msft->evt_prefix) + supported->evt_prefix = + kmemdup(rp->evt_prefix, rp->evt_prefix_len, GFP_KERNEL); + if (!supported->evt_prefix) goto failed; } - msft->evt_prefix_len = rp->evt_prefix_len; - msft->features = __le64_to_cpu(rp->features); - - if (msft->features & MSFT_FEATURE_MASK_CURVE_VALIDITY) - hdev->msft_curve_validity = true; + supported->evt_prefix_len = rp->evt_prefix_len; + supported->features = __le64_to_cpu(rp->features); kfree_skb(skb); return true; @@ -631,6 +634,7 @@ int msft_resume_sync(struct hci_dev *hdev) void msft_do_open(struct hci_dev *hdev) { struct msft_data *msft = hdev->msft_data; + struct msft_supported_features supported = {}; if (hdev->msft_opcode == HCI_OP_NOP) return; @@ -642,19 +646,29 @@ void msft_do_open(struct hci_dev *hdev) bt_dev_dbg(hdev, "Initialize MSFT extension"); - /* Reset existing MSFT data before re-reading */ - kfree(msft->evt_prefix); - msft->evt_prefix = NULL; - msft->evt_prefix_len = 0; - msft->features = 0; - - if (!read_supported_features(hdev, msft)) { - hdev->msft_data = NULL; - kfree(msft); + if (!read_supported_features(hdev, &supported)) { + hci_dev_lock(hdev); + kfree(msft->evt_prefix); + msft->evt_prefix = NULL; + msft->evt_prefix_len = 0; + msft->features = 0; + hci_dev_unlock(hdev); return; } - if (msft_monitor_supported(hdev)) { + hci_dev_lock(hdev); + + kfree(msft->evt_prefix); + msft->evt_prefix = supported.evt_prefix; + msft->evt_prefix_len = supported.evt_prefix_len; + msft->features = supported.features; + + if (supported.features & MSFT_FEATURE_MASK_CURVE_VALIDITY) + hdev->msft_curve_validity = true; + + hci_dev_unlock(hdev); + + if (supported.features & MSFT_FEATURE_MASK_LE_ADV_MONITOR) { msft->resuming = true; msft_set_filter_enable(hdev, true); /* Monitors get removed on power off, so we need to explicitly @@ -1067,12 +1081,15 @@ 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; + hci_dev_lock(hdev); + + msft = hdev->msft_data; if (!msft) - return; + goto unlock; /* When the extension has defined an event prefix, check that it * matches, and otherwise just return. @@ -1080,23 +1097,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 unlock; if (memcmp(evt_prefix, msft->evt_prefix, msft->evt_prefix_len)) - return; + goto unlock; } /* 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 unlock; evt = msft_skb_pull(hdev, skb, 0, sizeof(*evt)); if (!evt) - return; - - hci_dev_lock(hdev); + goto unlock; switch (*evt) { case MSFT_EV_LE_MONITOR_DEVICE: @@ -1110,6 +1125,7 @@ void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb) break; } +unlock: hci_dev_unlock(hdev); } -- 2.53.0