| Message ID | 20240104115453.1.Iaa08c695d3dcf819910ea723c3eb502935638172@changeid |
|---|---|
| State | Accepted |
| Commit | 4b85ee4187306be98113b8fa290d535ae6efa812 |
| Headers | show |
| Series | Bluetooth: Avoid potential use-after-free in hci_error_reset | expand |
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=814331 ---Test result--- Test Summary: CheckPatch PASS 0.46 seconds GitLint PASS 0.21 seconds SubjectPrefix PASS 0.07 seconds BuildKernel PASS 28.16 seconds CheckAllWarning PASS 30.93 seconds CheckSparse PASS 37.00 seconds CheckSmatch PASS 103.01 seconds BuildKernel32 PASS 28.80 seconds TestRunnerSetup PASS 454.51 seconds TestRunner_l2cap-tester PASS 23.83 seconds TestRunner_iso-tester PASS 46.40 seconds TestRunner_bnep-tester PASS 6.87 seconds TestRunner_mgmt-tester PASS 166.66 seconds TestRunner_rfcomm-tester PASS 10.79 seconds TestRunner_sco-tester PASS 14.23 seconds TestRunner_ioctl-tester PASS 12.28 seconds TestRunner_mesh-tester PASS 8.78 seconds TestRunner_smp-tester PASS 9.67 seconds TestRunner_userchan-tester PASS 7.24 seconds IncrementalBuild PASS 25.97 seconds --- Regards, Linux Bluetooth
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Thu, 4 Jan 2024 11:56:32 +0000 you wrote: > While handling the HCI_EV_HARDWARE_ERROR event, if the underlying > BT controller is not responding, the GPIO reset mechanism would > free the hci_dev and lead to a use-after-free in hci_error_reset. > > Here's the call trace observed on a ChromeOS device with Intel AX201: > queue_work_on+0x3e/0x6c > __hci_cmd_sync_sk+0x2ee/0x4c0 [bluetooth <HASH:3b4a6>] > ? init_wait_entry+0x31/0x31 > __hci_cmd_sync+0x16/0x20 [bluetooth <HASH:3b4a 6>] > hci_error_reset+0x4f/0xa4 [bluetooth <HASH:3b4a 6>] > process_one_work+0x1d8/0x33f > worker_thread+0x21b/0x373 > kthread+0x13a/0x152 > ? pr_cont_work+0x54/0x54 > ? kthread_blkcg+0x31/0x31 > ret_from_fork+0x1f/0x30 > > [...] Here is the summary with links: - Bluetooth: Avoid potential use-after-free in hci_error_reset https://git.kernel.org/bluetooth/bluetooth-next/c/c6d011a4e7d3 You are awesome, thank you!
Le 04/01/2024 à 12:56, Ying Hsu a écrit : > While handling the HCI_EV_HARDWARE_ERROR event, if the underlying > BT controller is not responding, the GPIO reset mechanism would > free the hci_dev and lead to a use-after-free in hci_error_reset. > > Here's the call trace observed on a ChromeOS device with Intel AX201: > queue_work_on+0x3e/0x6c > __hci_cmd_sync_sk+0x2ee/0x4c0 [bluetooth <HASH:3b4a6>] > ? init_wait_entry+0x31/0x31 > __hci_cmd_sync+0x16/0x20 [bluetooth <HASH:3b4a 6>] > hci_error_reset+0x4f/0xa4 [bluetooth <HASH:3b4a 6>] > process_one_work+0x1d8/0x33f > worker_thread+0x21b/0x373 > kthread+0x13a/0x152 > ? pr_cont_work+0x54/0x54 > ? kthread_blkcg+0x31/0x31 > ret_from_fork+0x1f/0x30 > > This patch holds the reference count on the hci_dev while processing > a HCI_EV_HARDWARE_ERROR event to avoid potential crash. > > Signed-off-by: Ying Hsu <yinghsu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > --- > Tested this commit on a chromebook with Intel BT controller. > > net/bluetooth/hci_core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 65601aa52e0d..a42417926028 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -1049,6 +1049,7 @@ static void hci_error_reset(struct work_struct *work) > { > struct hci_dev *hdev = container_of(work, struct hci_dev, error_reset); > > + hci_dev_hold(hdev); > BT_DBG("%s", hdev->name); > > if (hdev->hw_error) > @@ -1060,6 +1061,7 @@ static void hci_error_reset(struct work_struct *work) > return; ^^^^^ Should we also call hci_dev_put() if we hit this return? CJ > > hci_dev_do_open(hdev); > + hci_dev_put(hdev); > } > > void hci_uuids_clear(struct hci_dev *hdev)
Hi Christophe, On Fri, Jan 5, 2024 at 2:15 AM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > Le 04/01/2024 à 12:56, Ying Hsu a écrit : > > While handling the HCI_EV_HARDWARE_ERROR event, if the underlying > > BT controller is not responding, the GPIO reset mechanism would > > free the hci_dev and lead to a use-after-free in hci_error_reset. > > > > Here's the call trace observed on a ChromeOS device with Intel AX201: > > queue_work_on+0x3e/0x6c > > __hci_cmd_sync_sk+0x2ee/0x4c0 [bluetooth <HASH:3b4a6>] > > ? init_wait_entry+0x31/0x31 > > __hci_cmd_sync+0x16/0x20 [bluetooth <HASH:3b4a 6>] > > hci_error_reset+0x4f/0xa4 [bluetooth <HASH:3b4a 6>] > > process_one_work+0x1d8/0x33f > > worker_thread+0x21b/0x373 > > kthread+0x13a/0x152 > > ? pr_cont_work+0x54/0x54 > > ? kthread_blkcg+0x31/0x31 > > ret_from_fork+0x1f/0x30 > > > > This patch holds the reference count on the hci_dev while processing > > a HCI_EV_HARDWARE_ERROR event to avoid potential crash. > > > > Signed-off-by: Ying Hsu <yinghsu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > > --- > > Tested this commit on a chromebook with Intel BT controller. > > > > net/bluetooth/hci_core.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > index 65601aa52e0d..a42417926028 100644 > > --- a/net/bluetooth/hci_core.c > > +++ b/net/bluetooth/hci_core.c > > @@ -1049,6 +1049,7 @@ static void hci_error_reset(struct work_struct *work) > > { > > struct hci_dev *hdev = container_of(work, struct hci_dev, error_reset); > > > > + hci_dev_hold(hdev); > > BT_DBG("%s", hdev->name); > > > > if (hdev->hw_error) > > @@ -1060,6 +1061,7 @@ static void hci_error_reset(struct work_struct *work) > > return; > > ^^^^^ > Should we also call hci_dev_put() if we hit this return? Yep, I will fix that since Ive already pushed to bluetooth-next. > > CJ > > > > > hci_dev_do_open(hdev); > > + hci_dev_put(hdev); > > } > > > > void hci_uuids_clear(struct hci_dev *hdev) >
Hi, On Fri, Jan 5, 2024 at 10:36 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Christophe, > > On Fri, Jan 5, 2024 at 2:15 AM Christophe JAILLET > <christophe.jaillet@wanadoo.fr> wrote: > > > > Le 04/01/2024 à 12:56, Ying Hsu a écrit : > > > While handling the HCI_EV_HARDWARE_ERROR event, if the underlying > > > BT controller is not responding, the GPIO reset mechanism would > > > free the hci_dev and lead to a use-after-free in hci_error_reset. > > > > > > Here's the call trace observed on a ChromeOS device with Intel AX201: > > > queue_work_on+0x3e/0x6c > > > __hci_cmd_sync_sk+0x2ee/0x4c0 [bluetooth <HASH:3b4a6>] > > > ? init_wait_entry+0x31/0x31 > > > __hci_cmd_sync+0x16/0x20 [bluetooth <HASH:3b4a 6>] > > > hci_error_reset+0x4f/0xa4 [bluetooth <HASH:3b4a 6>] > > > process_one_work+0x1d8/0x33f > > > worker_thread+0x21b/0x373 > > > kthread+0x13a/0x152 > > > ? pr_cont_work+0x54/0x54 > > > ? kthread_blkcg+0x31/0x31 > > > ret_from_fork+0x1f/0x30 > > > > > > This patch holds the reference count on the hci_dev while processing > > > a HCI_EV_HARDWARE_ERROR event to avoid potential crash. > > > > > > Signed-off-by: Ying Hsu <yinghsu-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> > > > --- > > > Tested this commit on a chromebook with Intel BT controller. > > > > > > net/bluetooth/hci_core.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > > index 65601aa52e0d..a42417926028 100644 > > > --- a/net/bluetooth/hci_core.c > > > +++ b/net/bluetooth/hci_core.c > > > @@ -1049,6 +1049,7 @@ static void hci_error_reset(struct work_struct *work) > > > { > > > struct hci_dev *hdev = container_of(work, struct hci_dev, error_reset); > > > > > > + hci_dev_hold(hdev); > > > BT_DBG("%s", hdev->name); > > > > > > if (hdev->hw_error) > > > @@ -1060,6 +1061,7 @@ static void hci_error_reset(struct work_struct *work) > > > return; > > > > ^^^^^ > > Should we also call hci_dev_put() if we hit this return? > > Yep, I will fix that since Ive already pushed to bluetooth-next. Here is the proposed change: - if (hci_dev_do_close(hdev)) - return; + if (!hci_dev_do_close(hdev)) + hci_dev_do_open(hdev); - hci_dev_do_open(hdev); + hci_dev_put(hdev); > > > > CJ > > > > > > > > hci_dev_do_open(hdev); > > > + hci_dev_put(hdev); > > > } > > > > > > void hci_uuids_clear(struct hci_dev *hdev) > > > > > -- > Luiz Augusto von Dentz
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 65601aa52e0d..a42417926028 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1049,6 +1049,7 @@ static void hci_error_reset(struct work_struct *work) { struct hci_dev *hdev = container_of(work, struct hci_dev, error_reset); + hci_dev_hold(hdev); BT_DBG("%s", hdev->name); if (hdev->hw_error) @@ -1060,6 +1061,7 @@ static void hci_error_reset(struct work_struct *work) return; hci_dev_do_open(hdev); + hci_dev_put(hdev); } void hci_uuids_clear(struct hci_dev *hdev)
While handling the HCI_EV_HARDWARE_ERROR event, if the underlying BT controller is not responding, the GPIO reset mechanism would free the hci_dev and lead to a use-after-free in hci_error_reset. Here's the call trace observed on a ChromeOS device with Intel AX201: queue_work_on+0x3e/0x6c __hci_cmd_sync_sk+0x2ee/0x4c0 [bluetooth <HASH:3b4a6>] ? init_wait_entry+0x31/0x31 __hci_cmd_sync+0x16/0x20 [bluetooth <HASH:3b4a 6>] hci_error_reset+0x4f/0xa4 [bluetooth <HASH:3b4a 6>] process_one_work+0x1d8/0x33f worker_thread+0x21b/0x373 kthread+0x13a/0x152 ? pr_cont_work+0x54/0x54 ? kthread_blkcg+0x31/0x31 ret_from_fork+0x1f/0x30 This patch holds the reference count on the hci_dev while processing a HCI_EV_HARDWARE_ERROR event to avoid potential crash. Signed-off-by: Ying Hsu <yinghsu@chromium.org> --- Tested this commit on a chromebook with Intel BT controller. net/bluetooth/hci_core.c | 2 ++ 1 file changed, 2 insertions(+)