Is commit 4d94f0555827 safe?

Is commit 4d94f0555827 safe?

[Takashi Iwai]

[ I posted without the subject line mistakenly, so resent again;
  sorry if you have seen already read it ]

Hi Luiz,

due to the CVE assignment, I stumbled on the recent fix for BT
hci_core, the commit 4d94f0555827 ("Bluetooth: hci_core: Fix sleeping
function called from invalid context"), and wonder whether it's really
safe.

As already asked question at the patch review:
  https://patchwork.kernel.org/comment/26147087/
the code allows the callbacks to be called even after
hci_unregister_cb() returns.

Your assumption was that it's never called without the module removal,
but isn't hci_unregister_cb() also called from iso_exit() which can be
triggered via set_iso_socket_func() in mgmt.c?  Also, any 3rd party
module could call hci_unregister_cb() in a wild way, too -- even if
the function still remains, it doesn't mean that you can call it
safely if the caller already assumes it being unregistered.

In addition to that, I feel what the patch does as a bit too
heavy-lifting: it does kmalloc() and copy the whole hci_cb object,
which isn't quite small for each.  If the callback is still safe to
call after RCU protection, you may just keep the hci_cb pointer
instead of copying the whole content, too?

I couldn't find v1 patch in the patchwork, so not sure whether this
has been already discussed.  If so, let me know.


Thanks!

Takashi

Re: Is commit 4d94f0555827 safe?

[Luiz Augusto von Dentz]

Hi Takashi,

Well the assumption was that because we are doing a copy of the struct
being unregistered/freed would never cause any errors, so to trigger
something like UAF like the comment was suggesting the function
callback would need to be unmapped so even if the likes of iso_exit is
called it function (e.g. iso_connect_cfm) remains in memory.

You can find the previous version here:

https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000

Problem with it was that it is invalid to unlock and relock like that.

On Sun, Mar 2, 2025 at 4:59 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> [ I posted without the subject line mistakenly, so resent again;
>   sorry if you have seen already read it ]
>
> Hi Luiz,
>
> due to the CVE assignment, I stumbled on the recent fix for BT
> hci_core, the commit 4d94f0555827 ("Bluetooth: hci_core: Fix sleeping
> function called from invalid context"), and wonder whether it's really
> safe.
>
> As already asked question at the patch review:
>   https://patchwork.kernel.org/comment/26147087/
> the code allows the callbacks to be called even after
> hci_unregister_cb() returns.
>
> Your assumption was that it's never called without the module removal,
> but isn't hci_unregister_cb() also called from iso_exit() which can be
> triggered via set_iso_socket_func() in mgmt.c?  Also, any 3rd party
> module could call hci_unregister_cb() in a wild way, too -- even if
> the function still remains, it doesn't mean that you can call it
> safely if the caller already assumes it being unregistered.
>
> In addition to that, I feel what the patch does as a bit too
> heavy-lifting: it does kmalloc() and copy the whole hci_cb object,
> which isn't quite small for each.  If the callback is still safe to
> call after RCU protection, you may just keep the hci_cb pointer
> instead of copying the whole content, too?
>
> I couldn't find v1 patch in the patchwork, so not sure whether this
> has been already discussed.  If so, let me know.
>
>
> Thanks!
>
> Takashi
>


-- 
Luiz Augusto von Dentz

Re: Is commit 4d94f0555827 safe?

[Takashi Iwai]

On Mon, 03 Mar 2025 15:57:16 +0100,
Luiz Augusto von Dentz wrote:
> 
> Hi Takashi,
> 
> Well the assumption was that because we are doing a copy of the struct
> being unregistered/freed would never cause any errors, so to trigger
> something like UAF like the comment was suggesting the function
> callback would need to be unmapped so even if the likes of iso_exit is
> called it function (e.g. iso_connect_cfm) remains in memory.

But it doesn't guarantee that the callback function would really
work.  e.g. if the callback accesses some memory that was immediately
freed after the unregister call, it will lead to a UAF, even though
the function itself is still present on the memory.

That said, the current situation makes hard to judge the object life
time.

> You can find the previous version here:
> 
> https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> 
> Problem with it was that it is invalid to unlock and relock like that.

Thanks for the pointer!


BTW, I saw another patch posted to replace the mutex with spinlock
(and you replied later on that it's been already fixed).
Is it an acceptable approach at all?


Takashi

> 
> On Sun, Mar 2, 2025 at 4:59 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > [ I posted without the subject line mistakenly, so resent again;
> >   sorry if you have seen already read it ]
> >
> > Hi Luiz,
> >
> > due to the CVE assignment, I stumbled on the recent fix for BT
> > hci_core, the commit 4d94f0555827 ("Bluetooth: hci_core: Fix sleeping
> > function called from invalid context"), and wonder whether it's really
> > safe.
> >
> > As already asked question at the patch review:
> >   https://patchwork.kernel.org/comment/26147087/
> > the code allows the callbacks to be called even after
> > hci_unregister_cb() returns.
> >
> > Your assumption was that it's never called without the module removal,
> > but isn't hci_unregister_cb() also called from iso_exit() which can be
> > triggered via set_iso_socket_func() in mgmt.c?  Also, any 3rd party
> > module could call hci_unregister_cb() in a wild way, too -- even if
> > the function still remains, it doesn't mean that you can call it
> > safely if the caller already assumes it being unregistered.
> >
> > In addition to that, I feel what the patch does as a bit too
> > heavy-lifting: it does kmalloc() and copy the whole hci_cb object,
> > which isn't quite small for each.  If the callback is still safe to
> > call after RCU protection, you may just keep the hci_cb pointer
> > instead of copying the whole content, too?
> >
> > I couldn't find v1 patch in the patchwork, so not sure whether this
> > has been already discussed.  If so, let me know.
> >
> >
> > Thanks!
> >
> > Takashi
> >
> 
> 
> -- 
> Luiz Augusto von Dentz

Re: Is commit 4d94f0555827 safe?

[Luiz Augusto von Dentz]

Hi Takashi,

On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Mon, 03 Mar 2025 15:57:16 +0100,
> Luiz Augusto von Dentz wrote:
> >
> > Hi Takashi,
> >
> > Well the assumption was that because we are doing a copy of the struct
> > being unregistered/freed would never cause any errors, so to trigger
> > something like UAF like the comment was suggesting the function
> > callback would need to be unmapped so even if the likes of iso_exit is
> > called it function (e.g. iso_connect_cfm) remains in memory.
>
> But it doesn't guarantee that the callback function would really
> work.  e.g. if the callback accesses some memory that was immediately
> freed after the unregister call, it will lead to a UAF, even though
> the function itself is still present on the memory.
>
> That said, the current situation makes hard to judge the object life
> time.
>
> > You can find the previous version here:
> >
> > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> >
> > Problem with it was that it is invalid to unlock and relock like that.
>
> Thanks for the pointer!
>
>
> BTW, I saw another patch posted to replace the mutex with spinlock
> (and you replied later on that it's been already fixed).
> Is it an acceptable approach at all?

I don't remember if I saw that, but yeah anything that makes the issue
go away, and doesn't create new problems, would probably be
acceptable.

Re: Is commit 4d94f0555827 safe?

[Takashi Iwai]

On Mon, 03 Mar 2025 16:50:37 +0100,
Luiz Augusto von Dentz wrote:
> 
> Hi Takashi,
> 
> On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Mon, 03 Mar 2025 15:57:16 +0100,
> > Luiz Augusto von Dentz wrote:
> > >
> > > Hi Takashi,
> > >
> > > Well the assumption was that because we are doing a copy of the struct
> > > being unregistered/freed would never cause any errors, so to trigger
> > > something like UAF like the comment was suggesting the function
> > > callback would need to be unmapped so even if the likes of iso_exit is
> > > called it function (e.g. iso_connect_cfm) remains in memory.
> >
> > But it doesn't guarantee that the callback function would really
> > work.  e.g. if the callback accesses some memory that was immediately
> > freed after the unregister call, it will lead to a UAF, even though
> > the function itself is still present on the memory.
> >
> > That said, the current situation makes hard to judge the object life
> > time.
> >
> > > You can find the previous version here:
> > >
> > > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> > >
> > > Problem with it was that it is invalid to unlock and relock like that.
> >
> > Thanks for the pointer!
> >
> >
> > BTW, I saw another patch posted to replace the mutex with spinlock
> > (and you replied later on that it's been already fixed).
> > Is it an acceptable approach at all?
> 
> I don't remember if I saw that, but yeah anything that makes the issue
> go away, and doesn't create new problems, would probably be
> acceptable.

I saw this one:
  https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/


Takashi

Re: Is commit 4d94f0555827 safe?

[Luiz Augusto von Dentz]

Hi,

On Mon, Mar 3, 2025 at 10:56 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Mon, 03 Mar 2025 16:50:37 +0100,
> Luiz Augusto von Dentz wrote:
> >
> > Hi Takashi,
> >
> > On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > On Mon, 03 Mar 2025 15:57:16 +0100,
> > > Luiz Augusto von Dentz wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > Well the assumption was that because we are doing a copy of the struct
> > > > being unregistered/freed would never cause any errors, so to trigger
> > > > something like UAF like the comment was suggesting the function
> > > > callback would need to be unmapped so even if the likes of iso_exit is
> > > > called it function (e.g. iso_connect_cfm) remains in memory.
> > >
> > > But it doesn't guarantee that the callback function would really
> > > work.  e.g. if the callback accesses some memory that was immediately
> > > freed after the unregister call, it will lead to a UAF, even though
> > > the function itself is still present on the memory.
> > >
> > > That said, the current situation makes hard to judge the object life
> > > time.
> > >
> > > > You can find the previous version here:
> > > >
> > > > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> > > >
> > > > Problem with it was that it is invalid to unlock and relock like that.
> > >
> > > Thanks for the pointer!
> > >
> > >
> > > BTW, I saw another patch posted to replace the mutex with spinlock
> > > (and you replied later on that it's been already fixed).
> > > Is it an acceptable approach at all?
> >
> > I don't remember if I saw that, but yeah anything that makes the issue
> > go away, and doesn't create new problems, would probably be
> > acceptable.
>
> I saw this one:
>   https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/

Ive might have missed it, we will probably need to rebase it but other
than that it should be acceptable.

-- 
Luiz Augusto von Dentz

Re: Is commit 4d94f0555827 safe?

[Takashi Iwai]

On Mon, 03 Mar 2025 17:29:58 +0100,
Luiz Augusto von Dentz wrote:
> 
> Hi,
> 
> On Mon, Mar 3, 2025 at 10:56 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Mon, 03 Mar 2025 16:50:37 +0100,
> > Luiz Augusto von Dentz wrote:
> > >
> > > Hi Takashi,
> > >
> > > On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > >
> > > > On Mon, 03 Mar 2025 15:57:16 +0100,
> > > > Luiz Augusto von Dentz wrote:
> > > > >
> > > > > Hi Takashi,
> > > > >
> > > > > Well the assumption was that because we are doing a copy of the struct
> > > > > being unregistered/freed would never cause any errors, so to trigger
> > > > > something like UAF like the comment was suggesting the function
> > > > > callback would need to be unmapped so even if the likes of iso_exit is
> > > > > called it function (e.g. iso_connect_cfm) remains in memory.
> > > >
> > > > But it doesn't guarantee that the callback function would really
> > > > work.  e.g. if the callback accesses some memory that was immediately
> > > > freed after the unregister call, it will lead to a UAF, even though
> > > > the function itself is still present on the memory.
> > > >
> > > > That said, the current situation makes hard to judge the object life
> > > > time.
> > > >
> > > > > You can find the previous version here:
> > > > >
> > > > > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> > > > >
> > > > > Problem with it was that it is invalid to unlock and relock like that.
> > > >
> > > > Thanks for the pointer!
> > > >
> > > >
> > > > BTW, I saw another patch posted to replace the mutex with spinlock
> > > > (and you replied later on that it's been already fixed).
> > > > Is it an acceptable approach at all?
> > >
> > > I don't remember if I saw that, but yeah anything that makes the issue
> > > go away, and doesn't create new problems, would probably be
> > > acceptable.
> >
> > I saw this one:
> >   https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/
> 
> Ive might have missed it, we will probably need to rebase it but other
> than that it should be acceptable.

Does it mean that you'd revert the change and apply the above one
(with rebase or modification)?  Or would you keep a part of the
current change (e.g. match callback looks neat) while applying the
similar fix using the spinlock?


thanks,

Takashi

Re: Is commit 4d94f0555827 safe?

[Pauli Virtanen]

Hi,

ma, 2025-03-03 kello 17:38 +0100, Takashi Iwai kirjoitti:
> On Mon, 03 Mar 2025 17:29:58 +0100,
> Luiz Augusto von Dentz wrote:
> > 
> > Hi,
> > 
> > On Mon, Mar 3, 2025 at 10:56 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > 
> > > On Mon, 03 Mar 2025 16:50:37 +0100,
> > > Luiz Augusto von Dentz wrote:
> > > > 
> > > > Hi Takashi,
> > > > 
> > > > On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > > > 
> > > > > On Mon, 03 Mar 2025 15:57:16 +0100,
> > > > > Luiz Augusto von Dentz wrote:
> > > > > > 
> > > > > > Hi Takashi,
> > > > > > 
> > > > > > Well the assumption was that because we are doing a copy of the struct
> > > > > > being unregistered/freed would never cause any errors, so to trigger
> > > > > > something like UAF like the comment was suggesting the function
> > > > > > callback would need to be unmapped so even if the likes of iso_exit is
> > > > > > called it function (e.g. iso_connect_cfm) remains in memory.
> > > > > 
> > > > > But it doesn't guarantee that the callback function would really
> > > > > work.  e.g. if the callback accesses some memory that was immediately
> > > > > freed after the unregister call, it will lead to a UAF, even though
> > > > > the function itself is still present on the memory.
> > > > > 
> > > > > That said, the current situation makes hard to judge the object life
> > > > > time.
> > > > > 
> > > > > > You can find the previous version here:
> > > > > > 
> > > > > > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> > > > > > 
> > > > > > Problem with it was that it is invalid to unlock and relock like that.
> > > > > 
> > > > > Thanks for the pointer!
> > > > > 
> > > > > 
> > > > > BTW, I saw another patch posted to replace the mutex with spinlock
> > > > > (and you replied later on that it's been already fixed).
> > > > > Is it an acceptable approach at all?
> > > > 
> > > > I don't remember if I saw that, but yeah anything that makes the issue
> > > > go away, and doesn't create new problems, would probably be
> > > > acceptable.
> > > 
> > > I saw this one:
> > >   https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/
> > 
> > Ive might have missed it, we will probably need to rebase it but other
> > than that it should be acceptable.
> 
> Does it mean that you'd revert the change and apply the above one
> (with rebase or modification)?  Or would you keep a part of the
> current change (e.g. match callback looks neat) while applying the
> similar fix using the spinlock?

My current understanding of this is that the actual problem for
4d94f0555827 was incorrect RCU use at the callsite in
hci_le_create_big_complete_evt(). That part was rewritten in

commit 581dd2dc168f ("Bluetooth: hci_event: Fix using rcu_read_(un)lock
while iterating")

and now it no longer calls hci_connect_cfm() from atomic context. I
suspect there were no other callsites that needed hci callbacks be rcu-
safe, so the original mutex is maybe OK as well.

For the other patch 

https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/

The current code is doing rcu_read_unlock() in list_for_each_entry_rcu,
so it's not quite correct. This could be reorganized to restart the
loop after unlock and skip if (conn->abort_reason), which may be
preferable to spinlock in rcu critical section.

-- 
Pauli Virtanen

Re: Is commit 4d94f0555827 safe?

[Luiz Augusto von Dentz]

Hi Pauli, Takashi,

On Mon, Mar 3, 2025 at 12:47 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi,
>
> ma, 2025-03-03 kello 17:38 +0100, Takashi Iwai kirjoitti:
> > On Mon, 03 Mar 2025 17:29:58 +0100,
> > Luiz Augusto von Dentz wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Mar 3, 2025 at 10:56 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > >
> > > > On Mon, 03 Mar 2025 16:50:37 +0100,
> > > > Luiz Augusto von Dentz wrote:
> > > > >
> > > > > Hi Takashi,
> > > > >
> > > > > On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > > > >
> > > > > > On Mon, 03 Mar 2025 15:57:16 +0100,
> > > > > > Luiz Augusto von Dentz wrote:
> > > > > > >
> > > > > > > Hi Takashi,
> > > > > > >
> > > > > > > Well the assumption was that because we are doing a copy of the struct
> > > > > > > being unregistered/freed would never cause any errors, so to trigger
> > > > > > > something like UAF like the comment was suggesting the function
> > > > > > > callback would need to be unmapped so even if the likes of iso_exit is
> > > > > > > called it function (e.g. iso_connect_cfm) remains in memory.
> > > > > >
> > > > > > But it doesn't guarantee that the callback function would really
> > > > > > work.  e.g. if the callback accesses some memory that was immediately
> > > > > > freed after the unregister call, it will lead to a UAF, even though
> > > > > > the function itself is still present on the memory.
> > > > > >
> > > > > > That said, the current situation makes hard to judge the object life
> > > > > > time.
> > > > > >
> > > > > > > You can find the previous version here:
> > > > > > >
> > > > > > > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> > > > > > >
> > > > > > > Problem with it was that it is invalid to unlock and relock like that.
> > > > > >
> > > > > > Thanks for the pointer!
> > > > > >
> > > > > >
> > > > > > BTW, I saw another patch posted to replace the mutex with spinlock
> > > > > > (and you replied later on that it's been already fixed).
> > > > > > Is it an acceptable approach at all?
> > > > >
> > > > > I don't remember if I saw that, but yeah anything that makes the issue
> > > > > go away, and doesn't create new problems, would probably be
> > > > > acceptable.
> > > >
> > > > I saw this one:
> > > >   https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/
> > >
> > > Ive might have missed it, we will probably need to rebase it but other
> > > than that it should be acceptable.
> >
> > Does it mean that you'd revert the change and apply the above one
> > (with rebase or modification)?  Or would you keep a part of the
> > current change (e.g. match callback looks neat) while applying the
> > similar fix using the spinlock?
>
> My current understanding of this is that the actual problem for
> 4d94f0555827 was incorrect RCU use at the callsite in
> hci_le_create_big_complete_evt(). That part was rewritten in
>
> commit 581dd2dc168f ("Bluetooth: hci_event: Fix using rcu_read_(un)lock
> while iterating")

In that case maybe we can just revert the 4d94f0555827 ("Bluetooth:
hci_core: Fix sleeping
function called from invalid context") and see if that works, might
need to trigger syzbot just to confirm we don't introduce the original
problem.

> and now it no longer calls hci_connect_cfm() from atomic context. I
> suspect there were no other callsites that needed hci callbacks be rcu-
> safe, so the original mutex is maybe OK as well.
>
> For the other patch
>
> https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/
>
> The current code is doing rcu_read_unlock() in list_for_each_entry_rcu,
> so it's not quite correct. This could be reorganized to restart the
> loop after unlock and skip if (conn->abort_reason), which may be
> preferable to spinlock in rcu critical section.

I guess you referring to hci_cmd_sync_clear, yeah that is doing
unlock/lock in the loop which is incorrect, that said we don't really
need this change if for the above issue, although we need to
re-evaluate the use of mutex vs spin locks given that apparently there
could be issues other than just the BIG code hitting this sort of
problem.

> --
> Pauli Virtanen



-- 
Luiz Augusto von Dentz

Re: Is commit 4d94f0555827 safe?

[Takashi Iwai]

On Mon, 03 Mar 2025 19:19:47 +0100,
Luiz Augusto von Dentz wrote:
> 
> Hi Pauli, Takashi,
> 
> On Mon, Mar 3, 2025 at 12:47 PM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > Hi,
> >
> > ma, 2025-03-03 kello 17:38 +0100, Takashi Iwai kirjoitti:
> > > On Mon, 03 Mar 2025 17:29:58 +0100,
> > > Luiz Augusto von Dentz wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Mon, Mar 3, 2025 at 10:56 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > > >
> > > > > On Mon, 03 Mar 2025 16:50:37 +0100,
> > > > > Luiz Augusto von Dentz wrote:
> > > > > >
> > > > > > Hi Takashi,
> > > > > >
> > > > > > On Mon, Mar 3, 2025 at 10:10 AM Takashi Iwai <tiwai@suse.de> wrote:
> > > > > > >
> > > > > > > On Mon, 03 Mar 2025 15:57:16 +0100,
> > > > > > > Luiz Augusto von Dentz wrote:
> > > > > > > >
> > > > > > > > Hi Takashi,
> > > > > > > >
> > > > > > > > Well the assumption was that because we are doing a copy of the struct
> > > > > > > > being unregistered/freed would never cause any errors, so to trigger
> > > > > > > > something like UAF like the comment was suggesting the function
> > > > > > > > callback would need to be unmapped so even if the likes of iso_exit is
> > > > > > > > called it function (e.g. iso_connect_cfm) remains in memory.
> > > > > > >
> > > > > > > But it doesn't guarantee that the callback function would really
> > > > > > > work.  e.g. if the callback accesses some memory that was immediately
> > > > > > > freed after the unregister call, it will lead to a UAF, even though
> > > > > > > the function itself is still present on the memory.
> > > > > > >
> > > > > > > That said, the current situation makes hard to judge the object life
> > > > > > > time.
> > > > > > >
> > > > > > > > You can find the previous version here:
> > > > > > > >
> > > > > > > > https://syzkaller.appspot.com/text?tag=Patch&x=100c0de8580000
> > > > > > > >
> > > > > > > > Problem with it was that it is invalid to unlock and relock like that.
> > > > > > >
> > > > > > > Thanks for the pointer!
> > > > > > >
> > > > > > >
> > > > > > > BTW, I saw another patch posted to replace the mutex with spinlock
> > > > > > > (and you replied later on that it's been already fixed).
> > > > > > > Is it an acceptable approach at all?
> > > > > >
> > > > > > I don't remember if I saw that, but yeah anything that makes the issue
> > > > > > go away, and doesn't create new problems, would probably be
> > > > > > acceptable.
> > > > >
> > > > > I saw this one:
> > > > >   https://lore.kernel.org/all/20230907122234.146449-1-william.xuanziyang@huawei.com/
> > > >
> > > > Ive might have missed it, we will probably need to rebase it but other
> > > > than that it should be acceptable.
> > >
> > > Does it mean that you'd revert the change and apply the above one
> > > (with rebase or modification)?  Or would you keep a part of the
> > > current change (e.g. match callback looks neat) while applying the
> > > similar fix using the spinlock?
> >
> > My current understanding of this is that the actual problem for
> > 4d94f0555827 was incorrect RCU use at the callsite in
> > hci_le_create_big_complete_evt(). That part was rewritten in
> >
> > commit 581dd2dc168f ("Bluetooth: hci_event: Fix using rcu_read_(un)lock
> > while iterating")
> 
> In that case maybe we can just revert the 4d94f0555827 ("Bluetooth:
> hci_core: Fix sleeping
> function called from invalid context") and see if that works, might
> need to trigger syzbot just to confirm we don't introduce the original
> problem.

Fair enough, it sounds reasonable.


thanks,

Takashi