[PATCH][RESEND] static code analysis from cppcheck reports:

[PATCH][RESEND] static code analysis from cppcheck reports:

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

[drivers/video/fbdev/broadsheetfb.c:673]:
  (error) Memory leak: sector_buffer

sector_buffer is not being kfree'd on each call to
broadsheet_spiflash_rewrite_sector(), so free it.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/video/fbdev/broadsheetfb.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index 1c29bd1..0e5fde1 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -636,7 +636,7 @@ static int broadsheet_spiflash_rewrite_sector(struct broadsheetfb_par *par,
 		err = broadsheet_spiflash_read_range(par, start_sector_addr,
 						data_start_addr, sector_buffer);
 		if (err)
-			return err;
+			goto out;
 	}
 
 	/* now we copy our data into the right place in the sector buffer */
@@ -657,7 +657,7 @@ static int broadsheet_spiflash_rewrite_sector(struct broadsheetfb_par *par,
 		err = broadsheet_spiflash_read_range(par, tail_start_addr,
 			tail_len, sector_buffer + tail_start_addr);
 		if (err)
-			return err;
+			goto out;
 	}
 
 	/* if we got here we have the full sector that we want to rewrite. */
@@ -665,11 +665,13 @@ static int broadsheet_spiflash_rewrite_sector(struct broadsheetfb_par *par,
 	/* first erase the sector */
 	err = broadsheet_spiflash_erase_sector(par, start_sector_addr);
 	if (err)
-		return err;
+		goto out;
 
 	/* now write it */
 	err = broadsheet_spiflash_write_sector(par, start_sector_addr,
 					sector_buffer, sector_size);
+out:
+	kfree(sector_buffer);
 	return err;
 }
 
-- 
2.1.4

Re: [PATCH][RESEND] static code analysis from cppcheck reports:

[Geert Uytterhoeven]

On Mon, Jan 12, 2015 at 4:27 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> [drivers/video/fbdev/broadsheetfb.c:673]:
>   (error) Memory leak: sector_buffer
>
> sector_buffer is not being kfree'd on each call to
> broadsheet_spiflash_rewrite_sector(), so free it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

So you can make the box go OOM by repeatedly writing to the
loadstore_waveform device attribute file?
Do we want this applied to -stable?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH][RESEND] static code analysis from cppcheck reports:

[Colin Ian King]

On 12/01/15 18:02, Geert Uytterhoeven wrote:
> On Mon, Jan 12, 2015 at 4:27 PM, Colin King <colin.king@canonical.com> wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> [drivers/video/fbdev/broadsheetfb.c:673]:
>>   (error) Memory leak: sector_buffer
>>
>> sector_buffer is not being kfree'd on each call to
>> broadsheet_spiflash_rewrite_sector(), so free it.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> So you can make the box go OOM by repeatedly writing to the
> loadstore_waveform device attribute file?

..well, not quickly.

> Do we want this applied to -stable?
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

Re: [PATCH][RESEND] static code analysis from cppcheck reports:

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

On 12/01/15 17:27, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> [drivers/video/fbdev/broadsheetfb.c:673]:
>   (error) Memory leak: sector_buffer
> 
> sector_buffer is not being kfree'd on each call to
> broadsheet_spiflash_rewrite_sector(), so free it.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/video/fbdev/broadsheetfb.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Thanks, I've queued this for 3.19 fbdev fixes.

 Tomi




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]