linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: usb: fix memory leak in stk_camera_probe
@ 2021-07-14  3:23 Dongliang Mu
  2021-07-23 10:11 ` Dongliang Mu
  0 siblings, 1 reply; 15+ messages in thread
From: Dongliang Mu @ 2021-07-14  3:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Dongliang Mu; +Cc: linux-media, linux-kernel

stk_camera_probe mistakenly execute usb_get_intf and increase the
refcount of interface->dev.

Fix this by removing the execution of usb_get_intf.

Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
 drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
index a45d464427c4..5bd8e85b9452 100644
--- a/drivers/media/usb/stkwebcam/stk-webcam.c
+++ b/drivers/media/usb/stkwebcam/stk-webcam.c
@@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
 
 	dev->udev = udev;
 	dev->interface = interface;
-	usb_get_intf(interface);
 
 	if (hflip != -1)
 		dev->vsettings.hflip = hflip;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-07-14  3:23 [PATCH] media: usb: fix memory leak in stk_camera_probe Dongliang Mu
@ 2021-07-23 10:11 ` Dongliang Mu
  2021-09-02 10:23   ` Dongliang Mu
  0 siblings, 1 reply; 15+ messages in thread
From: Dongliang Mu @ 2021-07-23 10:11 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Dongliang Mu; +Cc: linux-media, linux-kernel

On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> stk_camera_probe mistakenly execute usb_get_intf and increase the
> refcount of interface->dev.
>
> Fix this by removing the execution of usb_get_intf.

Any idea about this patch?

>
> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
>  drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
> index a45d464427c4..5bd8e85b9452 100644
> --- a/drivers/media/usb/stkwebcam/stk-webcam.c
> +++ b/drivers/media/usb/stkwebcam/stk-webcam.c
> @@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
>
>         dev->udev = udev;
>         dev->interface = interface;
> -       usb_get_intf(interface);
>
>         if (hflip != -1)
>                 dev->vsettings.hflip = hflip;
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-07-23 10:11 ` Dongliang Mu
@ 2021-09-02 10:23   ` Dongliang Mu
  2021-09-02 10:39     ` Greg KH
                       ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 10:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Dongliang Mu
  Cc: linux-media, linux-kernel, Dan Carpenter, Greg KH

On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >
> > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > refcount of interface->dev.
> >
> > Fix this by removing the execution of usb_get_intf.
>
> Any idea about this patch?

+cc Dan Carpenter, gregkh

There is no reply in this thread in one month. Can someone give some
feedback on this patch?

>
> >
> > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> >  drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
> > index a45d464427c4..5bd8e85b9452 100644
> > --- a/drivers/media/usb/stkwebcam/stk-webcam.c
> > +++ b/drivers/media/usb/stkwebcam/stk-webcam.c
> > @@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
> >
> >         dev->udev = udev;
> >         dev->interface = interface;
> > -       usb_get_intf(interface);
> >
> >         if (hflip != -1)
> >                 dev->vsettings.hflip = hflip;
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:23   ` Dongliang Mu
@ 2021-09-02 10:39     ` Greg KH
  2021-09-02 10:54       ` Mauro Carvalho Chehab
  2021-09-02 10:49     ` Hans Verkuil
  2021-09-02 14:17     ` Dan Carpenter
  2 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2021-09-02 10:39 UTC (permalink / raw)
  To: Dongliang Mu
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Dan Carpenter

On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >
> > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >
> > > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > > refcount of interface->dev.
> > >
> > > Fix this by removing the execution of usb_get_intf.
> >
> > Any idea about this patch?
> 
> +cc Dan Carpenter, gregkh
> 
> There is no reply in this thread in one month. Can someone give some
> feedback on this patch?

This is the media developers domain, not much I can do here.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:23   ` Dongliang Mu
  2021-09-02 10:39     ` Greg KH
@ 2021-09-02 10:49     ` Hans Verkuil
  2021-09-02 11:06       ` Dongliang Mu
  2021-09-02 14:17     ` Dan Carpenter
  2 siblings, 1 reply; 15+ messages in thread
From: Hans Verkuil @ 2021-09-02 10:49 UTC (permalink / raw)
  To: Dongliang Mu, Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Dan Carpenter, Greg KH

Hi Dongliang,

