* [PATCH 1/2] nvme-auth: Fix off by one checks
@ 2022-07-18 11:09 Dan Carpenter
2022-07-18 11:10 ` [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key() Dan Carpenter
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Dan Carpenter @ 2022-07-18 11:09 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: linux-kernel, kernel-janitors, linux-nvme
The > ARRAY_SIZE() checks need to be >= ARRAY_SIZE() to prevent reading
one element beyond the end of the arrays.
Fixes: a476416bb57b ("nvme: implement In-Band authentication")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
The MAINTAINERS file needs to be updated for this new code.
drivers/nvme/common/auth.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 0c86ebce59d2..bfb16fec0aed 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -55,7 +55,7 @@ static struct nvme_auth_dhgroup_map {
const char *nvme_auth_dhgroup_name(u8 dhgroup_id)
{
- if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) ||
+ if ((dhgroup_id >= ARRAY_SIZE(dhgroup_map)) ||
!dhgroup_map[dhgroup_id].name ||
!strlen(dhgroup_map[dhgroup_id].name))
return NULL;
@@ -65,7 +65,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_name);
const char *nvme_auth_dhgroup_kpp(u8 dhgroup_id)
{
- if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) ||
+ if ((dhgroup_id >= ARRAY_SIZE(dhgroup_map)) ||
!dhgroup_map[dhgroup_id].kpp ||
!strlen(dhgroup_map[dhgroup_id].kpp))
return NULL;
@@ -113,7 +113,7 @@ static struct nvme_dhchap_hash_map {
const char *nvme_auth_hmac_name(u8 hmac_id)
{
- if ((hmac_id > ARRAY_SIZE(hash_map)) ||
+ if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
!hash_map[hmac_id].hmac ||
!strlen(hash_map[hmac_id].hmac))
return NULL;
@@ -123,7 +123,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_name);
const char *nvme_auth_digest_name(u8 hmac_id)
{
- if ((hmac_id > ARRAY_SIZE(hash_map)) ||
+ if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
!hash_map[hmac_id].digest ||
!strlen(hash_map[hmac_id].digest))
return NULL;
@@ -148,7 +148,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_id);
size_t nvme_auth_hmac_hash_len(u8 hmac_id)
{
- if ((hmac_id > ARRAY_SIZE(hash_map)) ||
+ if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
!hash_map[hmac_id].hmac ||
!strlen(hash_map[hmac_id].hmac))
return 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key()
2022-07-18 11:09 [PATCH 1/2] nvme-auth: Fix off by one checks Dan Carpenter
@ 2022-07-18 11:10 ` Dan Carpenter
2022-07-18 11:13 ` Dan Carpenter
2022-07-22 6:23 ` Hannes Reinecke
2022-07-22 4:48 ` [PATCH 1/2] nvme-auth: Fix off by one checks Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2022-07-18 11:10 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: linux-kernel, kernel-janitors, linux-nvme
A couple of the early error gotos call kfree_sensitive(transformed_key);
before "transformed_key" has been initialized.
Fixes: a476416bb57b ("nvme: implement In-Band authentication")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I wrote this several times in different ways, and kept on doing it
wrong so in the end I wrote it this way. It's a bigger diff, but I
think it's the clearest way to write it.
drivers/nvme/common/auth.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index bfb16fec0aed..1890193c7333 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -278,26 +278,33 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
shash->tfm = key_tfm;
ret = crypto_shash_setkey(key_tfm, key->key, key->len);
if (ret < 0)
- goto out_free_shash;
+ goto out_free_transformed_key;
ret = crypto_shash_init(shash);
if (ret < 0)
- goto out_free_shash;
+ goto out_free_transformed_key;
ret = crypto_shash_update(shash, nqn, strlen(nqn));
if (ret < 0)
- goto out_free_shash;
+ goto out_free_transformed_key;
ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
if (ret < 0)
- goto out_free_shash;
+ goto out_free_transformed_key;
ret = crypto_shash_final(shash, transformed_key);
+ if (ret < 0)
+ goto out_free_transformed_key;
+
+ kfree(shash);
+ crypto_free_shash(key_tfm);
+
+ return transformed_key;
+
+out_free_transformed_key:
+ kfree_sensitive(transformed_key);
out_free_shash:
kfree(shash);
out_free_key:
crypto_free_shash(key_tfm);
- if (ret < 0) {
- kfree_sensitive(transformed_key);
- return ERR_PTR(ret);
- }
- return transformed_key;
+
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(nvme_auth_transform_key);
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key()
2022-07-18 11:10 ` [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key() Dan Carpenter
@ 2022-07-18 11:13 ` Dan Carpenter
2022-07-22 6:23 ` Hannes Reinecke
1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2022-07-18 11:13 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: linux-kernel, kernel-janitors, linux-nvme
Sorry, the subject was supposed to be [PATCH 2/2]. Do I need to resend
for patchwork to accept it?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nvme-auth: Fix off by one checks
2022-07-18 11:09 [PATCH 1/2] nvme-auth: Fix off by one checks Dan Carpenter
2022-07-18 11:10 ` [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key() Dan Carpenter
@ 2022-07-22 4:48 ` Christoph Hellwig
2022-07-22 6:22 ` Hannes Reinecke
2022-07-25 5:37 ` Christoph Hellwig
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2022-07-22 4:48 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Hannes Reinecke, linux-kernel, kernel-janitors, linux-nvme
Hannes, can you review these please?
On Mon, Jul 18, 2022 at 02:09:32PM +0300, Dan Carpenter wrote:
> The > ARRAY_SIZE() checks need to be >= ARRAY_SIZE() to prevent reading
> one element beyond the end of the arrays.
>
> Fixes: a476416bb57b ("nvme: implement In-Band authentication")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> The MAINTAINERS file needs to be updated for this new code.
>
> drivers/nvme/common/auth.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index 0c86ebce59d2..bfb16fec0aed 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -55,7 +55,7 @@ static struct nvme_auth_dhgroup_map {
>
> const char *nvme_auth_dhgroup_name(u8 dhgroup_id)
> {
> - if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) ||
> + if ((dhgroup_id >= ARRAY_SIZE(dhgroup_map)) ||
> !dhgroup_map[dhgroup_id].name ||
> !strlen(dhgroup_map[dhgroup_id].name))
> return NULL;
> @@ -65,7 +65,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_name);
>
> const char *nvme_auth_dhgroup_kpp(u8 dhgroup_id)
> {
> - if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) ||
> + if ((dhgroup_id >= ARRAY_SIZE(dhgroup_map)) ||
> !dhgroup_map[dhgroup_id].kpp ||
> !strlen(dhgroup_map[dhgroup_id].kpp))
> return NULL;
> @@ -113,7 +113,7 @@ static struct nvme_dhchap_hash_map {
>
> const char *nvme_auth_hmac_name(u8 hmac_id)
> {
> - if ((hmac_id > ARRAY_SIZE(hash_map)) ||
> + if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
> !hash_map[hmac_id].hmac ||
> !strlen(hash_map[hmac_id].hmac))
> return NULL;
> @@ -123,7 +123,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_name);
>
> const char *nvme_auth_digest_name(u8 hmac_id)
> {
> - if ((hmac_id > ARRAY_SIZE(hash_map)) ||
> + if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
> !hash_map[hmac_id].digest ||
> !strlen(hash_map[hmac_id].digest))
> return NULL;
> @@ -148,7 +148,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_id);
>
> size_t nvme_auth_hmac_hash_len(u8 hmac_id)
> {
> - if ((hmac_id > ARRAY_SIZE(hash_map)) ||
> + if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
> !hash_map[hmac_id].hmac ||
> !strlen(hash_map[hmac_id].hmac))
> return 0;
> --
> 2.35.1
>
>
---end quoted text---
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nvme-auth: Fix off by one checks
2022-07-18 11:09 [PATCH 1/2] nvme-auth: Fix off by one checks Dan Carpenter
2022-07-18 11:10 ` [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key() Dan Carpenter
2022-07-22 4:48 ` [PATCH 1/2] nvme-auth: Fix off by one checks Christoph Hellwig
@ 2022-07-22 6:22 ` Hannes Reinecke
2022-07-25 5:37 ` Christoph Hellwig
3 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2022-07-22 6:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-kernel, kernel-janitors, linux-nvme
On 7/18/22 13:09, Dan Carpenter wrote:
> The > ARRAY_SIZE() checks need to be >= ARRAY_SIZE() to prevent reading
> one element beyond the end of the arrays.
>
> Fixes: a476416bb57b ("nvme: implement In-Band authentication")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> The MAINTAINERS file needs to be updated for this new code.
>
> drivers/nvme/common/auth.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index 0c86ebce59d2..bfb16fec0aed 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -55,7 +55,7 @@ static struct nvme_auth_dhgroup_map {
>
> const char *nvme_auth_dhgroup_name(u8 dhgroup_id)
> {
> - if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) ||
> + if ((dhgroup_id >= ARRAY_SIZE(dhgroup_map)) ||
> !dhgroup_map[dhgroup_id].name ||
> !strlen(dhgroup_map[dhgroup_id].name))
> return NULL;
> @@ -65,7 +65,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_name);
>
> const char *nvme_auth_dhgroup_kpp(u8 dhgroup_id)
> {
> - if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) ||
> + if ((dhgroup_id >= ARRAY_SIZE(dhgroup_map)) ||
> !dhgroup_map[dhgroup_id].kpp ||
> !strlen(dhgroup_map[dhgroup_id].kpp))
> return NULL;
> @@ -113,7 +113,7 @@ static struct nvme_dhchap_hash_map {
>
> const char *nvme_auth_hmac_name(u8 hmac_id)
> {
> - if ((hmac_id > ARRAY_SIZE(hash_map)) ||
> + if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
> !hash_map[hmac_id].hmac ||
> !strlen(hash_map[hmac_id].hmac))
> return NULL;
> @@ -123,7 +123,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_name);
>
> const char *nvme_auth_digest_name(u8 hmac_id)
> {
> - if ((hmac_id > ARRAY_SIZE(hash_map)) ||
> + if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
> !hash_map[hmac_id].digest ||
> !strlen(hash_map[hmac_id].digest))
> return NULL;
> @@ -148,7 +148,7 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_id);
>
> size_t nvme_auth_hmac_hash_len(u8 hmac_id)
> {
> - if ((hmac_id > ARRAY_SIZE(hash_map)) ||
> + if ((hmac_id >= ARRAY_SIZE(hash_map)) ||
> !hash_map[hmac_id].hmac ||
> !strlen(hash_map[hmac_id].hmac))
> return 0;
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key()
2022-07-18 11:10 ` [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key() Dan Carpenter
2022-07-18 11:13 ` Dan Carpenter
@ 2022-07-22 6:23 ` Hannes Reinecke
1 sibling, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2022-07-22 6:23 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-kernel, kernel-janitors, linux-nvme
On 7/18/22 13:10, Dan Carpenter wrote:
> A couple of the early error gotos call kfree_sensitive(transformed_key);
> before "transformed_key" has been initialized.
>
> Fixes: a476416bb57b ("nvme: implement In-Band authentication")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I wrote this several times in different ways, and kept on doing it
> wrong so in the end I wrote it this way. It's a bigger diff, but I
> think it's the clearest way to write it.
>
> drivers/nvme/common/auth.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index bfb16fec0aed..1890193c7333 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -278,26 +278,33 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
> shash->tfm = key_tfm;
> ret = crypto_shash_setkey(key_tfm, key->key, key->len);
> if (ret < 0)
> - goto out_free_shash;
> + goto out_free_transformed_key;
> ret = crypto_shash_init(shash);
> if (ret < 0)
> - goto out_free_shash;
> + goto out_free_transformed_key;
> ret = crypto_shash_update(shash, nqn, strlen(nqn));
> if (ret < 0)
> - goto out_free_shash;
> + goto out_free_transformed_key;
> ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
> if (ret < 0)
> - goto out_free_shash;
> + goto out_free_transformed_key;
> ret = crypto_shash_final(shash, transformed_key);
> + if (ret < 0)
> + goto out_free_transformed_key;
> +
> + kfree(shash);
> + crypto_free_shash(key_tfm);
> +
> + return transformed_key;
> +
> +out_free_transformed_key:
> + kfree_sensitive(transformed_key);
> out_free_shash:
> kfree(shash);
> out_free_key:
> crypto_free_shash(key_tfm);
> - if (ret < 0) {
> - kfree_sensitive(transformed_key);
> - return ERR_PTR(ret);
> - }
> - return transformed_key;
> +
> + return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(nvme_auth_transform_key);
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] nvme-auth: Fix off by one checks
2022-07-18 11:09 [PATCH 1/2] nvme-auth: Fix off by one checks Dan Carpenter
` (2 preceding siblings ...)
2022-07-22 6:22 ` Hannes Reinecke
@ 2022-07-25 5:37 ` Christoph Hellwig
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2022-07-25 5:37 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Hannes Reinecke, linux-kernel, kernel-janitors, linux-nvme
Thanks,
applied to nvme-5.20.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-07-25 5:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 11:09 [PATCH 1/2] nvme-auth: Fix off by one checks Dan Carpenter
2022-07-18 11:10 ` [PATCH] nvme-auth: Uninitialized variable in nvme_auth_transform_key() Dan Carpenter
2022-07-18 11:13 ` Dan Carpenter
2022-07-22 6:23 ` Hannes Reinecke
2022-07-22 4:48 ` [PATCH 1/2] nvme-auth: Fix off by one checks Christoph Hellwig
2022-07-22 6:22 ` Hannes Reinecke
2022-07-25 5:37 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).