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

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

[Sreeraj S Kurup]

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@gmail.com>
---
 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;
 		}
 	}
 
-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] nvmet: missing Fixes trailer for nvmet_setup_dhgroup() error
  cleanup
- [Low] nvmet: duplicated DH group cleanup on error in
  nvmet_setup_dhgroup()
--

commit b6a4d390d8e8eebaf9899dae456023c9299ff1cc
Author: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

> nvmet: clean up DH group state on initialization failure
>
> 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@gmail.com>

[Severity: Low]
This fixes incomplete error cleanup in nvmet_setup_dhgroup().  Should the
commit message include:

  Fixes: 7a277c37d352 ("nvmet-auth: Diffie-Hellman key exchange support")

> diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
> index ccba84b1344e0..7733d50de708f 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;
>  		}
>  	}

[Severity: Low]
This isn't a bug, but all three error paths in nvmet_setup_dhgroup() repeat
the same teardown of ctrl->dh_tfm, ctrl->dh_gid, ctrl->dh_keysize and
ctrl->dh_key.  Since kfree_sensitive(NULL) is a no-op, could these branches
jump to a single error label at the end of the function instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110084832.3508-1-sreekuttan2156239@gmail.com?part=1

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@gmail.com>
> ---
>   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@suse.de                                +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