2024-03-19 15:46:35

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

Qualcomm Bluetooth controllers can be registered either from a serdev
driver or from the Bluetooth line discipline. In the latter case, the
HCI UART serdev pointer is NULL, something which the driver needs to
handle without crashing.

This series fixes one such issue at setup() time which incidentally
masked a similar crash at suspend. Fix this in two separate patches so
that the latter issue is address in pre-6.2 stable kernels.

Johan


Johan Hovold (2):
Bluetooth: qca: fix NULL-deref on non-serdev suspend
Bluetooth: qca: fix NULL-deref on non-serdev setup

drivers/bluetooth/hci_qca.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

--
2.43.2



2024-03-19 15:48:48

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: qca: fix NULL-deref on non-serdev suspend

Qualcomm ROME controllers can be registered from the Bluetooth line
discipline and in this case the HCI UART serdev pointer is NULL.

Add the missing sanity check to prevent a NULL-pointer dereference when
wakeup() is called for a non-serdev controller during suspend.

Just return true for now to restore the original behaviour and address
the crash with pre-6.2 kernels, which do not have commit e9b3e5b8c657
("Bluetooth: hci_qca: only assign wakeup with serial port support") that
causes the crash to happen already at setup() time.

Fixes: c1a74160eaf1 ("Bluetooth: hci_qca: Add device_may_wakeup support")
Cc: [email protected] # 5.13
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/bluetooth/hci_qca.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index c73481c57741..84f728943962 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1672,6 +1672,9 @@ static bool qca_wakeup(struct hci_dev *hdev)
struct hci_uart *hu = hci_get_drvdata(hdev);
bool wakeup;

