linux-kernel - Re: [PATCH v2 4/4] platform/mellanox/mlxbf_pka: add _

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: <d4c48d92-e064-c050-d275-2d03f320c98b@linux.intel.com>
Date: Tue, 12 Aug 2025 12:47:44 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Ron Li <xiangrongl@...dia.com>
cc: Hans de Goede <hdegoede@...hat.com>, vadimp@...dia.com, 
    alok.a.tiwari@...cle.com, kblaiech@...dia.com, davthompson@...dia.com, 
    platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/4] platform/mellanox/mlxbf_pka: add __free(kfree)
 to handle memory allocation

On Mon, 11 Aug 2025, Ron Li wrote:

> Used __free(kfree) attribute to call kzalloc for allocating the 'data'

Hi Ron,

Grammar problems, but this patch won't be necessary, see below.

> variable in mlxbf_pka_drv_ring_ioctl(). So that the variable can be
> released automatically.
> 
> Reviewed-by: David Thompson <davthompson@...dia.com>
> Reviewed-by: Khalil Blaiech <kblaiech@...dia.com>
> Signed-off-by: Ron Li <xiangrongl@...dia.com>
> ---
>  drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> index 0f93c1aa7130..2dd773ec1dc3 100644
> --- a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> +++ b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> @@ -3,12 +3,14 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/cdev.h>
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/hw_random.h>
>  #include <linux/idr.h>
>  #include <linux/interrupt.h>
>  #include <linux/iommu.h>
>  #include <linux/ioport.h>
> +#include <linux/kdev_t.h>
>  #include <linux/kernel.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
> @@ -438,7 +440,6 @@ static long mlxbf_pka_drv_ring_ioctl(void *device_data, unsigned int cmd, unsign
>  		struct mlxbf_pka_dev_shim_s *shim;
>  		bool trng_present;
>  		u32 byte_cnt;
> -		u32 *data;
>  		int ret;
>  
>  		shim = ring_dev->ring->shim;
> @@ -456,25 +457,21 @@ static long mlxbf_pka_drv_ring_ioctl(void *device_data, unsigned int cmd, unsign
>  		 */
>  		byte_cnt = round_up(trng_data.count, MLXBF_PKA_TRNG_OUTPUT_CNT);
>  
> -		data = kzalloc(byte_cnt, GFP_KERNEL);
> +		u32 *data __free(kfree) = kzalloc(byte_cnt, GFP_KERNEL);
>  		if (!data)
>  			return -ENOMEM;
>  
>  		trng_present = mlxbf_pka_dev_has_trng(shim);
> -		if (!trng_present) {
> -			kfree(data);
> +		if (!trng_present)
>  			return -EAGAIN;
> -		}
>  
>  		ret = mlxbf_pka_dev_trng_read(shim, data, byte_cnt);
>  		if (ret) {
>  			dev_dbg(ring_dev->device, "TRNG failed %d\n", ret);
> -			kfree(data);
>  			return ret;
>  		}
>  
>  		ret = copy_to_user((void __user *)(trng_data.data), data, trng_data.count);
> -		kfree(data);
>  		return ret ? -EFAULT : 0;
>  	}

Please make the code to use the up-to-date conventions right from the 
start. We try to avoid changing same lines back and forth within the same 
series.

In general, anything newly introduced should use the most recent 
conventions. I might occassionally allow exceptions to this rule for 
a pre-existing file to remain consistent within that file but that
won't apply here.

-- 
 i.