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@xxxxxxx> 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