On 02/09/2021 12:23, Dongliang Mu wrote:
> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>
>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>>
>>> stk_camera_probe mistakenly execute usb_get_intf and increase the
>>> refcount of interface->dev.
>>>
>>> Fix this by removing the execution of usb_get_intf.
>>
>> Any idea about this patch?
> 
> +cc Dan Carpenter, gregkh
> 
> There is no reply in this thread in one month. Can someone give some
> feedback on this patch?

I saw that it was marked as Obsoleted in patchwork, but I might have confused
this patch with similar, but not identical, patches for this driver.

I've moved the state back to New.

Comments follow below:

> 
>>
>>>
>>> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
>>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
>>> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
>>> ---
>>>  drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
>>> index a45d464427c4..5bd8e85b9452 100644
>>> --- a/drivers/media/usb/stkwebcam/stk-webcam.c
>>> +++ b/drivers/media/usb/stkwebcam/stk-webcam.c
>>> @@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
>>>
>>>         dev->udev = udev;
>>>         dev->interface = interface;
>>> -       usb_get_intf(interface);

Even though this increments the refcount (which might well be unnecessary),
it is also decremented with usb_put_intf. So is there actually a bug here?

And if this is changed, then I expect that both get_intf and put_intf should be
removed, and not just the get.

Regards,

	Hans

>>>
>>>         if (hflip != -1)
>>>                 dev->vsettings.hflip = hflip;
>>> --
>>> 2.25.1
>>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:39     ` Greg KH
@ 2021-09-02 10:54       ` Mauro Carvalho Chehab
  2021-09-02 10:59         ` Dongliang Mu
  0 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-09-02 10:54 UTC (permalink / raw)
  To: Greg KH; +Cc: Dongliang Mu, linux-media, linux-kernel, Dan Carpenter

Em Thu, 2 Sep 2021 12:39:37 +0200
Greg KH <gregkh@linuxfoundation.org> escreveu:

> On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> > On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:  
> > >
> > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:  
> > > >
> > > > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > > > refcount of interface->dev.
> > > >
> > > > Fix this by removing the execution of usb_get_intf.  
> > >
> > > Any idea about this patch?  
> > 
> > +cc Dan Carpenter, gregkh
> > 
> > There is no reply in this thread in one month. Can someone give some
> > feedback on this patch?  
> 
> This is the media developers domain, not much I can do here.

There is a high volume of patches for the media subsystem. Anyway,
as your patch is at our patchwork instance:

	https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/

It should be properly tracked, and likely handled after the end of
the merge window.

> > > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>

If you're the author of the patch, it doesn't make much sense to
add a "Reported-by:" tag there. We only use it in order to give
someone's else credit to report an issue.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:54       ` Mauro Carvalho Chehab
@ 2021-09-02 10:59         ` Dongliang Mu
  2021-09-02 11:10           ` Dongliang Mu
  0 siblings, 1 reply; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 10:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Greg KH, linux-media, linux-kernel, Dan Carpenter

On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
>
> Em Thu, 2 Sep 2021 12:39:37 +0200
> Greg KH <gregkh@linuxfoundation.org> escreveu:
>
> > On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> > > On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > > >
> > > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > > > >
> > > > > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > > > > refcount of interface->dev.
> > > > >
> > > > > Fix this by removing the execution of usb_get_intf.
> > > >
> > > > Any idea about this patch?
> > >
> > > +cc Dan Carpenter, gregkh
> > >
> > > There is no reply in this thread in one month. Can someone give some
> > > feedback on this patch?
> >
> > This is the media developers domain, not much I can do here.
>
> There is a high volume of patches for the media subsystem. Anyway,
> as your patch is at our patchwork instance:
>
>         https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/
>
> It should be properly tracked, and likely handled after the end of
> the merge window.
>
> > > > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
>
> If you're the author of the patch, it doesn't make much sense to
> add a "Reported-by:" tag there. We only use it in order to give
> someone's else credit to report an issue.

I see. Someone told me this rule in another thread. I will update this
in the next version.

>
> Thanks,
> Mauro

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:49     ` Hans Verkuil
@ 2021-09-02 11:06       ` Dongliang Mu
  0 siblings, 0 replies; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 11:06 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Dan Carpenter, Greg KH