+ if (!hu->serdev)
+ return true;
+
/* BT SoC attached through the serial bus is handled by the serdev driver.
* So we need to use the device handle of the serdev driver to get the
* status of device may wakeup.
--
2.43.2


2024-03-19 16:35:45

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: qca: fix NULL-deref on non-serdev setup

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

---Test result---

Test Summary:
CheckPatch PASS 1.37 seconds
GitLint PASS 0.68 seconds
SubjectPrefix PASS 0.26 seconds
BuildKernel PASS 28.52 seconds
CheckAllWarning PASS 31.44 seconds
CheckSparse PASS 37.17 seconds
CheckSmatch PASS 100.28 seconds
BuildKernel32 PASS 28.14 seconds
TestRunnerSetup PASS 519.52 seconds
TestRunner_l2cap-tester PASS 20.20 seconds
TestRunner_iso-tester PASS 30.52 seconds
TestRunner_bnep-tester PASS 4.81 seconds
TestRunner_mgmt-tester FAIL 108.33 seconds
TestRunner_rfcomm-tester PASS 7.41 seconds
TestRunner_sco-tester PASS 15.08 seconds
TestRunner_ioctl-tester PASS 7.89 seconds
TestRunner_mesh-tester PASS 5.91 seconds
TestRunner_smp-tester PASS 6.93 seconds
TestRunner_userchan-tester PASS 5.04 seconds
IncrementalBuild PASS 32.26 seconds

Details
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2

Failed Test Cases
LL Privacy - Add Device 7 (AL is full) Failed 0.202 seconds


---
Regards,
Linux Bluetooth

2024-04-22 13:05:29

by quic_zijuhu

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On 4/22/2024 8:51 PM, Johan Hovold wrote:
> Hi Luiz,
>
> On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
>> Qualcomm Bluetooth controllers can be registered either from a serdev
>> driver or from the Bluetooth line discipline. In the latter case, the
>> HCI UART serdev pointer is NULL, something which the driver needs to
>> handle without crashing.
>>
>> This series fixes one such issue at setup() time which incidentally
>> masked a similar crash at suspend. Fix this in two separate patches so
>> that the latter issue is address in pre-6.2 stable kernels.
>
>> Johan Hovold (2):
>> Bluetooth: qca: fix NULL-deref on non-serdev suspend
>> Bluetooth: qca: fix NULL-deref on non-serdev setup
>
> Could you pick these up for 6.9 or 6.10?
>
> The patches are marked for stable backport and only privileged users can
> set the N_HCI line discipline these days (even if I'm not sure about
> pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
>
> Johan
>
Hi johan,
could you share the patch links for me to review. i can
't find them now

2024-04-22 13:30:58

by quic_zijuhu

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On 4/22/2024 9:20 PM, Johan Hovold wrote:
> On Mon, Apr 22, 2024 at 09:04:58PM +0800, quic_zijuhu wrote:
>> On 4/22/2024 8:51 PM, Johan Hovold wrote:
>>> On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
>
>>>> Johan Hovold (2):
>>>> Bluetooth: qca: fix NULL-deref on non-serdev suspend
>>>> Bluetooth: qca: fix NULL-deref on non-serdev setup
>>>
>>> Could you pick these up for 6.9 or 6.10?
>>>
>>> The patches are marked for stable backport and only privileged users can
>>> set the N_HCI line discipline these days (even if I'm not sure about
>>> pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
>
>> could you share the patch links for me to review. i can
>> 't find them now
>
> Sure, but you should bookmark lore.kernel.org in your browser as you can
> search the archives there yourself:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> Johan
NAK for your [PATCH 1/2] since the null checking is redundant with your
[PATCH 2/2].
NAK for your [PATCH 2/2], since it is same with my earlier fix
https://lore.kernel.org/all/[email protected]/
my new patchset for btattach tool still has this change.



2024-04-22 13:43:43

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On Mon, Apr 22, 2024 at 09:30:28PM +0800, quic_zijuhu wrote:
> On 4/22/2024 9:20 PM, Johan Hovold wrote:
> > On Mon, Apr 22, 2024 at 09:04:58PM +0800, quic_zijuhu wrote:
> >> On 4/22/2024 8:51 PM, Johan Hovold wrote:
> >>> On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
> >
> >>>> Johan Hovold (2):
> >>>> Bluetooth: qca: fix NULL-deref on non-serdev suspend
> >>>> Bluetooth: qca: fix NULL-deref on non-serdev setup
> >>>
> >>> Could you pick these up for 6.9 or 6.10?
> >>>
> >>> The patches are marked for stable backport and only privileged users can
> >>> set the N_HCI line discipline these days (even if I'm not sure about
> >>> pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
> >
> >> could you share the patch links for me to review. i can
> >> 't find them now
> >
> > Sure, but you should bookmark lore.kernel.org in your browser as you can
> > search the archives there yourself:
> >
> > https://lore.kernel.org/lkml/[email protected]/

> NAK for your [PATCH 1/2] since the null checking is redundant with your
> [PATCH 2/2].

I explained in the cover letter why it is split up like this. If you
don't bother reading, then we will not bother listening to you.

> NAK for your [PATCH 2/2], since it is same with my earlier fix
> https://lore.kernel.org/all/[email protected]/
> my new patchset for btattach tool still has this change.

The fix does not depend on your btattach series, which has also been
rejected.

You clearly have some learning to do on how to interact with the kernel
community and to write proper commit messages and patches. If you start
listening to feedback and try not to piss everyone off perhaps you can
even get your patches merged one day. [1][2]

Johan

[1] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#m8e495666a71eb0e7ae54c82554dfff1fc96983e7
[2] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#med0610646a8fd8b3c8586abca9895b124b2d2534

2024-04-22 13:45:24

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

Hi Johan,

On Mon, Apr 22, 2024 at 9:20 AM Johan Hovold <[email protected]> wrote:
>
> On Mon, Apr 22, 2024 at 09:04:58PM +0800, quic_zijuhu wrote:
> > On 4/22/2024 8:51 PM, Johan Hovold wrote:
> > > On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
>
> > >> Johan Hovold (2):
> > >> Bluetooth: qca: fix NULL-deref on non-serdev suspend
> > >> Bluetooth: qca: fix NULL-deref on non-serdev setup
> > >
> > > Could you pick these up for 6.9 or 6.10?
> > >
> > > The patches are marked for stable backport and only privileged users can
> > > set the N_HCI line discipline these days (even if I'm not sure about
> > > pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
>
> > could you share the patch links for me to review. i can
> > 't find them now
>
> Sure, but you should bookmark lore.kernel.org in your browser as you can
> search the archives there yourself:
>
> https://lore.kernel.org/lkml/[email protected]/

Did you send these to linux-bluetooth? I don't see them in:

https://patchwork.kernel.org/project/bluetooth/list/


--
Luiz Augusto von Dentz

2024-04-22 13:53:35

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On Mon, Apr 22, 2024 at 09:44:59AM -0400, Luiz Augusto von Dentz wrote:
> On Mon, Apr 22, 2024 at 9:20 AM Johan Hovold <[email protected]> wrote:

> > https://lore.kernel.org/lkml/[email protected]/
>
> Did you send these to linux-bluetooth? I don't see them in:
>
> https://patchwork.kernel.org/project/bluetooth/list/

Yes, I did and they are in the linux-bluetooth archives on lore. Not
sure why they don't show up in the tracker.

Do you want me to resend?

Johan

2024-04-22 13:53:43

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

Hi Johan,

On Mon, Apr 22, 2024 at 9:51 AM Johan Hovold <[email protected]> wrote:
>
> On Mon, Apr 22, 2024 at 09:44:59AM -0400, Luiz Augusto von Dentz wrote:
> > On Mon, Apr 22, 2024 at 9:20 AM Johan Hovold <[email protected]> wrote:
>
> > > https://lore.kernel.org/lkml/[email protected]/
> >
> > Did you send these to linux-bluetooth? I don't see them in:
> >
> > https://patchwork.kernel.org/project/bluetooth/list/
>
> Yes, I did and they are in the linux-bluetooth archives on lore. Not
> sure why they don't show up in the tracker.
>
> Do you want me to resend?
>
> Johan

Yes, please resend them.


--
Luiz Augusto von Dentz

2024-04-22 13:54:09

by quic_zijuhu

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On 4/22/2024 9:43 PM, Johan Hovold wrote:
> On Mon, Apr 22, 2024 at 09:30:28PM +0800, quic_zijuhu wrote:
>> On 4/22/2024 9:20 PM, Johan Hovold wrote:
>>> On Mon, Apr 22, 2024 at 09:04:58PM +0800, quic_zijuhu wrote:
>>>> On 4/22/2024 8:51 PM, Johan Hovold wrote:
>>>>> On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
>>>
>>>>>> Johan Hovold (2):
>>>>>> Bluetooth: qca: fix NULL-deref on non-serdev suspend
>>>>>> Bluetooth: qca: fix NULL-deref on non-serdev setup
>>>>>
>>>>> Could you pick these up for 6.9 or 6.10?
>>>>>
>>>>> The patches are marked for stable backport and only privileged users can
>>>>> set the N_HCI line discipline these days (even if I'm not sure about
>>>>> pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
>>>
>>>> could you share the patch links for me to review. i can
>>>> 't find them now
>>>
>>> Sure, but you should bookmark lore.kernel.org in your browser as you can
>>> search the archives there yourself:
>>>
>>> https://lore.kernel.org/lkml/[email protected]/
>
>> NAK for your [PATCH 1/2] since the null checking is redundant with your
>> [PATCH 2/2].
>
> I explained in the cover letter why it is split up like this. If you
> don't bother reading, then we will not bother listening to you.
>
>> NAK for your [PATCH 2/2], since it is same with my earlier fix
>> https://lore.kernel.org/all/[email protected]/
>> my new patchset for btattach tool still has this change.
>
> The fix does not depend on your btattach series, which has also been
> rejected.
>
these my v1 and v2 for this issue which are earlier then yours.
they are not rejected but not responded.

https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/

> You clearly have some learning to do on how to interact with the kernel
> community and to write proper commit messages and patches. If you start
> listening to feedback and try not to piss everyone off perhaps you can
> even get your patches merged one day. [1][2]
>
> Johan
>
> [1] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#m8e495666a71eb0e7ae54c82554dfff1fc96983e7
> [2] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#med0610646a8fd8b3c8586abca9895b124b2d2534


2024-04-22 14:15:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

Hi Quic_zijuhu,

On Mon, Apr 22, 2024 at 9:53 AM quic_zijuhu <[email protected]> wrote:
>
> On 4/22/2024 9:43 PM, Johan Hovold wrote:
> > On Mon, Apr 22, 2024 at 09:30:28PM +0800, quic_zijuhu wrote:
> >> On 4/22/2024 9:20 PM, Johan Hovold wrote:
> >>> On Mon, Apr 22, 2024 at 09:04:58PM +0800, quic_zijuhu wrote:
> >>>> On 4/22/2024 8:51 PM, Johan Hovold wrote:
> >>>>> On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
> >>>
> >>>>>> Johan Hovold (2):
> >>>>>> Bluetooth: qca: fix NULL-deref on non-serdev suspend
> >>>>>> Bluetooth: qca: fix NULL-deref on non-serdev setup
> >>>>>
> >>>>> Could you pick these up for 6.9 or 6.10?
> >>>>>
> >>>>> The patches are marked for stable backport and only privileged users can
> >>>>> set the N_HCI line discipline these days (even if I'm not sure about
> >>>>> pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
> >>>
> >>>> could you share the patch links for me to review. i can
> >>>> 't find them now
> >>>
> >>> Sure, but you should bookmark lore.kernel.org in your browser as you can
> >>> search the archives there yourself:
> >>>
> >>> https://lore.kernel.org/lkml/[email protected]/
> >
> >> NAK for your [PATCH 1/2] since the null checking is redundant with your
> >> [PATCH 2/2].
> >
> > I explained in the cover letter why it is split up like this. If you
> > don't bother reading, then we will not bother listening to you.
> >
> >> NAK for your [PATCH 2/2], since it is same with my earlier fix
> >> https://lore.kernel.org/all/[email protected]/
> >> my new patchset for btattach tool still has this change.
> >
> > The fix does not depend on your btattach series, which has also been
> > rejected.
> >
> these my v1 and v2 for this issue which are earlier then yours.
> they are not rejected but not responded.
>
> https://lore.kernel.org/all/[email protected]/
> https://lore.kernel.org/all/[email protected]/
>
> > You clearly have some learning to do on how to interact with the kernel
> > community and to write proper commit messages and patches. If you start
> > listening to feedback and try not to piss everyone off perhaps you can
> > even get your patches merged one day. [1][2]
> >
> > Johan
> >
> > [1] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#m8e495666a71eb0e7ae54c82554dfff1fc96983e7
> > [2] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#med0610646a8fd8b3c8586abca9895b124b2d2534
>

They probably need to be resend as well, you have so many sets pending
that makes it hard to know which should go first, next time please
wait until each set is merged before sending the next since I can't
know if they are really independent of each other or not.

--
Luiz Augusto von Dentz

2024-04-22 14:23:33

by quic_zijuhu

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On 4/22/2024 10:15 PM, Luiz Augusto von Dentz wrote:
> Hi Quic_zijuhu,
>
> On Mon, Apr 22, 2024 at 9:53 AM quic_zijuhu <[email protected]> wrote:
>>
>> On 4/22/2024 9:43 PM, Johan Hovold wrote:
>>> On Mon, Apr 22, 2024 at 09:30:28PM +0800, quic_zijuhu wrote:
>>>> On 4/22/2024 9:20 PM, Johan Hovold wrote:
>>>>> On Mon, Apr 22, 2024 at 09:04:58PM +0800, quic_zijuhu wrote:
>>>>>> On 4/22/2024 8:51 PM, Johan Hovold wrote:
>>>>>>> On Tue, Mar 19, 2024 at 04:46:09PM +0100, Johan Hovold wrote:
>>>>>
>>>>>>>> Johan Hovold (2):
>>>>>>>> Bluetooth: qca: fix NULL-deref on non-serdev suspend
>>>>>>>> Bluetooth: qca: fix NULL-deref on non-serdev setup
>>>>>>>
>>>>>>> Could you pick these up for 6.9 or 6.10?
>>>>>>>
>>>>>>> The patches are marked for stable backport and only privileged users can
>>>>>>> set the N_HCI line discipline these days (even if I'm not sure about
>>>>>>> pre-5.14 kernels...) so it may be fine to wait for 6.10 if you prefer.
>>>>>
>>>>>> could you share the patch links for me to review. i can
>>>>>> 't find them now
>>>>>
>>>>> Sure, but you should bookmark lore.kernel.org in your browser as you can
>>>>> search the archives there yourself:
>>>>>
>>>>> https://lore.kernel.org/lkml/[email protected]/
>>>
>>>> NAK for your [PATCH 1/2] since the null checking is redundant with your
>>>> [PATCH 2/2].
>>>
>>> I explained in the cover letter why it is split up like this. If you
>>> don't bother reading, then we will not bother listening to you.
>>>
>>>> NAK for your [PATCH 2/2], since it is same with my earlier fix
>>>> https://lore.kernel.org/all/[email protected]/
>>>> my new patchset for btattach tool still has this change.
>>>
>>> The fix does not depend on your btattach series, which has also been
>>> rejected.
>>>
>> these my v1 and v2 for this issue which are earlier then yours.
>> they are not rejected but not responded.
>>
>> https://lore.kernel.org/all/[email protected]/
>> https://lore.kernel.org/all/[email protected]/
>>
>>> You clearly have some learning to do on how to interact with the kernel
>>> community and to write proper commit messages and patches. If you start
>>> listening to feedback and try not to piss everyone off perhaps you can
>>> even get your patches merged one day. [1][2]
>>>
>>> Johan
>>>
>>> [1] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#m8e495666a71eb0e7ae54c82554dfff1fc96983e7
>>> [2] https://lore.kernel.org/linux-bluetooth/[email protected]/T/#med0610646a8fd8b3c8586abca9895b124b2d2534
>>
>
> They probably need to be resend as well, you have so many sets pending
> that makes it hard to know which should go first, next time please
> wait until each set is merged before sending the next since I can't
> know if they are really independent of each other or not.
>
okay. let me also provide a list of patches required for BT maintainers
in another thread.


2024-04-22 15:19:33

by quic_zijuhu

[permalink] [raw]
Subject: Re: [PATCH 0/2] Bluetooth: qca: fix NULL-deref on non-serdev setup

On 4/22/2024 11:02 PM, Johan Hovold wrote:
> On Mon, Apr 22, 2024 at 09:53:48PM +0800, quic_zijuhu wrote:
>> On 4/22/2024 9:43 PM, Johan Hovold wrote:
>>> On Mon, Apr 22, 2024 at 09:30:28PM +0800, quic_zijuhu wrote:
>>>> On 4/22/2024 9:20 PM, Johan Hovold wrote:
>
>>>> NAK for your [PATCH 1/2] since the null checking is redundant with your
>>>> [PATCH 2/2].
>>>
>>> I explained in the cover letter why it is split up like this. If you
>>> don't bother reading, then we will not bother listening to you.
>>>
>>>> NAK for your [PATCH 2/2], since it is same with my earlier fix
>>>> https://lore.kernel.org/all/[email protected]/
>>>> my new patchset for btattach tool still has this change.
>>>
>>> The fix does not depend on your btattach series, which has also been
>>> rejected.
>>>
>> these my v1 and v2 for this issue which are earlier then yours.
>> they are not rejected but not responded.
>>
>> https://lore.kernel.org/all/[email protected]/
>> https://lore.kernel.org/all/[email protected]/
>
> Here is your *v3* as part of the rejected btattach series:
>
> https://lore.kernel.org/all/[email protected]/
>
no, v3 is a separate patch here and not was rejected.
https://lore.kernel.org/all/[email protected]/
and it was sent after your patch.

then i included the fix into a patch serials for tool btattach shown by
below link
https://lore.kernel.org/all/[email protected]/
you reviewed my the serials and also did not rejected the fix.
and it also doesn't get any negative comments.

> Apparently you had earlier also sent it separately, I see now in lore.
> It's all a big mess.
>
>>> You clearly have some learning to do on how to interact with the kernel
>>> community and to write proper commit messages and patches. If you start
>>> listening to feedback and try not to piss everyone off perhaps you can
>>> even get your patches merged one day. [1][2]
>
> Johan