All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev
@ 2022-12-29 10:28 Krzysztof Kozlowski
  2022-12-29 10:58 ` [v2] " bluez.test.bot
  2023-01-12 15:20 ` [PATCH v2] " Krzysztof Kozlowski
  0 siblings, 2 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-29 10:28 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz, Zijun Hu,
	linux-bluetooth, linux-kernel
  Cc: Krzysztof Kozlowski, stable

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: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev
  2022-12-29 10:28 [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev Krzysztof Kozlowski
@ 2022-12-29 10:58 ` bluez.test.bot
  2023-01-12 15:20 ` [PATCH v2] " Krzysztof Kozlowski
  1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2022-12-29 10:58 UTC (permalink / raw)
  To: linux-bluetooth, krzysztof.kozlowski

[-- Attachment #1: Type: text/plain, Size: 3229 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=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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev
  2022-12-29 10:28 [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev Krzysztof Kozlowski
  2022-12-29 10:58 ` [v2] " bluez.test.bot
@ 2023-01-12 15:20 ` Krzysztof Kozlowski
  2023-01-12 16:07   ` Luiz Augusto von Dentz
  1 sibling, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-12 15:20 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz, Zijun Hu,
	linux-bluetooth, linux-kernel
  Cc: stable

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: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev
  2023-01-12 15:20 ` [PATCH v2] " Krzysztof Kozlowski
@ 2023-01-12 16:07   ` Luiz Augusto von Dentz
  2023-01-12 16:13     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-01-12 16:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Marcel Holtmann, Johan Hedberg, Zijun Hu, linux-bluetooth,
	linux-kernel, stable

Hi Krzysztof,

On Thu, Jan 12, 2023 at 7:20 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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: <stable@vger.kernel.org>
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >
> > ---
>
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev
  2023-01-12 16:07   ` Luiz Augusto von Dentz
@ 2023-01-12 16:13     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-12 16:13 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, Zijun Hu, linux-bluetooth,
	linux-kernel, stable

On 12/01/2023 17:07, Luiz Augusto von Dentz wrote:
>>> Fixes: 7e7bbddd029b ("Bluetooth: hci_qca: Fix qca6390 enable failure after warm reboot")
>>> Cc: <stable@vger.kernel.org>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>
>>> ---
>>
>> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-12 16:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-29 10:28 [PATCH v2] Bluetooth: hci_qca: Fix driver shutdown on closed serdev Krzysztof Kozlowski
2022-12-29 10:58 ` [v2] " bluez.test.bot
2023-01-12 15:20 ` [PATCH v2] " Krzysztof Kozlowski
2023-01-12 16:07   ` Luiz Augusto von Dentz
2023-01-12 16:13     ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.