On Thu, Sep 2, 2021 at 6:49 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Dongliang,
>
> On 02/09/2021 12:23, Dongliang Mu wrote:
> > On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>
> >> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>>
> >>> stk_camera_probe mistakenly execute usb_get_intf and increase the
> >>> refcount of interface->dev.
> >>>
> >>> Fix this by removing the execution of usb_get_intf.
> >>
> >> Any idea about this patch?
> >
> > +cc Dan Carpenter, gregkh
> >
> > There is no reply in this thread in one month. Can someone give some
> > feedback on this patch?
>
> I saw that it was marked as Obsoleted in patchwork, but I might have confused
> this patch with similar, but not identical, patches for this driver.
>
> I've moved the state back to New.
>
> Comments follow below:
>
> >
> >>
> >>>
> >>> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> >>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> >>> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> >>> ---
> >>>  drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
> >>>  1 file changed, 1 deletion(-)
> >>>
> >>> diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
> >>> index a45d464427c4..5bd8e85b9452 100644
> >>> --- a/drivers/media/usb/stkwebcam/stk-webcam.c
> >>> +++ b/drivers/media/usb/stkwebcam/stk-webcam.c
> >>> @@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
> >>>
> >>>         dev->udev = udev;
> >>>         dev->interface = interface;
> >>> -       usb_get_intf(interface);
>
> Even though this increments the refcount (which might well be unnecessary),
> it is also decremented with usb_put_intf. So is there actually a bug here?
>

Yes, if the increment and decrement of refcount are balanced, it's fine.

But this probe function only increases the refcount, but forgets to
decrease the refcount.

> And if this is changed, then I expect that both get_intf and put_intf should be
> removed, and not just the get.
>
> Regards,
>
>         Hans
>
> >>>
> >>>         if (hflip != -1)
> >>>                 dev->vsettings.hflip = hflip;
> >>> --
> >>> 2.25.1
> >>>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:59         ` Dongliang Mu
@ 2021-09-02 11:10           ` Dongliang Mu
  2021-09-02 11:15             ` Hans Verkuil
  0 siblings, 1 reply; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 11:10 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Greg KH, linux-media, linux-kernel, Dan Carpenter, Pavel Skripkin

On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
> >
> > Em Thu, 2 Sep 2021 12:39:37 +0200
> > Greg KH <gregkh@linuxfoundation.org> escreveu:
> >
> > > On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> > > > On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > > > >
> > > > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > > > > >
> > > > > > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > > > > > refcount of interface->dev.
> > > > > >
> > > > > > Fix this by removing the execution of usb_get_intf.
> > > > >
> > > > > Any idea about this patch?
> > > >
> > > > +cc Dan Carpenter, gregkh
> > > >
> > > > There is no reply in this thread in one month. Can someone give some
> > > > feedback on this patch?
> > >
> > > This is the media developers domain, not much I can do here.
> >
> > There is a high volume of patches for the media subsystem. Anyway,
> > as your patch is at our patchwork instance:
> >
> >         https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/
> >
> > It should be properly tracked, and likely handled after the end of
> > the merge window.

Hi Mauro,

I found there is another fix [1] for the same memory leak from Pavel
Skripkin (already cc-ed in this thread).

[1] https://www.spinics.net/lists/stable/msg479628.html

> >
> > > > > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> >
> > If you're the author of the patch, it doesn't make much sense to
> > add a "Reported-by:" tag there. We only use it in order to give
> > someone's else credit to report an issue.
>
> I see. Someone told me this rule in another thread. I will update this
> in the next version.
>
> >
> > Thanks,
> > Mauro

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 11:10           ` Dongliang Mu
@ 2021-09-02 11:15             ` Hans Verkuil
  2021-09-02 11:22               ` Dongliang Mu
  0 siblings, 1 reply; 15+ messages in thread
From: Hans Verkuil @ 2021-09-02 11:15 UTC (permalink / raw)
  To: Dongliang Mu, Mauro Carvalho Chehab
  Cc: Greg KH, linux-media, linux-kernel, Dan Carpenter, Pavel Skripkin

On 02/09/2021 13:10, Dongliang Mu wrote:
> On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>
>> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
>>>
>>> Em Thu, 2 Sep 2021 12:39:37 +0200
>>> Greg KH <gregkh@linuxfoundation.org> escreveu:
>>>
>>>> On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
>>>>> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>>>>>
>>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>>>>>>
>>>>>>> stk_camera_probe mistakenly execute usb_get_intf and increase the
>>>>>>> refcount of interface->dev.
>>>>>>>
>>>>>>> Fix this by removing the execution of usb_get_intf.
>>>>>>
>>>>>> Any idea about this patch?
>>>>>
>>>>> +cc Dan Carpenter, gregkh
>>>>>
>>>>> There is no reply in this thread in one month. Can someone give some
>>>>> feedback on this patch?
>>>>
>>>> This is the media developers domain, not much I can do here.
>>>
>>> There is a high volume of patches for the media subsystem. Anyway,
>>> as your patch is at our patchwork instance:
>>>
>>>         https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/
>>>
>>> It should be properly tracked, and likely handled after the end of
>>> the merge window.
> 
> Hi Mauro,
> 
> I found there is another fix [1] for the same memory leak from Pavel
> Skripkin (already cc-ed in this thread).
> 
> [1] https://www.spinics.net/lists/stable/msg479628.html

