Re: [PATCH] HID: elo: Fix refcount leak in elo_probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 25, 2022 at 5:15 PM Greg KH <greg@xxxxxxxxx> wrote:
>
> On Thu, Feb 17, 2022 at 10:25:02AM -0500, Alan Stern wrote:
> > On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote:
> > > Salah sent a bunch of these.  The reasoning was explained in this email.
> > >
> > > https://www.spinics.net/lists/kernel/msg4026672.html
> > >
> > > When he resent the patch, Greg said that taking the reference wasn't
> > > needed so the patch wasn't applied.  (Also it had the same reference
> > > leak so that's a second reason it wasn't applied).
> >
> > Indeed, the kerneldoc for usb_get_intf() does say that each reference
> > held by a driver must be refcounted.  And there's nothing wrong with
> > doing that, _provided_ you do it correctly.
> >
> > But if you know the extra refcount will never be needed (because the
> > reference will be dropped before the usb_interface in question is
> > removed), fiddling with the reference count is unnecessary.  I guess
> > whether or not to do it could be considered a matter of taste.
> >
> > On the other hand, it wouldn't hurt to update the kerneldoc for
> > usb_get_intf() (and usb_get_dev() also).  We could point out that if a
> > driver does not access the usb_interface structure after its disconnect
> > routine returns, incrementing the refcount isn't mandatory.
>
> That would be good to add to prevent this type of confusion in the
> future.

Hi Jiri Kosina,

from my understanding, my previous patch and patch from Alan Stern can
all fix the underlying issue. But as Dan said, the patch from Alan is
more elegant.

However, the revert commit from you said, my commit introduces memory
leak, which confuses me.

HID: elo: Revert USB reference counting

Commit 817b8b9 ("HID: elo: fix memory leak in elo_probe") introduced
memory leak on error path, but more importantly the whole USB reference
counting is not needed at all in the first place ......

If it is really my patch that introduces the memory leak, please let me know.

best regards,
Dongliang Mu

>
> thanks,
>
> greg k-h



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux