Re: [PATCH 1/4] mmc: vub300: fix NULL-deref on disconnect

[Ulf Hansson]

On Tue, 31 Mar 2026 at 12:32, Johan Hovold <johan@xxxxxxxxxx> wrote:
>
> On Tue, Mar 31, 2026 at 12:13:41PM +0200, Ulf Hansson wrote:
> > On Fri, 27 Mar 2026 at 11:52, Johan Hovold <johan@xxxxxxxxxx> wrote:
> > >
> > > Make sure to deregister the controller before dropping the reference to
> > > the driver data on disconnect to avoid NULL-pointer dereferences or
> > > use-after-free.
> > >
> > > Fixes: 88095e7b473a ("mmc: Add new VUB300 USB-to-SD/SDIO/MMC driver")
> > > Cc: stable@xxxxxxxxxxxxxxx      # 3.0
> > > Cc: Tony Olech <tony.olech@xxxxxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> > > ---
> > >  drivers/mmc/host/vub300.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
> > > index ff49d0770506..f173c7cf4e1a 100644
> > > --- a/drivers/mmc/host/vub300.c
> > > +++ b/drivers/mmc/host/vub300.c
> > > @@ -2365,8 +2365,8 @@ static void vub300_disconnect(struct usb_interface *interface)
> > >                         usb_set_intfdata(interface, NULL);
> > >                         /* prevent more I/O from starting */
> > >                         vub300->interface = NULL;
> > > -                       kref_put(&vub300->kref, vub300_delete);
> > >                         mmc_remove_host(mmc);
> > > +                       kref_put(&vub300->kref, vub300_delete);
> >
> > While this seems like a step in the right direction, I don't see why
> > calling usb_set_intfdata(interface, NULL)
>
> The interface data is only used in the USB bus callbacks and is not
> needed after disconnect().
>
> > and assigning
> > vub300->interface = NULL is safe.
> >
> > For example, some of the workqueues might be running a work that uses
> > the vub300->interface, isn't that a problem too?
>
> The driver uses this pointer to indicate that the device has been
> disconnected. That doesn't mean that the implementation is correct (e.g.
> the check in vub300_pollwork_thread() should use some locking) but that
> would be pre-existing issues.

Right, that was my thinking as well.

Out of curiosity, are you planning on fixing these issues too or is
that left for later?

Kind regards
Uffe