linux-kernel - Re: [PATCH v2] Bluetooth: btusb: Add NULL check for data in btusb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABBYNZJJdCw=iP3ketY_fDt8HcKe6mAo9LFZY9i5oSxnF=HZ1Q@mail.gmail.com>
Date: Mon, 6 Jan 2025 15:58:03 -0500
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: En-Wei Wu <en-wei.wu@...onical.com>
Cc: marcel@...tmann.org, linux-bluetooth@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Markus.Elfring@....de
Subject: Re: [PATCH v2] Bluetooth: btusb: Add NULL check for data in btusb_suspend

Hi En-Wei,

On Mon, Dec 23, 2024 at 4:44 AM En-Wei Wu <en-wei.wu@...onical.com> wrote:
>
> When performing warm boot tests with an MT7920 device, we encounter
> NULL pointer dereferences with failure rate 5/30. The crash occurs
> during device suspend when btusb attempts to access data->hdev where
> data is NULL. This may happen due to a race condition between PM suspend
> and device disconnect. The root cause needs further investigation.

Ok, so we expect this to be backported or not? Because if you do then
we need to fix its root cause and not add a workaround.

>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> Workqueue: pm pm_runtime_work
> RIP: 0010:btusb_suspend+0x1d/0x1d0 [btusb]

This looks like probe hasn't finished yet, btusb_probe is still
pending/executing and usb_set_infdata hasn't been performed, but
suspend is called before that completes (?).

> Add a NULL check for data and return -ENODEV in this case to prevent
> the NULL pointer dereference. This indicates that the device is no
> longer available, which is appropriate when the driver's private data
> is missing.

Once we find the root cause then we need the Fixes tag to point out
where this regression was introduced.

> Signed-off-by: En-Wei Wu <en-wei.wu@...onical.com>
> ---
> Changes in v2:
> - Change "dereference" to "dereferences" in commit message
> - Re-flow commit message lines to use more characters per line
>
>  drivers/bluetooth/btusb.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 279fe6c115fa..a0461528548b 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -4096,6 +4096,9 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>
>         BT_DBG("intf %p", intf);
>
> +       if (!data)
> +               return -ENODEV;
> +
>         /* Don't auto-suspend if there are connections; external suspend calls
>          * shall never fail.
>          */
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz