Re: [PATCH net] wifi: rtw88: properly drop usb interface reference on error

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Feb 24, 2026 at 12:46:02AM +0000, Ping-Ke Shih wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > If an error happens in the usb probe path, in rtw_usb_intf_init(), the
> > usb interface reference needs to be properly dropped, otherwise is is
> > incorrectly increased when returning to the USB core.
> > 
> > Cc: Ping-Ke Shih <pkshih@realtek.com>
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_2000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/net/wireless/realtek/rtw88/usb.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> > index 433b06c8d8a6..36ac20039ce2 100644
> > --- a/drivers/net/wireless/realtek/rtw88/usb.c
> > +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> > @@ -1046,13 +1046,17 @@ static int rtw_usb_intf_init(struct rtw_dev *rtwdev,
> > 
> >         rtwusb->udev = udev;
> >         ret = rtw_usb_parse(rtwdev, intf);
> > -       if (ret)
> > +       if (ret) {
> > +               usb_put_dev(udev);
> >                 return ret;
> > +       }
> > 
> >         rtwusb->usb_data = kcalloc(RTW_USB_MAX_RXTX_COUNT, sizeof(u32),
> >                                    GFP_KERNEL);
> > -       if (!rtwusb->usb_data)
> > +       if (!rtwusb->usb_data) {
> > +               usb_put_dev(udev);
> >                 return -ENOMEM;
> > +       }
> > 
> >         usb_set_intfdata(intf, rtwdev->hw);
> > 
> 
> Since rtwusb->udev isn't used right after assignment in this function.
> Would it be simpler that moving usb_get_dev() downward like below?

What is even simpler, and easier, is to never call usb_get_dev() at all
anyway as it's not needed :)

I created that pattern a few decades ago when we thought that it was
going to be required, but as long as the usb interface is bound to the
driver, that pointer is going to be valid so there's no real need to
increment the reference count, except to feel good about doing it.

I'll gladly do that fix instead, if you want me to, I was just trying to
follow the style of the existing code and fix up the current bug.

> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index db60e142268d..6e5c9c6f3e00 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -1041,10 +1041,8 @@ static int rtw_usb_intf_init(struct rtw_dev *rtwdev,
>                              struct usb_interface *intf)
>  {
>         struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> -       struct usb_device *udev = usb_get_dev(interface_to_usbdev(intf));
>         int ret;
> 
> -       rtwusb->udev = udev;
>         ret = rtw_usb_parse(rtwdev, intf);
>         if (ret)
>                 return ret;
> @@ -1054,6 +1052,8 @@ static int rtw_usb_intf_init(struct rtw_dev *rtwdev,
>         if (!rtwusb->usb_data)
>                 return -ENOMEM;
> 
> +       rtwusb->udev = usb_get_dev(interface_to_usbdev(intf));

That too works, or again, just drop the usb_get_dev() and usb_put_dev()
calls entirely.

thanks,

greg k-h