netdev - Re: [PATCH net-next 1/4] qed: Populate nvm image attribute shadow.

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: <20180327130113.GA28037@dev-r-vrt-155.mtr.labs.mlnx>
Date:   Tue, 27 Mar 2018 16:01:13 +0300
From:   Yuval Mintz <yuvalm@...lanox.com>
To:     Sudarsana Reddy Kalluru <sudarsana.kalluru@...ium.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, Ariel.Elior@...ium.com
Subject: Re: [PATCH net-next 1/4] qed: Populate nvm image attribute shadow.

On Mon, Mar 26, 2018 at 03:13:45AM -0700, Sudarsana Reddy Kalluru wrote:
> This patch add support for populating the flash image attributes.
s/add/adds/

[...]

> -int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn,
> -					struct qed_ptt *p_ptt,
> -					struct bist_nvm_image_att *p_image_att,
> +int qed_mcp_bist_nvm_get_image_att(struct qed_hwfn *p_hwfn,
> +				   struct qed_ptt *p_ptt,
> +				   struct bist_nvm_image_att *p_image_att,
>  					u32 image_index)

Indentation seems broken.

>  
> +int qed_mcp_nvm_info_populate(struct qed_hwfn *p_hwfn)
> +{
> +	struct qed_nvm_image_info *nvm_info = &p_hwfn->nvm_info;
> +	struct qed_ptt *p_ptt;
> +	int rc;
> +	u32 i;
> +
> +	p_ptt = qed_ptt_acquire(p_hwfn);
> +	if (!p_ptt) {
> +		DP_ERR(p_hwfn, "failed to acquire ptt\n");
> +		return -EBUSY;
> +	}
> +
> +	/* Acquire from MFW the amount of available images */
> +	nvm_info->num_images = 0;
> +	rc = qed_mcp_bist_nvm_get_num_images(p_hwfn,
> +					     p_ptt, &nvm_info->num_images);
> +	if (rc == -EOPNOTSUPP) {
> +		DP_INFO(p_hwfn, "DRV_MSG_CODE_BIST_TEST is not supported\n");
> +		goto out;
> +	} else if ((rc != 0) || (nvm_info->num_images == 0)) {

rc || !nvm_info->num_images

> +		DP_ERR(p_hwfn, "Failed getting number of images\n");
> +		goto err0;
> +	}
> +
> +	nvm_info->image_att =
> +	    kmalloc(nvm_info->num_images * sizeof(struct bist_nvm_image_att),
> +		    GFP_KERNEL);

Indentation can be better than this.

[...]

> --- a/drivers/net/ethernet/qlogic/qed/qed_selftest.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
> @@ -125,10 +125,11 @@ int qed_selftest_nvram(struct qed_dev *cdev)
>  	}
>  
>  	/* Acquire from MFW the amount of available images */
> -	rc = qed_mcp_bist_nvm_test_get_num_images(p_hwfn, p_ptt, &num_images);
> +	rc = qed_mcp_bist_nvm_get_num_images(p_hwfn, p_ptt, &num_images);
>  	if (rc || !num_images) {
>  		DP_ERR(p_hwfn, "Failed getting number of images\n");
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto err0;

Well, this one is a bug fix [Failure flow currently leaks a PTT entry].
If you don't want to treat it as one that's fine, but I think it
deserves its own patch in the series.

>  	}
>  
>  	/* Iterate over images and validate CRC */
> @@ -136,8 +137,8 @@ int qed_selftest_nvram(struct qed_dev *cdev)
>  		/* This mailbox returns information about the image required for
>  		 * reading it.
>  		 */
> -		rc = qed_mcp_bist_nvm_test_get_image_att(p_hwfn, p_ptt,
> -							 &image_att, i);
> +		rc = qed_mcp_bist_nvm_get_image_att(p_hwfn, p_ptt,
> +						    &image_att, i);
>  		if (rc) {
>  			DP_ERR(p_hwfn,
>  			       "Failed getting image index %d attributes\n",
> -- 
> 1.8.3.1
>