Ah, that's why I marked it as Obsoleted :-)

Regards,

	Hans

> 
>>>
>>>>>>> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
>>>>>>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
>>>>>>> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
>>>
>>> If you're the author of the patch, it doesn't make much sense to
>>> add a "Reported-by:" tag there. We only use it in order to give
>>> someone's else credit to report an issue.
>>
>> I see. Someone told me this rule in another thread. I will update this
>> in the next version.
>>
>>>
>>> Thanks,
>>> Mauro


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 11:15             ` Hans Verkuil
@ 2021-09-02 11:22               ` Dongliang Mu
  2021-09-02 18:14                 ` Pavel Skripkin
  0 siblings, 1 reply; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 11:22 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Greg KH, linux-media, linux-kernel,
	Dan Carpenter, Pavel Skripkin

On Thu, Sep 2, 2021 at 7:15 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 02/09/2021 13:10, Dongliang Mu wrote:
> > On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>
> >> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
> >>>
> >>> Em Thu, 2 Sep 2021 12:39:37 +0200
> >>> Greg KH <gregkh@linuxfoundation.org> escreveu:
> >>>
> >>>> On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> >>>>> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>>>>>
> >>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>>>>>>
> >>>>>>> stk_camera_probe mistakenly execute usb_get_intf and increase the
> >>>>>>> refcount of interface->dev.
> >>>>>>>
> >>>>>>> Fix this by removing the execution of usb_get_intf.
> >>>>>>
> >>>>>> Any idea about this patch?
> >>>>>
> >>>>> +cc Dan Carpenter, gregkh
> >>>>>
> >>>>> There is no reply in this thread in one month. Can someone give some
> >>>>> feedback on this patch?
> >>>>
> >>>> This is the media developers domain, not much I can do here.
> >>>
> >>> There is a high volume of patches for the media subsystem. Anyway,
> >>> as your patch is at our patchwork instance:
> >>>
> >>>         https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/
> >>>
> >>> It should be properly tracked, and likely handled after the end of
> >>> the merge window.
> >
> > Hi Mauro,
> >
> > I found there is another fix [1] for the same memory leak from Pavel
> > Skripkin (already cc-ed in this thread).
> >
> > [1] https://www.spinics.net/lists/stable/msg479628.html
>
> Ah, that's why I marked it as Obsoleted :-)

Oh, I see. If that patch is already merged, please remark my patch as Obsoleted.

Curiously, I did not get an email notification to mark my patch as
Obsoleted before. Why?

>
> Regards,
>
>         Hans
>
> >
> >>>
> >>>>>>> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> >>>>>>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> >>>>>>> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> >>>
> >>> If you're the author of the patch, it doesn't make much sense to
> >>> add a "Reported-by:" tag there. We only use it in order to give
> >>> someone's else credit to report an issue.
> >>
> >> I see. Someone told me this rule in another thread. I will update this
> >> in the next version.
> >>
> >>>
> >>> Thanks,
> >>> Mauro
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 10:23   ` Dongliang Mu
  2021-09-02 10:39     ` Greg KH
  2021-09-02 10:49     ` Hans Verkuil
@ 2021-09-02 14:17     ` Dan Carpenter
  2021-09-02 23:45       ` Dongliang Mu
  2 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-09-02 14:17 UTC (permalink / raw)
  To: Dongliang Mu; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Greg KH

On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >
> > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >
> > > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > > refcount of interface->dev.
> > >
> > > Fix this by removing the execution of usb_get_intf.
> >
> > Any idea about this patch?
> 
> +cc Dan Carpenter, gregkh
> 
> There is no reply in this thread in one month. Can someone give some
> feedback on this patch?
> 
> >
> > >
> > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > ---
> > >  drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
> > > index a45d464427c4..5bd8e85b9452 100644
> > > --- a/drivers/media/usb/stkwebcam/stk-webcam.c
> > > +++ b/drivers/media/usb/stkwebcam/stk-webcam.c
> > > @@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
> > >
> > >         dev->udev = udev;
> > >         dev->interface = interface;
> > > -       usb_get_intf(interface);


