linux-kernel - Re: [PATCH v2] scsi: fcoe: add missed kfree() in an error path

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]

Date:   Mon, 13 Jul 2020 13:53:37 -0400
From:   "Ewan D. Milne" <emilne@...hat.com>
To:     Jing Xiangfeng <jingxiangfeng@...wei.com>, hare@...e.de,
        jejb@...ux.ibm.com, martin.petersen@...cle.com,
        robert.w.love@...el.com, Neerav.Parikh@...el.com,
        Markus.Elfring@....de
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] scsi: fcoe: add missed kfree() in an error path

See below.

On Thu, 2020-07-09 at 20:05 +0800, Jing Xiangfeng wrote:
> fcoe_fdmi_info() misses to call kfree() in an error path.
> Add a label 'free_fdmi' and jump to it.
> 
> Fixes: f07d46bbc9ba ("fcoe: Fix smatch warning in fcoe_fdmi_info
> function")
> Signed-off-by: Jing Xiangfeng <jingxiangfeng@...wei.com>
> ---
>  drivers/scsi/fcoe/fcoe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 25dae9f0b205..a63057a03772 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -830,7 +830,7 @@ static void fcoe_fdmi_info(struct fc_lport
> *lport, struct net_device *netdev)
>  		if (rc) {
>  			printk(KERN_INFO "fcoe: Failed to retrieve FDMI
> "
>  					"information from netdev.\n");
> -			return;
> +			goto free_fdmi;
>  		}
>  
>  		snprintf(fc_host_serial_number(lport->host),
> @@ -868,6 +868,7 @@ static void fcoe_fdmi_info(struct fc_lport
> *lport, struct net_device *netdev)
>  
>  		/* Enable FDMI lport states */
>  		lport->fdmi_enabled = 1;
> +free_fdmi:
>  		kfree(fdmi);
>  	} else {
>  		lport->fdmi_enabled = 0;

Normally I would like to see goto labels for error paths outside
conditionals and at the end of the function.  In this case it would
seem to be cleaner to put an else { } clause in the if (rc) above
around the snprintf() calls.

-Ewan