Re: [PATCH] Bluetooth: hci_sync: Avoid use-after-free in dbg for hci_remove_adv_monitor()

From: Doug Anderson
Date: Fri Jun 30 2023 - 18:31:30 EST


Hi,

On Fri, Jun 30, 2023 at 3:11 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> > > @@ -1980,9 +1981,10 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
> > > goto free_monitor;
> > >
> > > case HCI_ADV_MONITOR_EXT_MSFT:
> > > + handle = monitor->handle;
> > > status = msft_remove_monitor(hdev, monitor);
> > > bt_dev_dbg(hdev, "%s remove monitor %d msft status %d",
> > > - hdev->name, monitor->handle, status);
> > > + hdev->name, handle, status);
> >
> > Just move the call to bt_dev_dbg under msft_remove_monitor,
>
> Sure. I wasn't sure how much the order of the printout matters, but if
> it doesn't then just putting the print first makes sense. Done in v2.

So I assumed that this meant you just wanted me to switch the order,
which I did for v2. ...but then Manish pointed out that meant I wasn't
printing the right status.

Looking again, maybe you meant that I should move the debug statement
into the msft_remove_monitor(). I'm not convinced that's any cleaner.
That would mean adding an "exit" label to that function just for the
printout. It also makes the printout asymmetric with other similar
printouts.

I'm going back to v1 here. If I've misunderstood then I guess I can
always spin again. :-/

-Doug