The patch is wrong.  We're storing a reference to "interface".

	dev->interface = interface;

So we need to boost the refcount of interface.  Pavel Skripkin was on
the right patch but you need to add a:

	usb_put_intf(interface);

to the stk_camera_disconnect() function as you sort of mentioned.
That's the correct fix.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 11:22               ` Dongliang Mu
@ 2021-09-02 18:14                 ` Pavel Skripkin
  2021-09-02 23:51                   ` Dongliang Mu
  0 siblings, 1 reply; 15+ messages in thread
From: Pavel Skripkin @ 2021-09-02 18:14 UTC (permalink / raw)
  To: Dongliang Mu, Hans Verkuil
  Cc: Mauro Carvalho Chehab, Greg KH, linux-media, linux-kernel, Dan Carpenter

On 9/2/21 14:22, Dongliang Mu wrote:
> On Thu, Sep 2, 2021 at 7:15 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 02/09/2021 13:10, Dongliang Mu wrote:
>> > On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>> >>
>> >> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
>> >>>
>> >>> Em Thu, 2 Sep 2021 12:39:37 +0200
>> >>> Greg KH <gregkh@linuxfoundation.org> escreveu:
>> >>>
>> >>>> On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
>> >>>>> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>> >>>>>>
>> >>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>> >>>>>>>
>> >>>>>>> stk_camera_probe mistakenly execute usb_get_intf and increase the
>> >>>>>>> refcount of interface->dev.
>> >>>>>>>
>> >>>>>>> Fix this by removing the execution of usb_get_intf.
>> >>>>>>
>> >>>>>> Any idea about this patch?
>> >>>>>
>> >>>>> +cc Dan Carpenter, gregkh
>> >>>>>
>> >>>>> There is no reply in this thread in one month. Can someone give some
>> >>>>> feedback on this patch?
>> >>>>
>> >>>> This is the media developers domain, not much I can do here.
>> >>>
>> >>> There is a high volume of patches for the media subsystem. Anyway,
>> >>> as your patch is at our patchwork instance:
>> >>>
>> >>>         https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/
>> >>>
>> >>> It should be properly tracked, and likely handled after the end of
>> >>> the merge window.
>> >
>> > Hi Mauro,
>> >
>> > I found there is another fix [1] for the same memory leak from Pavel
>> > Skripkin (already cc-ed in this thread).
>> >
>> > [1] https://www.spinics.net/lists/stable/msg479628.html
>>
>> Ah, that's why I marked it as Obsoleted :-)
> 
> Oh, I see. If that patch is already merged, please remark my patch as Obsoleted.
> 
> Curiously, I did not get an email notification to mark my patch as
> Obsoleted before. Why?
> 
>>

Hi, Dongliang!

Yep my patch has been merged already (1 month ago, I guess).




With regards,
Pavel Skripkin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 14:17     ` Dan Carpenter
@ 2021-09-02 23:45       ` Dongliang Mu
  0 siblings, 0 replies; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 23:45 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Greg KH

On Thu, Sep 2, 2021 at 10:18 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> > On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >
> > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > > >
> > > > stk_camera_probe mistakenly execute usb_get_intf and increase the
> > > > refcount of interface->dev.
> > > >
> > > > Fix this by removing the execution of usb_get_intf.
> > >
> > > Any idea about this patch?
> >
> > +cc Dan Carpenter, gregkh
> >
> > There is no reply in this thread in one month. Can someone give some
> > feedback on this patch?
> >
> > >
> > > >
> > > > Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > > ---
> > > >  drivers/media/usb/stkwebcam/stk-webcam.c | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c b/drivers/media/usb/stkwebcam/stk-webcam.c
> > > > index a45d464427c4..5bd8e85b9452 100644
> > > > --- a/drivers/media/usb/stkwebcam/stk-webcam.c
> > > > +++ b/drivers/media/usb/stkwebcam/stk-webcam.c
> > > > @@ -1311,7 +1311,6 @@ static int stk_camera_probe(struct usb_interface *interface,
> > > >
> > > >         dev->udev = udev;
> > > >         dev->interface = interface;
> > > > -       usb_get_intf(interface);
>
>
> The patch is wrong.  We're storing a reference to "interface".
>
>         dev->interface = interface;
>
> So we need to boost the refcount of interface.  Pavel Skripkin was on
> the right patch but you need to add a:
>
>         usb_put_intf(interface);
>
> to the stk_camera_disconnect() function as you sort of mentioned.
> That's the correct fix.

Thanks for your explanation, Dan. It's really helpful.

I sent the inquiry email in this thread because I did not receive the
notification of patchwork to mark my patch as obsolete and did not
notice Pavel had sent one patch before.

Now, this patch is marked as obsolete. Let's ignore it now.

>
> regards,
> dan carpenter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] media: usb: fix memory leak in stk_camera_probe
  2021-09-02 18:14                 ` Pavel Skripkin
@ 2021-09-02 23:51                   ` Dongliang Mu
  0 siblings, 0 replies; 15+ messages in thread
