Re: [PATCH v1 1/2] Bluetooth: btnxpuart: Fix btnxpuart_close

[Neeraj sanjay kale]

Hi Marcel

> > > From: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>
> > >
> > > Unfortunately, btnxpuart_close() may trigger a BUG: scheduling while
> atomic.
> > > Fix this by properly purging the transmit queue and freeing the receive
> skb.
> > >
> > > Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP
> > > Bluetooth chipsets")
> > >
> > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@xxxxxxxxxxx>
> > > ---
> > >
> > >  drivers/bluetooth/btnxpuart.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/bluetooth/btnxpuart.c
> > > b/drivers/bluetooth/btnxpuart.c index b7e66b7ac570..9cb7529eef09
> > > 100644
> > > --- a/drivers/bluetooth/btnxpuart.c
> > > +++ b/drivers/bluetooth/btnxpuart.c
> > > @@ -1234,6 +1234,9 @@ static int btnxpuart_close(struct hci_dev
> > > *hdev)
> > >
> > >         ps_wakeup(nxpdev);
> > >         serdev_device_close(nxpdev->serdev);
> > > +       skb_queue_purge(&nxpdev->txq);
> > > +       kfree_skb(nxpdev->rx_skb);
> > > +       nxpdev->rx_skb = NULL;
> > >         clear_bit(BTNXPUART_SERDEV_OPEN, &nxpdev->tx_state);
> > >         return 0;
> > >  }
> > This is already done in btnxpuart_flush(), which is called by
> > hci_dev_close_sync(), before it calls btnxpuart_close().
> 
> Yes, I was also wondering about that.
> 
> > Is btnxpuart_flush() not called during your testing?
> 
> Yes, I even added some more tracing to confirm this. However, without my
> fix (which BTW was inspired by looking at the former hci_mrvl.c driver) this
> bug is really occuring. Just keep loading/un-loading the kernel module a few
> times any you may hit it.
> 
Our QA team has been running load/unload tests for quite some time now, and
such an issue was never reported.
I am not sure why you do not see the btnxpuart_flush () been called, but I tested this patch
on my setup, where both, btnxpuart_flush() and btnxpuart_close() are called, and I
don’t see any issue due to kfree_skb and txq purge been done twice.

This looks ok to me.

Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@xxxxxxx>