Messages in this thread Patch in this message | | | From | Tom Rix <> | Subject | [PATCH] nvme-auth: fix input checking | Date | Sat, 16 Jul 2022 16:39:24 -0400 |
| |
clang build fails with this representative error drivers/nvme/common/auth.c:59:31: error: address of array 'dhgroup_map[dhgroup_id].name' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] !dhgroup_map[dhgroup_id].name || ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
Input is checked with this pattern if (i > ARRAY_SIZE(a)) fail return a[i].x
This is an off-by-one bug. Change to if (i < ARRAY_SIZE(a) return a[i].x return fail
By specifying the 'name' element of the nvme_auth_dhgroup_map struct as a constant, pre sized array, it will be never be NULL. So this and similar pointer checks are not needed.
By inspection, none of the strings are zero length. So the strlen() check is also not needed.
The array dhgroup_map[] is unchanging so it should have a const type qualifier.
The hash_map[] array has an additional problem. The zeroth element of the array is not explicitly initialized so it is implicitly zero initialized. The pointer will be valid but be empty strings. Instead of calling strlen(), check for the zeroth element.
Fixes: a476416bb57b ("nvme: implement In-Band authentication") Signed-off-by: Tom Rix <trix@redhat.com> --- drivers/nvme/common/auth.c | 54 +++++++++++++++----------------------- 1 file changed, 21 insertions(+), 33 deletions(-)
diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c index 0c86ebce59d2..b7f7ab37786c 100644 --- a/drivers/nvme/common/auth.c +++ b/drivers/nvme/common/auth.c @@ -35,7 +35,7 @@ u32 nvme_auth_get_seqnum(void) } EXPORT_SYMBOL_GPL(nvme_auth_get_seqnum); -static struct nvme_auth_dhgroup_map { +static const struct nvme_auth_dhgroup_map { const char name[16]; const char kpp[16]; } dhgroup_map[] = { @@ -55,21 +55,17 @@ static struct nvme_auth_dhgroup_map { const char *nvme_auth_dhgroup_name(u8 dhgroup_id) { - if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) || - !dhgroup_map[dhgroup_id].name || - !strlen(dhgroup_map[dhgroup_id].name)) - return NULL; - return dhgroup_map[dhgroup_id].name; + if (dhgroup_id < ARRAY_SIZE(dhgroup_map)) + return dhgroup_map[dhgroup_id].name; + return NULL; } EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_name); const char *nvme_auth_dhgroup_kpp(u8 dhgroup_id) { - if ((dhgroup_id > ARRAY_SIZE(dhgroup_map)) || - !dhgroup_map[dhgroup_id].kpp || - !strlen(dhgroup_map[dhgroup_id].kpp)) - return NULL; - return dhgroup_map[dhgroup_id].kpp; + if (dhgroup_id < ARRAY_SIZE(dhgroup_map)) + return dhgroup_map[dhgroup_id].kpp; + return NULL; } EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_kpp); @@ -78,9 +74,6 @@ u8 nvme_auth_dhgroup_id(const char *dhgroup_name) int i; for (i = 0; i < ARRAY_SIZE(dhgroup_map); i++) { - if (!dhgroup_map[i].name || - !strlen(dhgroup_map[i].name)) - continue; if (!strncmp(dhgroup_map[i].name, dhgroup_name, strlen(dhgroup_map[i].name))) return i; @@ -89,7 +82,7 @@ u8 nvme_auth_dhgroup_id(const char *dhgroup_name) } EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_id); -static struct nvme_dhchap_hash_map { +static const struct nvme_dhchap_hash_map { int len; const char hmac[15]; const char digest[8]; @@ -113,21 +106,19 @@ static struct nvme_dhchap_hash_map { const char *nvme_auth_hmac_name(u8 hmac_id) { - if ((hmac_id > ARRAY_SIZE(hash_map)) || - !hash_map[hmac_id].hmac || - !strlen(hash_map[hmac_id].hmac)) - return NULL; - return hash_map[hmac_id].hmac; + if ((hmac_id > 0) && + (hmac_id < ARRAY_SIZE(hash_map))) + return hash_map[hmac_id].hmac; + return NULL; } EXPORT_SYMBOL_GPL(nvme_auth_hmac_name); const char *nvme_auth_digest_name(u8 hmac_id) { - if ((hmac_id > ARRAY_SIZE(hash_map)) || - !hash_map[hmac_id].digest || - !strlen(hash_map[hmac_id].digest)) - return NULL; - return hash_map[hmac_id].digest; + if ((hmac_id > 0) && + (hmac_id < ARRAY_SIZE(hash_map))) + return hash_map[hmac_id].digest; + return NULL; } EXPORT_SYMBOL_GPL(nvme_auth_digest_name); @@ -135,9 +126,7 @@ u8 nvme_auth_hmac_id(const char *hmac_name) { int i; - for (i = 0; i < ARRAY_SIZE(hash_map); i++) { - if (!hash_map[i].hmac || !strlen(hash_map[i].hmac)) - continue; + for (i = 1; i < ARRAY_SIZE(hash_map); i++) { if (!strncmp(hash_map[i].hmac, hmac_name, strlen(hash_map[i].hmac))) return i; @@ -148,11 +137,10 @@ EXPORT_SYMBOL_GPL(nvme_auth_hmac_id); size_t nvme_auth_hmac_hash_len(u8 hmac_id) { - if ((hmac_id > ARRAY_SIZE(hash_map)) || - !hash_map[hmac_id].hmac || - !strlen(hash_map[hmac_id].hmac)) - return 0; - return hash_map[hmac_id].len; + if ((hmac_id > 0) && + (hmac_id < ARRAY_SIZE(hash_map))) + return hash_map[hmac_id].len; + return 0; } EXPORT_SYMBOL_GPL(nvme_auth_hmac_hash_len); -- 2.27.0
| |