[PATCH] Bluetooth: hci_qca: Do not write to the serial port after it is closed

[PATCH] Bluetooth: hci_qca: Do not write to the serial port after it is closed

[Hans de Goede]

From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>

hci_uart_close() closes the serdev port if HCI_QUIRK_NON_PERSISTENT_SETUP
is set (for example, for the WCN399x family). A failed hci_dev_open_sync()
following a successful qca_setup() calls hdev->close() but not
hdev->shutdown(), so the port is closed while power->vregs_on is left true.
qca_serdev_remove() then passes its power->vregs_on test and calls
qca_power_off(), which writes to the closed port unconditionally.

Seen on a WCN3988 by unbinding the driver after a controller failure. The
trace below is from a 7.0.0 based kernel, where qca_power_off() was still
named qca_power_shutdown():

  Unable to handle kernel NULL pointer dereference at virtual address
  0000000000000038
  Call trace:
   tty_set_termios+0x50/0x238 (P)
   ttyport_set_baudrate+0x84/0xc0
   serdev_device_set_baudrate+0x24/0x40
   qca_power_shutdown+0x158/0x1fc [hci_uart]
   qca_serdev_remove+0x54/0x68 [hci_uart]
   serdev_drv_remove+0x1c/0x2c
   device_remove+0x4c/0x80
   device_release_driver_internal+0x1cc/0x224
   device_driver_detach+0x18/0x24
   unbind_store+0xb4/0xc0

Check HCI_UART_PROTO_READY, which hci_uart_close() clears in the same place
it closes the port, before writing to it. The regulator disable is left
unconditional so the controller is still powered down.

The dangling serport->tty that turns this into a use-after-free is
addressed in a separate patch.

Fixes: fa9ad876b8e0 ("Bluetooth: hci_qca: Add support for Qualcomm Bluetooth chip wcn3990")
Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/bluetooth/hci_qca.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index c4575036483a..0cee54949221 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2229,8 +2229,8 @@ static void qca_power_off(struct hci_uart *hu)
 	bool sw_ctrl_state;
 	struct qca_power *power;
 
-	/* From this point we go into power off state. But serial port is
-	 * still open, stop queueing the IBS data and flush all the buffered
+	/* From this point we go into power off state. But serial port may
+	 * still be open, stop queueing the IBS data and flush all the buffered
 	 * data in skb's.
 	 */
 	spin_lock_irqsave(&qca->hci_ibs_lock, flags);
@@ -2252,8 +2252,14 @@ static void qca_power_off(struct hci_uart *hu)
 	case QCA_WCN3990:
 	case QCA_WCN3991:
 	case QCA_WCN3998:
-		host_set_baudrate(hu, 2400);
-		qca_send_power_pulse(hu, false);
+		/* Both of these write to the serial port which may have
+		 * already been closed by hci_uart_close(), which closes
+		 * the port if HCI_QUIRK_NON_PERSISTENT_SETUP is set.
+		 */
+		if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+			host_set_baudrate(hu, 2400);
+			qca_send_power_pulse(hu, false);
+		}
 		break;
 	default:
 		break;
-- 
2.55.0

Re: [PATCH] Bluetooth: hci_qca: Do not write to the serial port after it is closed

[Hans de Goede]

Hi All,

On 19-Aug-26 14:54, Hans de Goede wrote:
> From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
> 
> hci_uart_close() closes the serdev port if HCI_QUIRK_NON_PERSISTENT_SETUP
> is set (for example, for the WCN399x family). A failed hci_dev_open_sync()
> following a successful qca_setup() calls hdev->close() but not
> hdev->shutdown(), so the port is closed while power->vregs_on is left true.
> qca_serdev_remove() then passes its power->vregs_on test and calls
> qca_power_off(), which writes to the closed port unconditionally.
> 
> Seen on a WCN3988 by unbinding the driver after a controller failure. The
> trace below is from a 7.0.0 based kernel, where qca_power_off() was still
> named qca_power_shutdown():
> 
>   Unable to handle kernel NULL pointer dereference at virtual address
>   0000000000000038
>   Call trace:
>    tty_set_termios+0x50/0x238 (P)
>    ttyport_set_baudrate+0x84/0xc0
>    serdev_device_set_baudrate+0x24/0x40
>    qca_power_shutdown+0x158/0x1fc [hci_uart]
>    qca_serdev_remove+0x54/0x68 [hci_uart]
>    serdev_drv_remove+0x1c/0x2c
>    device_remove+0x4c/0x80
>    device_release_driver_internal+0x1cc/0x224
>    device_driver_detach+0x18/0x24
>    unbind_store+0xb4/0xc0
> 
> Check HCI_UART_PROTO_READY, which hci_uart_close() clears in the same place
> it closes the port, before writing to it. The regulator disable is left
> unconditional so the controller is still powered down.
> 
> The dangling serport->tty that turns this into a use-after-free is
> addressed in a separate patch.

That separate patch can be found here:

https://lore.kernel.org/linux-bluetooth/20260819125748.194377-1-johannes.goede@oss.qualcomm.com/

Regards,

Hans

RE: Bluetooth: hci_qca: Do not write to the serial port after it is closed

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

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=1148470

---Test result---

Test Summary:
CheckPatch                    PASS      0.59 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       PASS      0.21 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      27.66 seconds
CheckAllWarning               PASS      30.19 seconds
CheckSparse                   PASS      28.77 seconds
BuildKernel32                 PASS      26.67 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      506.63 seconds
IncrementalBuild              PASS      25.77 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/616

---
Regards,
Linux Bluetooth