From: Dongliang Mu @ 2021-09-02 23:51 UTC (permalink / raw)
  To: Pavel Skripkin
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Greg KH, linux-media,
	linux-kernel, Dan Carpenter

On Fri, Sep 3, 2021 at 2:14 AM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> On 9/2/21 14:22, Dongliang Mu wrote:
> > On Thu, Sep 2, 2021 at 7:15 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> On 02/09/2021 13:10, Dongliang Mu wrote:
> >> > On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >> >>
> >> >> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
> >> >>>
> >> >>> Em Thu, 2 Sep 2021 12:39:37 +0200
> >> >>> Greg KH <gregkh@linuxfoundation.org> escreveu:
> >> >>>
> >> >>>> On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> >> >>>>> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >> >>>>>>
> >> >>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >> >>>>>>>
> >> >>>>>>> stk_camera_probe mistakenly execute usb_get_intf and increase the
> >> >>>>>>> refcount of interface->dev.
> >> >>>>>>>
> >> >>>>>>> Fix this by removing the execution of usb_get_intf.
> >> >>>>>>
> >> >>>>>> Any idea about this patch?
> >> >>>>>
> >> >>>>> +cc Dan Carpenter, gregkh
> >> >>>>>
> >> >>>>> There is no reply in this thread in one month. Can someone give some
> >> >>>>> feedback on this patch?
> >> >>>>
> >> >>>> This is the media developers domain, not much I can do here.
> >> >>>
> >> >>> There is a high volume of patches for the media subsystem. Anyway,
> >> >>> as your patch is at our patchwork instance:
> >> >>>
> >> >>>         https://patchwork.linuxtv.org/project/linux-media/patch/20210714032340.504836-1-mudongliangabcd@gmail.com/
> >> >>>
> >> >>> It should be properly tracked, and likely handled after the end of
> >> >>> the merge window.
> >> >
> >> > Hi Mauro,
> >> >
> >> > I found there is another fix [1] for the same memory leak from Pavel
> >> > Skripkin (already cc-ed in this thread).
> >> >
> >> > [1] https://www.spinics.net/lists/stable/msg479628.html
> >>
> >> Ah, that's why I marked it as Obsoleted :-)
> >
> > Oh, I see. If that patch is already merged, please remark my patch as Obsoleted.
> >
> > Curiously, I did not get an email notification to mark my patch as
> > Obsoleted before. Why?
> >
> >>
>
> Hi, Dongliang!
>
> Yep my patch has been merged already (1 month ago, I guess).

Yes. When I submit this patch, your patch is still pending. I did not
know someone is already sending a patch with good quality.

So I am curious if there are any methods to notify people there are
already some similar patches in the patchwork or some other resources.

>
>
>
>
> With regards,
> Pavel Skripkin

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-09-02 23:51 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-14  3:23 [PATCH] media: usb: fix memory leak in stk_camera_probe Dongliang Mu
2021-07-23 10:11 ` Dongliang Mu
2021-09-02 10:23   ` Dongliang Mu
2021-09-02 10:39     ` Greg KH
2021-09-02 10:54       ` Mauro Carvalho Chehab
2021-09-02 10:59         ` Dongliang Mu
2021-09-02 11:10           ` Dongliang Mu
2021-09-02 11:15             ` Hans Verkuil
2021-09-02 11:22               ` Dongliang Mu
2021-09-02 18:14                 ` Pavel Skripkin
2021-09-02 23:51                   ` Dongliang Mu
2021-09-02 10:49     ` Hans Verkuil
2021-09-02 11:06       ` Dongliang Mu
2021-09-02 14:17     ` Dan Carpenter
2021-09-02 23:45       ` Dongliang Mu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).