Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1208843

Re: [PATCHv2 5/5] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro to avoid possible memory leak

From Arend van Spriel <arend@broadcom.com>
Newsgroups linux.kernel
Subject Re: [PATCHv2 5/5] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro to avoid possible memory leak
Date 2015-08-17 21:50 +0200
Message-ID <pYyzU-a5-19@gated-at.bofh.it> (permalink)
References <pYygy-8f8-11@gated-at.bofh.it> <pYygy-8f8-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 08/17/2015 09:28 PM, Raphaël Beamonte wrote:
> The MACRO_WILC_BUFFER() macro was using a return statement, and didn't

Probable MACRO_WILC_BUFFER should be MALLOC_WILC_BUFFER here.

> take care of possible memory leaks and subsequent bugs when it was failing
> after succeeding some allocations. This patch corrects this behavior.
>
> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com>
> ---
>   drivers/staging/wilc1000/wilc_exported_buf.c | 37 ++++++++++++++++++++--------
>   1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_exported_buf.c b/drivers/staging/wilc1000/wilc_exported_buf.c
> index c9a5943..0f3bdad 100644
> --- a/drivers/staging/wilc1000/wilc_exported_buf.c
> +++ b/drivers/staging/wilc1000/wilc_exported_buf.c
> @@ -8,13 +8,6 @@
>   #define LINUX_TX_SIZE	(64 * 1024)
>   #define WILC1000_FW_SIZE (4 * 1024)
>
> -#define MALLOC_WILC_BUFFER(name, size)	\
> -	exported_ ## name = kmalloc(size, GFP_KERNEL);	  \
> -	if (!exported_ ## name) {   \
> -		printk("fail to alloc: %s memory\n", exported_ ## name);  \
> -		return -ENOBUFS;	\
> -	}
> -
>   /*
>    * Add necessary buffer pointers
>    */
> @@ -45,11 +38,35 @@ static int __init wilc_module_init(void)
>   	/*
>   	 * alloc necessary memory
>   	 */
> -	MALLOC_WILC_BUFFER(g_tx_buf, LINUX_TX_SIZE)
> -	MALLOC_WILC_BUFFER(g_rx_buf, LINUX_RX_SIZE)
> -	MALLOC_WILC_BUFFER(g_fw_buf, WILC1000_FW_SIZE)
> +	exported_g_tx_buf = kmalloc(LINUX_TX_SIZE, GFP_KERNEL);
> +	if (!exported_g_tx_buf) {
> +		pr_err("fail to alloc tx buf");

There is really no need to print an error message here. kmalloc will 
blurb enough info when it fails.

So these buffers are globals? So does this driver support multiple 
devices, ie. how are these used when two wilc1000 supported devices are 
present.

Regards,
Arend

> +		return -ENOMEM;
> +	}
> +
> +	exported_g_rx_buf = kmalloc(LINUX_RX_SIZE, GFP_KERNEL);
> +	if (!exported_g_rx_buf) {
> +		pr_err("fail to alloc rx buf");
> +		goto free_g_tx_buf;
> +	}
> +
> +	exported_g_fw_buf = kmalloc(WILC1000_FW_SIZE, GFP_KERNEL);
> +	if (!exported_g_fw_buf) {
> +		pr_err("fail to alloc fw buf");
> +		goto free_g_rx_buf;
> +	}
>
>   	return 0;
> +
> +free_g_rx_buf:
> +	kfree(exported_g_rx_buf);
> +	exported_g_rx_buf = NULL;
> +
> +free_g_tx_buf:
> +	kfree(exported_g_tx_buf);
> +	exported_g_tx_buf = NULL;
> +
> +	return -ENOMEM;
>   }
>
>   static void __exit wilc_module_deinit(void)
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCHv2 5/5] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro to avoid possible memory leak Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-08-17 21:30 +0200
  Re: [PATCHv2 5/5] staging: wilc1000: replace MALLOC_WILC_BUFFER()  macro to avoid possible memory leak Arend van Spriel <arend@broadcom.com> - 2015-08-17 21:50 +0200
    Re: [PATCHv2 5/5] staging: wilc1000: replace MALLOC_WILC_BUFFER()  macro to avoid possible memory leak Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-08-18 01:20 +0200
    [PATCHv3] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro to avoid possible memory leak Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-08-18 01:20 +0200
      Re: [PATCHv3] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro  to avoid possible memory leak Dan Carpenter <dan.carpenter@oracle.com> - 2015-08-18 01:50 +0200
        Re: [PATCHv3] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro  to avoid possible memory leak Dan Carpenter <dan.carpenter@oracle.com> - 2015-08-18 11:20 +0200
          Re: [PATCHv3] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro  to avoid possible memory leak Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-08-18 19:10 +0200
            Re: [PATCHv3] staging: wilc1000: replace MALLOC_WILC_BUFFER() macro  to avoid possible memory leak Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-08-19 05:00 +0200
              [PATCHv4 0/2] staging: wilc1000: code improvements Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-08-19 05:20 +0200

csiph-web