* [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware()
@ 2015-02-11 20:58 Christian Engelmayer
2015-02-11 21:45 ` Matthias Schwarzott
0 siblings, 1 reply; 5+ messages in thread
From: Christian Engelmayer @ 2015-02-11 20:58 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, zzam, hans.verkuil, Christian Engelmayer
In case of an error function si2165_upload_firmware() releases the already
requested firmware in the exit path. However, there is one deviation where
the function directly returns. Use the correct cleanup so that the firmware
memory gets freed correctly. Detected by Coverity CID 1269120.
Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
---
Compile tested only. Applies against linux-next.
---
drivers/media/dvb-frontends/si2165.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
index 98ddb49ad52b..4cc5d10ed0d4 100644
--- a/drivers/media/dvb-frontends/si2165.c
+++ b/drivers/media/dvb-frontends/si2165.c
@@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state *state)
/* reset crc */
ret = si2165_writereg8(state, 0x0379, 0x01);
if (ret)
- return ret;
+ goto error;
ret = si2165_upload_firmware_block(state, data, len,
&offset, block_count);
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware()
2015-02-11 20:58 [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware() Christian Engelmayer
@ 2015-02-11 21:45 ` Matthias Schwarzott
2015-02-11 23:38 ` Luis de Bethencourt
0 siblings, 1 reply; 5+ messages in thread
From: Matthias Schwarzott @ 2015-02-11 21:45 UTC (permalink / raw)
To: Christian Engelmayer, linux-media; +Cc: mchehab, hans.verkuil
On 11.02.2015 21:58, Christian Engelmayer wrote:
> In case of an error function si2165_upload_firmware() releases the already
> requested firmware in the exit path. However, there is one deviation where
> the function directly returns. Use the correct cleanup so that the firmware
> memory gets freed correctly. Detected by Coverity CID 1269120.
>
> Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
> ---
> Compile tested only. Applies against linux-next.
> ---
> drivers/media/dvb-frontends/si2165.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
> index 98ddb49ad52b..4cc5d10ed0d4 100644
> --- a/drivers/media/dvb-frontends/si2165.c
> +++ b/drivers/media/dvb-frontends/si2165.c
> @@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state *state)
> /* reset crc */
> ret = si2165_writereg8(state, 0x0379, 0x01);
> if (ret)
> - return ret;
> + goto error;
>
> ret = si2165_upload_firmware_block(state, data, len,
> &offset, block_count);
>
Good catch.
Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware()
2015-02-11 21:45 ` Matthias Schwarzott
@ 2015-02-11 23:38 ` Luis de Bethencourt
2015-02-11 23:42 ` Antti Palosaari
0 siblings, 1 reply; 5+ messages in thread
From: Luis de Bethencourt @ 2015-02-11 23:38 UTC (permalink / raw)
To: Matthias Schwarzott
Cc: Christian Engelmayer, linux-media, mchehab, hans.verkuil
On Wed, Feb 11, 2015 at 10:45:01PM +0100, Matthias Schwarzott wrote:
> On 11.02.2015 21:58, Christian Engelmayer wrote:
> > In case of an error function si2165_upload_firmware() releases the already
> > requested firmware in the exit path. However, there is one deviation where
> > the function directly returns. Use the correct cleanup so that the firmware
> > memory gets freed correctly. Detected by Coverity CID 1269120.
> >
> > Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
> > ---
> > Compile tested only. Applies against linux-next.
> > ---
> > drivers/media/dvb-frontends/si2165.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
> > index 98ddb49ad52b..4cc5d10ed0d4 100644
> > --- a/drivers/media/dvb-frontends/si2165.c
> > +++ b/drivers/media/dvb-frontends/si2165.c
> > @@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state *state)
> > /* reset crc */
> > ret = si2165_writereg8(state, 0x0379, 0x01);
> > if (ret)
> > - return ret;
> > + goto error;
> >
> > ret = si2165_upload_firmware_block(state, data, len,
> > &offset, block_count);
> >
> Good catch.
>
> Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
>
Good catch indeed.
Can I sign off? Not sure what the rules are.
Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware()
2015-02-11 23:38 ` Luis de Bethencourt
@ 2015-02-11 23:42 ` Antti Palosaari
2015-02-12 0:13 ` Luis de Bethencourt
0 siblings, 1 reply; 5+ messages in thread
From: Antti Palosaari @ 2015-02-11 23:42 UTC (permalink / raw)
To: Luis de Bethencourt, Matthias Schwarzott
Cc: Christian Engelmayer, linux-media, mchehab, hans.verkuil
On 02/12/2015 01:38 AM, Luis de Bethencourt wrote:
> On Wed, Feb 11, 2015 at 10:45:01PM +0100, Matthias Schwarzott wrote:
>> On 11.02.2015 21:58, Christian Engelmayer wrote:
>>> In case of an error function si2165_upload_firmware() releases the already
>>> requested firmware in the exit path. However, there is one deviation where
>>> the function directly returns. Use the correct cleanup so that the firmware
>>> memory gets freed correctly. Detected by Coverity CID 1269120.
>>>
>>> Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
>>> ---
>>> Compile tested only. Applies against linux-next.
>>> ---
>>> drivers/media/dvb-frontends/si2165.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
>>> index 98ddb49ad52b..4cc5d10ed0d4 100644
>>> --- a/drivers/media/dvb-frontends/si2165.c
>>> +++ b/drivers/media/dvb-frontends/si2165.c
>>> @@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state *state)
>>> /* reset crc */
>>> ret = si2165_writereg8(state, 0x0379, 0x01);
>>> if (ret)
>>> - return ret;
>>> + goto error;
>>>
>>> ret = si2165_upload_firmware_block(state, data, len,
>>> &offset, block_count);
>>>
>> Good catch.
>>
>> Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
>>
>
> Good catch indeed.
>
> Can I sign off? Not sure what the rules are.
>
> Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
You cannot sign it unless patch is going through hands. Probably you
want review it. Check documentation "SubmittingPatches".
https://www.kernel.org/doc/Documentation/SubmittingPatches
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware()
2015-02-11 23:42 ` Antti Palosaari
@ 2015-02-12 0:13 ` Luis de Bethencourt
0 siblings, 0 replies; 5+ messages in thread
From: Luis de Bethencourt @ 2015-02-12 0:13 UTC (permalink / raw)
To: Antti Palosaari
Cc: Matthias Schwarzott, Christian Engelmayer, linux-media, mchehab,
hans.verkuil
On Thu, Feb 12, 2015 at 01:42:45AM +0200, Antti Palosaari wrote:
> On 02/12/2015 01:38 AM, Luis de Bethencourt wrote:
> >On Wed, Feb 11, 2015 at 10:45:01PM +0100, Matthias Schwarzott wrote:
> >>On 11.02.2015 21:58, Christian Engelmayer wrote:
> >>>In case of an error function si2165_upload_firmware() releases the already
> >>>requested firmware in the exit path. However, there is one deviation where
> >>>the function directly returns. Use the correct cleanup so that the firmware
> >>>memory gets freed correctly. Detected by Coverity CID 1269120.
> >>>
> >>>Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
> >>>---
> >>>Compile tested only. Applies against linux-next.
> >>>---
> >>> drivers/media/dvb-frontends/si2165.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
> >>>index 98ddb49ad52b..4cc5d10ed0d4 100644
> >>>--- a/drivers/media/dvb-frontends/si2165.c
> >>>+++ b/drivers/media/dvb-frontends/si2165.c
> >>>@@ -505,7 +505,7 @@ static int si2165_upload_firmware(struct si2165_state *state)
> >>> /* reset crc */
> >>> ret = si2165_writereg8(state, 0x0379, 0x01);
> >>> if (ret)
> >>>- return ret;
> >>>+ goto error;
> >>>
> >>> ret = si2165_upload_firmware_block(state, data, len,
> >>> &offset, block_count);
> >>>
> >>Good catch.
> >>
> >>Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
> >>
> >
> >Good catch indeed.
> >
> >Can I sign off? Not sure what the rules are.
> >
> >Signed-off-by: Luis de Bethencourt <luis.bg@samsung.com>
>
>
> You cannot sign it unless patch is going through hands. Probably you want
> review it. Check documentation "SubmittingPatches".
>
> https://www.kernel.org/doc/Documentation/SubmittingPatches
>
> regards
> Antti
>
> --
> http://palosaari.fi/
> --
Hi Antti,
That was an interesting read. Now I know how these tags work :)
Thanks for the pointing it out to me.
So I meant "Reviewed-by:"
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-12 0:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-11 20:58 [PATCH] [media] si2165: Fix possible leak in si2165_upload_firmware() Christian Engelmayer
2015-02-11 21:45 ` Matthias Schwarzott
2015-02-11 23:38 ` Luis de Bethencourt
2015-02-11 23:42 ` Antti Palosaari
2015-02-12 0:13 ` Luis de Bethencourt
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).