Re: [PATCH] nvmet: clean up DH group state on initialization failure

[Hannes Reinecke]

On 11/10/26 9:48 AM, Sreeraj S Kurup wrote:
> nvmet_setup_dhgroup() leaves the DH transform and group ID
> initialized when private-key generation, public-key allocation,
> or public-key generation fails.
>
> A subsequent invocation with the same DH group incorrectly
> returns success because the existing transform and group ID
> are treated as proof of successful initialization.
>
> Release the transform and reset the group state on initialization
> failures so that subsequent attempts can properly retry setup.
>
> Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@xxxxxxxxx>
> ---
>   drivers/nvme/target/auth.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
> index ccba84b1344e..7733d50de708 100644
> --- a/drivers/nvme/target/auth.c
> +++ b/drivers/nvme/target/auth.c
> @@ -105,6 +105,10 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)
>   		if (ret < 0) {
>   			pr_debug("%s: ctrl %d failed to generate private key, err %d\n",
>   				 __func__, ctrl->cntlid, ret);
> +			crypto_free_kpp(ctrl->dh_tfm);
> +			ctrl->dh_tfm = NULL;
> +			ctrl->dh_gid = 0;
> +			ctrl->dh_keysize = 0;
>   			kfree_sensitive(ctrl->dh_key);
>   			ctrl->dh_key = NULL;
>   			return ret;
> @@ -115,6 +119,10 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)
>   		if (!ctrl->dh_key) {
>   			pr_warn("ctrl %d failed to allocate public key\n",
>   				ctrl->cntlid);
> +			crypto_free_kpp(ctrl->dh_tfm);
> +			ctrl->dh_tfm = NULL;
> +			ctrl->dh_gid = 0;
> +			ctrl->dh_keysize = 0;
>   			return -ENOMEM;
>   		}
>   		ret = nvme_auth_gen_pubkey(ctrl->dh_tfm, ctrl->dh_key,
> @@ -122,8 +130,12 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)
>   		if (ret < 0) {
>   			pr_warn("ctrl %d failed to generate public key\n",
>   				ctrl->cntlid);
> -			kfree(ctrl->dh_key);
> +			kfree_sensitive(ctrl->dh_key);
>   			ctrl->dh_key = NULL;
> +			crypto_free_kpp(ctrl->dh_tfm);
> +			ctrl->dh_tfm = NULL;
> +			ctrl->dh_gid = 0;
> +			ctrl->dh_keysize = 0;
>   		}
>   	}
>   

Can't we make this a helper?

Cheers,

Hannes
--
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@xxxxxxx                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich