netdev - Re: [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1331882675.4251.10.camel@lb-tlvb-eilong.il.broadcom.com>
Date:	Fri, 16 Mar 2012 09:24:34 +0200
From:	"Eilon Greenstein" <eilong@...adcom.com>
To:	"Michal Schmidt" <mschmidt@...hat.com>
cc:	netdev@...r.kernel.org, "Yuval Mintz" <yuvalmin@...adcom.com>,
	"Yaniv Rosner" <yanivr@...adcom.com>,
	"Dmitry Kravkov" <dmitry@...adcom.com>
Subject: Re: [PATCH net 2/2] bnx2x: fix memory leak in
 bnx2x_init_firmware()

On Fri, 2012-03-16 at 01:08 +0100, Michal Schmidt wrote:
> When cycling the interface down and up, bnx2x_init_firmware() knows that
> the firmware is already loaded, but nevertheless it allocates certain
> arrays anew (init_data, init_ops, init_ops_offsets, iro_arr). The old
> arrays are leaked.
> 
> Fix the leaks by returning early if the firmware was already loaded.
> Because if the firmware is loaded, so are the arrays.
> 
> Signed-off-by: Michal Schmidt <mschmidt@...hat.com>


Acked-by: Eilon Greenstein <eilong@...adcom.com>

Thanks Michal!

> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |   50 ++++++++++-----------
>  1 files changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 00ff62f..b69f876 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -10824,38 +10824,36 @@ do {									\
>  
>  int bnx2x_init_firmware(struct bnx2x *bp)
>  {
> +	const char *fw_file_name;
>  	struct bnx2x_fw_file_hdr *fw_hdr;
>  	int rc;
>  
> +	if (bp->firmware)
> +		return 0;
>  
> -	if (!bp->firmware) {
> -		const char *fw_file_name;
> -
> -		if (CHIP_IS_E1(bp))
> -			fw_file_name = FW_FILE_NAME_E1;
> -		else if (CHIP_IS_E1H(bp))
> -			fw_file_name = FW_FILE_NAME_E1H;
> -		else if (!CHIP_IS_E1x(bp))
> -			fw_file_name = FW_FILE_NAME_E2;
> -		else {
> -			BNX2X_ERR("Unsupported chip revision\n");
> -			return -EINVAL;
> -		}
> -		BNX2X_DEV_INFO("Loading %s\n", fw_file_name);
> +	if (CHIP_IS_E1(bp))
> +		fw_file_name = FW_FILE_NAME_E1;
> +	else if (CHIP_IS_E1H(bp))
> +		fw_file_name = FW_FILE_NAME_E1H;
> +	else if (!CHIP_IS_E1x(bp))
> +		fw_file_name = FW_FILE_NAME_E2;
> +	else {
> +		BNX2X_ERR("Unsupported chip revision\n");
> +		return -EINVAL;
> +	}
> +	BNX2X_DEV_INFO("Loading %s\n", fw_file_name);
>  
> -		rc = request_firmware(&bp->firmware, fw_file_name,
> -				      &bp->pdev->dev);
> -		if (rc) {
> -			BNX2X_ERR("Can't load firmware file %s\n",
> -				  fw_file_name);
> -			goto request_firmware_exit;
> -		}
> +	rc = request_firmware(&bp->firmware, fw_file_name, &bp->pdev->dev);
> +	if (rc) {
> +		BNX2X_ERR("Can't load firmware file %s\n",
> +			  fw_file_name);
> +		goto request_firmware_exit;
> +	}
>  
> -		rc = bnx2x_check_firmware(bp);
> -		if (rc) {
> -			BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
> -			goto request_firmware_exit;
> -		}
> +	rc = bnx2x_check_firmware(bp);
> +	if (rc) {
> +		BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
> +		goto request_firmware_exit;
>  	}
>  
>  	fw_hdr = (struct bnx2x_fw_file_hdr *)bp->firmware->data;




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html