netdev - Re: [PATCH net-next] enic: initialize 'a1' argument to vnic_dev

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] [day] [month] [year] [list]

Date: Wed, 24 Apr 2024 19:00:57 +0100
From: Simon Horman <horms@...nel.org>
To: Satish Kharat <satishkh@...co.com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next] enic: initialize 'a1' argument to vnic_dev_cmd

On Mon, Apr 22, 2024 at 08:51:54PM -0700, Satish Kharat wrote:
> Always initialize the 'a1' argument passed to
> vnic_dev_cmd, keep it consistent.

Hi Satish,

I think this patch also addresses cases where a0 are passed
uninitialised, so probably that should be mentioned too.

Also, I think it would be worth explaining a bit more about
why this change is being made. Does it resolve a bug?
If not, perhaps mention why.

The comments above and below notwithstanding,
I agree that the code changes are correct.

> Signed-off-by: Satish Kharat <satishkh@...co.com>
> ---
>  drivers/net/ethernet/cisco/enic/vnic_dev.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c b/drivers/net/ethernet/cisco/enic/vnic_dev.c
> index 12a83fa1302d..288cbc3fa4fc 100644
> --- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
> +++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
> @@ -718,14 +718,14 @@ int vnic_dev_hang_reset_done(struct vnic_dev *vdev, int *done)
>  
>  int vnic_dev_hang_notify(struct vnic_dev *vdev)
>  {
> -	u64 a0, a1;
> +	u64 a0 = 0, a1 = 0;
>  	int wait = 1000;
>  	return vnic_dev_cmd(vdev, CMD_HANG_NOTIFY, &a0, &a1, wait);
>  }
>  
>  int vnic_dev_get_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)
>  {
> -	u64 a0, a1;
> +	u64 a0 = 0, a1 = 0;
>  	int wait = 1000;
>  	int err, i;
>  
> @@ -1164,7 +1164,7 @@ int vnic_dev_deinit_done(struct vnic_dev *vdev, int *status)
>  
>  int vnic_dev_set_mac_addr(struct vnic_dev *vdev, u8 *mac_addr)
>  {
> -	u64 a0, a1;
> +	u64 a0, a1 = 0;
>  	int wait = 1000;
>  	int i;
>  
> @@ -1230,6 +1230,7 @@ int vnic_dev_classifier(struct vnic_dev *vdev, u8 cmd, u16 *entry,
>  		dma_free_coherent(&vdev->pdev->dev, tlv_size, tlv_va, tlv_pa);
>  	} else if (cmd == CLSF_DEL) {
>  		a0 = *entry;
> +		a1 = 0;

Assuming this is not a bug fix, while we are here the scope of the
declaration of a0 and a1 could be reduced to this block.

>  		ret = vnic_dev_cmd(vdev, CMD_DEL_FILTER, &a0, &a1, wait);
>  	}
>  
> -- 
> 2.44.0
> 
>