Hi Pauli, Kuniyuki,
On Mon, Jun 16, 2025 at 2:12 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Hi,
>
> ma, 2025-06-16 kello 10:37 -0700, Kuniyuki Iwashima kirjoitti:
> > From: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
> >
> > syzbot reported use-after-free in vhci_flush() without repro. [0]
> >
> [clip]
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 07a8b4281a39..d648b514e2df 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -64,9 +64,9 @@ static DEFINE_IDA(hci_index_ida);
> >
> > /* Get HCI device by index.
> > * Device is held on return. */
> > -struct hci_dev *hci_dev_get(int index)
> > +static struct hci_dev *__hci_dev_get(int index, int *srcu_index)
> > {
> > - struct hci_dev *hdev = NULL, *d;
> > + struct hci_dev *hdev = NULL;
> >
> > BT_DBG("%d", index);
> >
> > @@ -74,9 +74,11 @@ struct hci_dev *hci_dev_get(int index)
> > return NULL;
> >
> > read_lock(&hci_dev_list_lock);
> > - list_for_each_entry(d, &hci_dev_list, list) {
> > - if (d->id == index) {
> > - hdev = hci_dev_hold(d);
> > + list_for_each_entry(hdev, &hci_dev_list, list) {
> > + if (hdev->id == index) {
> > + hci_dev_hold(hdev);
> > + if (srcu_index)
> > + *srcu_index = srcu_read_lock(&hdev->srcu);
> > break;
> > }
> > }
> > @@ -84,6 +86,22 @@ struct hci_dev *hci_dev_get(int index)
> > return hdev;
> > }
>
> If no list item has `hdev->id == index`, doesn't this now return the
> list head -> crash?
Seems wrong to me as well, *d was used to hold the current entry but
that has been removed so hdev would be used instead which may return a
valid/non-NULL entry even when its index doesn't match. Btw, are there
any documentation regarding the usage of SRCU in such cases where
there are still references?
Usually the hci_unregister_dev is called by the driver to inform the
hardware has been unplugged from the system, so we do want to be able
to abort any ongoing usage of the hci_dev so in this particular case
perhaps it is easier to just check if HCI_UNREGISTER has been set
before attempting to flush.
>
> --
> Pauli Virtanen
>
--
Luiz Augusto von Dentz
