linux-kernel - Re: [PATCH] Input: pxrc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 15 Jul 2018 12:12:44 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Marcus Folkesson <marcus.folkesson@...il.com>,
        Alexey Khoroshilov <khoroshilov@...ras.ru>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>, ldv-project@...uxtesting.org
Subject: Re: [PATCH] Input: pxrc - fix leak of usb_device

On Sun, Jul 15, 2018 at 01:06:32PM +0300, Dmitry Torokhov wrote:
> On Sun, Jul 15, 2018 at 10:42 AM Marcus Folkesson
> <marcus.folkesson@...il.com> wrote:
> >
> > On Sat, Jul 14, 2018 at 08:51:09AM +0000, Dmitry Torokhov wrote:
> > > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote:
> > > > Hi Alexey,
> > > >
> > > > Good catch!
> > > >
> > > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote:
> > > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev()
> > > > > anywhere in the driver.
> > > > >
> > > > > The patch adds one to error handling code and to pxrc_disconnect().
> > > > >
> > > > > Found by Linux Driver Verification project (linuxtesting.org).
> > > > >
> > > > > Signed-off-by: Alexey Khoroshilov <khoroshilov@...ras.ru>
> > > >
> > > > Reviewed-by: Marcus Folkesson <marcus.folkesson@...il.com>
> > >
> > > Hmm, the biggest question however if we need to "take" the device, as I
> > > do not think interface can outlive the device, and whether we actually
> > > need to store it in pxrc, as we only need it during set up, as far as I
> > > can see.
> >
> > Yep, the device is only used during setup.
> > I interpret the comments for usb_get_dev() as you should take a
> > reference count on the device even if you only use the interface, but I
> > could be wrong.
> >
> > From usb_get_dev()::
> >
> >          * usb_get_dev - increments the reference count of the usb device structure
> >          * @dev: the device being referenced
> >          *
> >          * Each live reference to a device should be refcounted.
> >          *
> >          * Drivers for USB interfaces should normally record such references in
> >          * their probe() methods, when they bind to an interface, and release
> >          * them by calling usb_put_dev(), in their disconnect() methods.
> 
> Hmm, usb device is a parent of usb interface so our driver model rules
> ensure that usb device should not disappear while interface device is
> still there. Greg, is this comment still valid?

Yes, that is true.  But remember that interface devices can go away
while the parent is still present, so if you need the interface pointer,
you have to grab a reference on it.

thanks,

greg k-h