2022-12-29 10:33:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev

The driver shutdown callback (which sends EDL_SOC_RESET to the device
over serdev) should not be invoked when HCI device is not open (e.g. if
hci_dev_open_sync() failed), because the serdev and its TTY are not open
either. Also skip this step if device is powered off
(qca_power_shutdown()).

The shutdown callback causes use-after-free during system reboot with
Qualcomm Atheros Bluetooth:

Unable to handle kernel paging request at virtual address 0072662f67726fd7
...
CPU: 6 PID: 1 Comm: systemd-shutdow Tainted: G W 6.1.0-rt5-00325-g8a5f56bcfcca #8
Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
Call trace:
tty_driver_flush_buffer+0x4/0x30
serdev_device_write_flush+0x24/0x34
qca_serdev_shutdown+0x80/0x130 [hci_uart]
device_shutdown+0x15c/0x260
kernel_restart+0x48/0xac

KASAN report:

BUG: KASAN: use-after-free in tty_driver_flush_buffer+0x1c/0x50
Read of size 8 at addr ffff16270c2e0018 by task systemd-shutdow/1

CPU: 7 PID: 1 Comm: systemd-shutdow Not tainted 6.1.0-next-20221220-00014-gb85aaf97fb01-dirty #28
Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
Call trace:
dump_backtrace.part.0+0xdc/0xf0
show_stack+0x18/0x30
dump_stack_lvl+0x68/0x84
print_report+0x188/0x488
kasan_report+0xa4/0xf0
__asan_load8+0x80/0xac
tty_driver_flush_buffer+0x1c/0x50
ttyport_write_flush+0x34/0x44
serdev_device_write_flush+0x48/0x60
qca_serdev_shutdown+0x124/0x274
device_shutdown+0x1e8/0x350
kernel_restart+0x48/0xb0
__do_sys_reboot+0x244/0x2d0
__arm64_sys_reboot+0x54/0x70
invoke_syscall+0x60/0x190
el0_svc_common.constprop.0+0x7c/0x160
do_el0_svc+0x44/0xf0
el0_svc+0x2c/0x6c
el0t_64_sync_handler+0xbc/0x140
el0t_64_sync+0x190/0x194

Fixes: 7e7bbddd029b ("Bluetooth: hci_qca: Fix qca6390 enable failure after warm reboot")
Cc: <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Changes since v1:
1. Drop serdev patch and fix it only on BT side.
2. Update commit msg.
---
drivers/bluetooth/hci_qca.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index bb7623fe53a8..157fc4d024c1 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2166,10 +2166,16 @@ static void qca_serdev_shutdown(struct device *dev)
int timeout = msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS);
struct serdev_device *serdev = to_serdev_device(dev);
struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
+ struct hci_uart *hu = &qcadev->serdev_hu;
+ struct hci_dev *hdev = hu->hdev;
+ struct qca_data *qca = hu->priv;
const u8 ibs_wake_cmd[] = { 0xFD };
const u8 edl_reset_soc_cmd[] = { 0x01, 0x00, 0xFC, 0x01, 0x05 };

if (qcadev->btsoc_type == QCA_QCA6390) {
+ if (test_bit(QCA_BT_OFF, &qca->flags) || !test_bit(HCI_RUNNING, &hdev->flags))
+ return;
+
serdev_device_write_flush(serdev);
ret = serdev_device_write_buf(serdev, ibs_wake_cmd,
sizeof(ibs_wake_cmd));
--
2.34.1


2022-12-29 11:14:29

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev

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

---Test result---

Test Summary:
CheckPatch FAIL 1.02 seconds
GitLint FAIL 0.54 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 31.41 seconds
CheckAllWarning PASS 34.25 seconds
CheckSparse WARNING 38.73 seconds
CheckSmatch PASS 105.67 seconds
BuildKernel32 PASS 30.25 seconds
TestRunnerSetup PASS 437.49 seconds
TestRunner_l2cap-tester PASS 16.61 seconds
TestRunner_iso-tester PASS 17.11 seconds
TestRunner_bnep-tester PASS 5.75 seconds
TestRunner_mgmt-tester PASS 109.64 seconds
TestRunner_rfcomm-tester PASS 9.02 seconds
TestRunner_sco-tester PASS 8.39 seconds
TestRunner_ioctl-tester PASS 9.74 seconds
TestRunner_mesh-tester PASS 7.16 seconds
TestRunner_smp-tester PASS 8.16 seconds
TestRunner_userchan-tester PASS 6.01 seconds
IncrementalBuild PASS 28.30 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#93:
Unable to handle kernel paging request at virtual address 0072662f67726fd7

total: 0 errors, 1 warnings, 16 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13083411.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
14: B1 Line exceeds max length (99>80): " CPU: 6 PID: 1 Comm: systemd-shutdow Tainted: G W 6.1.0-rt5-00325-g8a5f56bcfcca #8"
28: B1 Line exceeds max length (99>80): " CPU: 7 PID: 1 Comm: systemd-shutdow Not tainted 6.1.0-next-20221220-00014-gb85aaf97fb01-dirty #28"
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
drivers/bluetooth/hci_qca.c:1014:26: warning: cast to restricted __le16drivers/bluetooth/hci_qca.c:1028:37: warning: cast to restricted __le32


---
Regards,
Linux Bluetooth

2023-01-12 15:32:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev

On 29/12/2022 11:28, Krzysztof Kozlowski wrote:
> The driver shutdown callback (which sends EDL_SOC_RESET to the device
> over serdev) should not be invoked when HCI device is not open (e.g. if
> hci_dev_open_sync() failed), because the serdev and its TTY are not open
> either. Also skip this step if device is powered off
> (qca_power_shutdown()).
>
> The shutdown callback causes use-after-free during system reboot with
> Qualcomm Atheros Bluetooth:
>
> Unable to handle kernel paging request at virtual address 0072662f67726fd7
> ...
> CPU: 6 PID: 1 Comm: systemd-shutdow Tainted: G W 6.1.0-rt5-00325-g8a5f56bcfcca #8
> Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> Call trace:
> tty_driver_flush_buffer+0x4/0x30
> serdev_device_write_flush+0x24/0x34
> qca_serdev_shutdown+0x80/0x130 [hci_uart]
> device_shutdown+0x15c/0x260
> kernel_restart+0x48/0xac
>
> KASAN report:
>
> BUG: KASAN: use-after-free in tty_driver_flush_buffer+0x1c/0x50
> Read of size 8 at addr ffff16270c2e0018 by task systemd-shutdow/1
>
> CPU: 7 PID: 1 Comm: systemd-shutdow Not tainted 6.1.0-next-20221220-00014-gb85aaf97fb01-dirty #28
> Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> Call trace:
> dump_backtrace.part.0+0xdc/0xf0
> show_stack+0x18/0x30
> dump_stack_lvl+0x68/0x84
> print_report+0x188/0x488
> kasan_report+0xa4/0xf0
> __asan_load8+0x80/0xac
> tty_driver_flush_buffer+0x1c/0x50
> ttyport_write_flush+0x34/0x44
> serdev_device_write_flush+0x48/0x60
> qca_serdev_shutdown+0x124/0x274
> device_shutdown+0x1e8/0x350
> kernel_restart+0x48/0xb0
> __do_sys_reboot+0x244/0x2d0
> __arm64_sys_reboot+0x54/0x70
> invoke_syscall+0x60/0x190
> el0_svc_common.constprop.0+0x7c/0x160
> do_el0_svc+0x44/0xf0
> el0_svc+0x2c/0x6c
> el0t_64_sync_handler+0xbc/0x140
> el0t_64_sync+0x190/0x194
>
> Fixes: 7e7bbddd029b ("Bluetooth: hci_qca: Fix qca6390 enable failure after warm reboot")
> Cc: <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> ---

Any comments on this? Patchwork tools complain on longer line, but
without it checkpatch would complain as well, so I assume you do not
expect to fix it?

Best regards,
Krzysztof

2023-01-12 16:16:49

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev

Hi Krzysztof,

On Thu, Jan 12, 2023 at 7:20 AM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 29/12/2022 11:28, Krzysztof Kozlowski wrote:
> > The driver shutdown callback (which sends EDL_SOC_RESET to the device
> > over serdev) should not be invoked when HCI device is not open (e.g. if
> > hci_dev_open_sync() failed), because the serdev and its TTY are not open
> > either. Also skip this step if device is powered off
> > (qca_power_shutdown()).
> >
> > The shutdown callback causes use-after-free during system reboot with
> > Qualcomm Atheros Bluetooth:
> >
> > Unable to handle kernel paging request at virtual address 0072662f67726fd7
> > ...
> > CPU: 6 PID: 1 Comm: systemd-shutdow Tainted: G W 6.1.0-rt5-00325-g8a5f56bcfcca #8
> > Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> > Call trace:
> > tty_driver_flush_buffer+0x4/0x30
> > serdev_device_write_flush+0x24/0x34
> > qca_serdev_shutdown+0x80/0x130 [hci_uart]
> > device_shutdown+0x15c/0x260
> > kernel_restart+0x48/0xac
> >
> > KASAN report:
> >
> > BUG: KASAN: use-after-free in tty_driver_flush_buffer+0x1c/0x50
> > Read of size 8 at addr ffff16270c2e0018 by task systemd-shutdow/1
> >
> > CPU: 7 PID: 1 Comm: systemd-shutdow Not tainted 6.1.0-next-20221220-00014-gb85aaf97fb01-dirty #28
> > Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
> > Call trace:
> > dump_backtrace.part.0+0xdc/0xf0
> > show_stack+0x18/0x30
> > dump_stack_lvl+0x68/0x84
> > print_report+0x188/0x488
> > kasan_report+0xa4/0xf0
> > __asan_load8+0x80/0xac
> > tty_driver_flush_buffer+0x1c/0x50
> > ttyport_write_flush+0x34/0x44
> > serdev_device_write_flush+0x48/0x60
> > qca_serdev_shutdown+0x124/0x274
> > device_shutdown+0x1e8/0x350
> > kernel_restart+0x48/0xb0
> > __do_sys_reboot+0x244/0x2d0
> > __arm64_sys_reboot+0x54/0x70
> > invoke_syscall+0x60/0x190
> > el0_svc_common.constprop.0+0x7c/0x160
> > do_el0_svc+0x44/0xf0
> > el0_svc+0x2c/0x6c
> > el0t_64_sync_handler+0xbc/0x140
> > el0t_64_sync+0x190/0x194
> >
> > Fixes: 7e7bbddd029b ("Bluetooth: hci_qca: Fix qca6390 enable failure after warm reboot")
> > Cc: <[email protected]>
> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >
> > ---
>
> Any comments on this? Patchwork tools complain on longer line, but
> without it checkpatch would complain as well, so I assume you do not
> expect to fix it?

I did apply it already:

https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=a18fca670e14f0c09c2ed332cf2c6d77a4ae05f9

Not sure why the bot hasn't responded to you.

> Best regards,
> Krzysztof
>


--
Luiz Augusto von Dentz

2023-01-12 16:17:47

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev

On 12/01/2023 17:07, Luiz Augusto von Dentz wrote:
>>> Fixes: 7e7bbddd029b ("Bluetooth: hci_qca: Fix qca6390 enable failure after warm reboot")
>>> Cc: <[email protected]>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>>
>>> ---
>>
>> Any comments on this? Patchwork tools complain on longer line, but
>> without it checkpatch would complain as well, so I assume you do not
>> expect to fix it?
>
> I did apply it already:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=a18fca670e14f0c09c2ed332cf2c6d77a4ae05f9
>
> Not sure why the bot hasn't responded to you.


Ah, thank you then. I looked at Patchwork and it was still in state "New".

Best regards,
Krzysztof