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: Wed, 10 Jan 2024 05:41:00 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Markus Elfring <Markus.Elfring@....de>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "kernel-janitors@...r.kernel.org"
	<kernel-janitors@...r.kernel.org>, Dexuan Cui <decui@...rosoft.com>, Haiyang
 Zhang <haiyangz@...rosoft.com>, "K. Y. Srinivasan" <kys@...rosoft.com>, Wei
 Liu <wei.liu@...nel.org>
CC: LKML <linux-kernel@...r.kernel.org>, "cocci@...ia.fr" <cocci@...ia.fr>
Subject: RE: [PATCH] Drivers: hv: vmbus: One function call less in
 create_gpadl_header() after error detection

From: Markus Elfring <Markus.Elfring@....de> Sent: Tuesday, December 26, 2023 11:09 AM
> 
> The kfree() function was called in two cases by
> the create_gpadl_header() function during error handling
> even if the passed variable contained a null pointer.
> This issue was detected by using the Coccinelle software.
> 
> Thus use another label.

Interestingly, there's a third case in this function where
"goto nomem" is done, and in this case, msgbody is NULL.
Does Coccinelle not complain about that case as well?

As I'm sure you know, the code is correct as is, because kfree()
checks for a NULL argument.  So this is really an exercise in
making Coccinelle happy.  To me, the additional label is
incremental complexity for someone to deal with when
reading the code at some time in the future.  So I'd vote for
leaving the code as is.  But it's not a big deal either way.  I
can see you've been cleaning up a lot of Coccinelle-reported
issues across the kernel, most of which result in code
simplifications.  If leaving this unchanged causes you problems,
then I won't object (though perhaps that 3rd "goto nomem"
should be dealt with as well for consistency).

Michael

> 
> Signed-off-by: Markus Elfring <elfring@...rs.sourceforge.net>
> ---
>  drivers/hv/channel.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 56f7e06c673e..4d1bbda895d8 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -336,7 +336,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  			  sizeof(struct gpa_range) + pfncount * sizeof(u64);
>  		msgheader =  kzalloc(msgsize, GFP_KERNEL);
>  		if (!msgheader)
> -			goto nomem;
> +			goto free_body;
> 
>  		INIT_LIST_HEAD(&msgheader->submsglist);
>  		msgheader->msgsize = msgsize;
> @@ -417,7 +417,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  			  sizeof(struct gpa_range) + pagecount * sizeof(u64);
>  		msgheader = kzalloc(msgsize, GFP_KERNEL);
>  		if (msgheader == NULL)
> -			goto nomem;
> +			goto free_body;
> 
>  		INIT_LIST_HEAD(&msgheader->submsglist);
>  		msgheader->msgsize = msgsize;
> @@ -439,6 +439,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  	return 0;
>  nomem:
>  	kfree(msgheader);
> +free_body:
>  	kfree(msgbody);
>  	return -ENOMEM;
>  }
> --
> 2.43.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