2021-05-03 10:09:54

by Pavel Skripkin

[permalink] [raw]
Subject: [PATCH v2] bluetooth: hci_qca: fix potential GPF

In qca_power_shutdown() qcadev local variable is
initialized by hu->serdev.dev private data, but
hu->serdev can be NULL and there is a check for it.

Since, qcadev is not used before

if (!hu->serdev)
return;

we can move its initialization after this "if" to
prevent GPF.

Fixes: 5559904ccc08 ("Bluetooth: hci_qca: Add QCA Rome power off support to the qca_power_shutdown()")
Cc: [email protected] # v5.6+
Cc: Rocky Liao <[email protected]>
Signed-off-by: Pavel Skripkin <[email protected]>
---
drivers/bluetooth/hci_qca.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index de36af63e182..9589ef6c0c26 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1820,8 +1820,6 @@ static void qca_power_shutdown(struct hci_uart *hu)
unsigned long flags;
enum qca_btsoc_type soc_type = qca_soc_type(hu);

- qcadev = serdev_device_get_drvdata(hu->serdev);
-
/* 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
* data in skb's.
@@ -1837,6 +1835,8 @@ static void qca_power_shutdown(struct hci_uart *hu)
if (!hu->serdev)
return;

+ qcadev = serdev_device_get_drvdata(hu->serdev);
+
if (qca_is_wcn399x(soc_type)) {
host_set_baudrate(hu, 2400);
qca_send_power_pulse(hu, false);
--
2.31.1


2021-05-03 11:15:29

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2] bluetooth: hci_qca: fix potential GPF

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

---Test result---

##############################
Test: CheckPatch - FAIL
bluetooth: hci_qca: fix potential GPF
WARNING: Unknown commit id '5559904ccc08', maybe rebased or not pulled?
#18:
Fixes: 5559904ccc08 ("Bluetooth: hci_qca: Add QCA Rome power off support to the qca_power_shutdown()")

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.

"[PATCH] bluetooth: hci_qca: fix potential GPF" has style problems, please review.

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


##############################
Test: CheckGitLint - FAIL
bluetooth: hci_qca: fix potential GPF
9: B3 Line contains hard tab characters (\t): " if (!hu->serdev)"
10: B3 Line contains hard tab characters (\t): " return;"
15: B1 Line exceeds max length (102>80): "Fixes: 5559904ccc08 ("Bluetooth: hci_qca: Add QCA Rome power off support to the qca_power_shutdown()")"


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - PASS
Total: 416, Passed: 403 (96.9%), Failed: 0, Not Run: 13

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (43.20 kB)
bnep-tester.log (3.48 kB)
mgmt-tester.log (533.97 kB)
rfcomm-tester.log (11.41 kB)
sco-tester.log (9.68 kB)
smp-tester.log (11.55 kB)
userchan-tester.log (5.33 kB)
Download all attachments

2021-05-03 11:46:54

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH v2] bluetooth: hci_qca: fix potential GPF

On Mon, May 03, 2021 at 01:06:05PM +0300, Pavel Skripkin wrote:
> In qca_power_shutdown() qcadev local variable is
> initialized by hu->serdev.dev private data, but
> hu->serdev can be NULL and there is a check for it.
>
> Since, qcadev is not used before
>
> if (!hu->serdev)
> return;
>
> we can move its initialization after this "if" to
> prevent GPF.
>
> Fixes: 5559904ccc08 ("Bluetooth: hci_qca: Add QCA Rome power off support to the qca_power_shutdown()")
> Cc: [email protected] # v5.6+
> Cc: Rocky Liao <[email protected]>
> Signed-off-by: Pavel Skripkin <[email protected]>
> ---

Next time, put a changelog here so we know what changed since earlier
version(s).

Reviewed-by: Johan Hovold <[email protected]>

> drivers/bluetooth/hci_qca.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index de36af63e182..9589ef6c0c26 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1820,8 +1820,6 @@ static void qca_power_shutdown(struct hci_uart *hu)
> unsigned long flags;
> enum qca_btsoc_type soc_type = qca_soc_type(hu);
>
> - qcadev = serdev_device_get_drvdata(hu->serdev);
> -
> /* 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
> * data in skb's.
> @@ -1837,6 +1835,8 @@ static void qca_power_shutdown(struct hci_uart *hu)
> if (!hu->serdev)
> return;
>
> + qcadev = serdev_device_get_drvdata(hu->serdev);
> +
> if (qca_is_wcn399x(soc_type)) {
> host_set_baudrate(hu, 2400);
> qca_send_power_pulse(hu, false);