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

Hi,

On 19-Aug-26 18:25, Luiz Augusto von Dentz wrote:
> Hi Hans,
> 
> On Wed, Aug 19, 2026 at 8:59 AM Hans de Goede
> <[email protected]> wrote:
>>
>> Hi All,
>>
>> On 19-Aug-26 14:54, Hans de Goede wrote:
>>> From: Ibrahim Abdelkader <[email protected]>
>>>
>>> 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/[email protected]/
> 
> Hmm, I wonder if both should go via bluetooth tree?

This is just some hardening to turn the use-after-free fixed
in this patch into a NULL pointer deref. Since NULL pointer
derefs are better (easier detected for one) then a use-after-free.

There is no dependency between the 2 patches and they touch
different subsystems.

So having them separate seems better to me. But either way
works for me. If you want this to go through the bluetooth
tree and Greg is ok with that, that is fine too.

Regards,

Hans