Re: [PATCH] mwifiex: fix memory leak in mwifiex_save_hidden_ssid_channels()

2016-11-09 Thread Brian Norris 
On Wed, Nov 09, 2016 at 11:37:28AM +0800, Ricky Liang wrote:
> kmemleak reports memory leak in mwifiex_save_hidden_ssid_channels():
> 
> unreferenced object 0xffc0a2914780 (size 192):
>   comm "ksdioirqd/mmc2", pid 2004, jiffies 4307182506 (age 820.684s)
>   hex dump (first 32 bytes):
> 00 06 47 49 4e 2d 32 67 01 03 c8 60 6c 03 01 40  ..GIN-2g...`l..@
> 07 10 54 57 20 34 04 1e 64 05 24 84 03 24 95 04  ..TW 4..d.$..$..
>   backtrace:
> [] create_object+0x164/0x2b4
> [] kmemleak_alloc+0x50/0x88
> [] __kmalloc_track_caller+0x1bc/0x264
> [] kmemdup+0x38/0x64
> [] mwifiex_fill_new_bss_desc+0x3c/0x130 [mwifiex]
> [] mwifiex_save_curr_bcn+0x4ec/0x640 [mwifiex]
> [] mwifiex_handle_event_ext_scan_report+0x1d4/0x268 
> [mwifiex]
> [] mwifiex_process_sta_event+0x378/0x898 [mwifiex]
> [] mwifiex_process_event+0x1a8/0x1e8 [mwifiex]
> [] mwifiex_main_process+0x258/0x534 [mwifiex]
> [] 0xffbffc258858
> [] process_sdio_pending_irqs+0xf8/0x160
> [] sdio_irq_thread+0x9c/0x1a4
> [] kthread+0xf4/0x100
> [] ret_from_fork+0xc/0x50
> [] 0x
> 
> Signed-off-by: Ricky Liang 
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c 
> b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 97c9765..98ce072 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1671,6 +1671,10 @@ static int mwifiex_save_hidden_ssid_channels(struct 
> mwifiex_private *priv,
>   }
>  
>  done:
> + /* beacon_ie buffer was allocated in function
> +  * mwifiex_fill_new_bss_desc(). Free it now.
> +  */
> + kfree(bss_desc->beacon_buf);

For a bit, I thought this was possibly a sort of double-free, since
mwifiex_fill_new_bss_desc() might actually fail to allocate
->beacon_buf, but kfree(NULL) is safe, so:

Reviewed-by: Brian Norris 

>   kfree(bss_desc);
>   return 0;
>  }
> -- 
> 2.6.6
>

RE: [PATCH] mwifiex: fix memory leak in mwifiex_save_hidden_ssid_channels()

2016-11-09 Thread Amitkumar Karwar 
> From: Ricky Liang [mailto:jcli...@chromium.org]
> Sent: Wednesday, November 09, 2016 9:07 AM
> Cc: Ricky Liang; Amitkumar Karwar; Nishant Sarmukadam; Kalle Valo; open
> list:MARVELL MWIFIEX WIRELESS DRIVER; open list:NETWORKING DRIVERS; open
> list
> Subject: [PATCH] mwifiex: fix memory leak in
> mwifiex_save_hidden_ssid_channels()
> 
> kmemleak reports memory leak in mwifiex_save_hidden_ssid_channels():
> 
> unreferenced object 0xffc0a2914780 (size 192):
>   comm "ksdioirqd/mmc2", pid 2004, jiffies 4307182506 (age 820.684s)
>   hex dump (first 32 bytes):
> 00 06 47 49 4e 2d 32 67 01 03 c8 60 6c 03 01 40  ..GIN-2g...`l..@
> 07 10 54 57 20 34 04 1e 64 05 24 84 03 24 95 04  ..TW 4..d.$..$..
>   backtrace:
> [] create_object+0x164/0x2b4
> [] kmemleak_alloc+0x50/0x88
> [] __kmalloc_track_caller+0x1bc/0x264
> [] kmemdup+0x38/0x64
> [] mwifiex_fill_new_bss_desc+0x3c/0x130 [mwifiex]
> [] mwifiex_save_curr_bcn+0x4ec/0x640 [mwifiex]
> []
> mwifiex_handle_event_ext_scan_report+0x1d4/0x268 [mwifiex]
> [] mwifiex_process_sta_event+0x378/0x898 [mwifiex]
> [] mwifiex_process_event+0x1a8/0x1e8 [mwifiex]
> [] mwifiex_main_process+0x258/0x534 [mwifiex]
> [] 0xffbffc258858
> [] process_sdio_pending_irqs+0xf8/0x160
> [] sdio_irq_thread+0x9c/0x1a4
> [] kthread+0xf4/0x100
> [] ret_from_fork+0xc/0x50
> [] 0x
> 
> Signed-off-by: Ricky Liang 
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c
> b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 97c9765..98ce072 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1671,6 +1671,10 @@ static int
> mwifiex_save_hidden_ssid_channels(struct mwifiex_private *priv,
>   }
> 
>  done:
> + /* beacon_ie buffer was allocated in function
> +  * mwifiex_fill_new_bss_desc(). Free it now.
> +  */
> + kfree(bss_desc->beacon_buf);
>   kfree(bss_desc);
>   return 0;
>  }

Acked-by: Amitkumar Karwar 

Regards,
Amitkumar