2021-07-14 03:29:13

by Dongliang Mu

[permalink] [raw]
Subject: [PATCH] media: usb: fix memory leak in stk_camera_probe

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 <[email protected]>
Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
Signed-off-by: Dongliang Mu <[email protected]>
---
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


2021-07-23 10:14:13

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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 <[email protected]>
> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> Signed-off-by: Dongliang Mu <[email protected]>
> ---
> 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
>

2021-09-02 10:25:07

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <[email protected]> wrote:
>
> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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 <[email protected]>
> > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > Signed-off-by: Dongliang Mu <[email protected]>
> > ---
> > 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
> >

2021-09-02 10:41:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <[email protected]> wrote:
> >
> > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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

2021-09-02 10:56:18

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

Hi Dongliang,

On 02/09/2021 12:23, Dongliang Mu wrote:
> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <[email protected]> wrote:
>>
>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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 <[email protected]>
>>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
>>> Signed-off-by: Dongliang Mu <[email protected]>
>>> ---
>>> 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
>>>

2021-09-02 10:57:08

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

Em Thu, 2 Sep 2021 12:39:37 +0200
Greg KH <[email protected]> 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 <[email protected]> wrote:
> > >
> > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/

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

> > > > Reported-by: Dongliang Mu <[email protected]>
> > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > Signed-off-by: Dongliang Mu <[email protected]>

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

2021-09-02 11:18:41

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On 02/09/2021 13:10, Dongliang Mu wrote:
> On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <[email protected]> wrote:
>>
>> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <[email protected]> wrote:
>>>
>>> Em Thu, 2 Sep 2021 12:39:37 +0200
>>> Greg KH <[email protected]> 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 <[email protected]> wrote:
>>>>>>
>>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/
>>>
>>> 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 <[email protected]>
>>>>>>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
>>>>>>> Signed-off-by: Dongliang Mu <[email protected]>
>>>
>>> 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

2021-09-02 14:25:08

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 02, 2021 at 06:23:36PM +0800, Dongliang Mu wrote:
> On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <[email protected]> wrote:
> >
> > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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 <[email protected]>
> > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > Signed-off-by: Dongliang Mu <[email protected]>
> > > ---
> > > 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

2021-09-02 18:18:08

by Pavel Skripkin

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On 9/2/21 14:22, Dongliang Mu wrote:
> On Thu, Sep 2, 2021 at 7:15 PM Hans Verkuil <[email protected]> wrote:
>>
>> On 02/09/2021 13:10, Dongliang Mu wrote:
>> > On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <[email protected]> wrote:
>> >>
>> >> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <[email protected]> wrote:
>> >>>
>> >>> Em Thu, 2 Sep 2021 12:39:37 +0200
>> >>> Greg KH <[email protected]> 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 <[email protected]> wrote:
>> >>>>>>
>> >>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/
>> >>>
>> >>> 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

2021-09-02 19:55:46

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 2, 2021 at 6:49 PM Hans Verkuil <[email protected]> wrote:
>
> Hi Dongliang,
>
> On 02/09/2021 12:23, Dongliang Mu wrote:
> > On Fri, Jul 23, 2021 at 6:11 PM Dongliang Mu <[email protected]> wrote:
> >>
> >> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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 <[email protected]>
> >>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> >>> Signed-off-by: Dongliang Mu <[email protected]>
> >>> ---
> >>> 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
> >>>
>

2021-09-02 19:56:21

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <[email protected]> wrote:
>
> Em Thu, 2 Sep 2021 12:39:37 +0200
> Greg KH <[email protected]> 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 <[email protected]> wrote:
> > > >
> > > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/
>
> It should be properly tracked, and likely handled after the end of
> the merge window.
>
> > > > > Reported-by: Dongliang Mu <[email protected]>
> > > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > > Signed-off-by: Dongliang Mu <[email protected]>
>
> 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

2021-09-02 19:56:22

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <[email protected]> wrote:
>
> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <[email protected]> wrote:
> >
> > Em Thu, 2 Sep 2021 12:39:37 +0200
> > Greg KH <[email protected]> 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 <[email protected]> wrote:
> > > > >
> > > > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/
> >
> > 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 <[email protected]>
> > > > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > > > Signed-off-by: Dongliang Mu <[email protected]>
> >
> > 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

2021-09-02 19:56:25

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 2, 2021 at 7:15 PM Hans Verkuil <[email protected]> wrote:
>
> On 02/09/2021 13:10, Dongliang Mu wrote:
> > On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <[email protected]> wrote:
> >>
> >> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <[email protected]> wrote:
> >>>
> >>> Em Thu, 2 Sep 2021 12:39:37 +0200
> >>> Greg KH <[email protected]> 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 <[email protected]> wrote:
> >>>>>>
> >>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/
> >>>
> >>> 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 <[email protected]>
> >>>>>>> Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> >>>>>>> Signed-off-by: Dongliang Mu <[email protected]>
> >>>
> >>> 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
>

2021-09-03 02:14:57

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Fri, Sep 3, 2021 at 2:14 AM Pavel Skripkin <[email protected]> wrote:
>
> On 9/2/21 14:22, Dongliang Mu wrote:
> > On Thu, Sep 2, 2021 at 7:15 PM Hans Verkuil <[email protected]> wrote:
> >>
> >> On 02/09/2021 13:10, Dongliang Mu wrote:
> >> > On Thu, Sep 2, 2021 at 6:59 PM Dongliang Mu <[email protected]> wrote:
> >> >>
> >> >> On Thu, Sep 2, 2021 at 6:54 PM Mauro Carvalho Chehab <[email protected]> wrote:
> >> >>>
> >> >>> Em Thu, 2 Sep 2021 12:39:37 +0200
> >> >>> Greg KH <[email protected]> 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 <[email protected]> wrote:
> >> >>>>>>
> >> >>>>>> On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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/[email protected]/
> >> >>>
> >> >>> 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

2021-09-03 03:25:08

by Dongliang Mu

[permalink] [raw]
Subject: Re: [PATCH] media: usb: fix memory leak in stk_camera_probe

On Thu, Sep 2, 2021 at 10:18 PM Dan Carpenter <[email protected]> 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 <[email protected]> wrote:
> > >
> > > On Wed, Jul 14, 2021 at 11:23 AM Dongliang Mu <[email protected]> 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 <[email protected]>
> > > > Fixes: 0aa77f6c2954 ("[media] move the remaining USB drivers to drivers/media/usb")
> > > > Signed-off-by: Dongliang Mu <[email protected]>
> > > > ---
> > > > 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