lkml.org 
[lkml]   [2022]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH] nvme-auth: fix input checking
    Date
    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
    \
     
     \ /
      Last update: 2022-07-16 22:40    [W:3.798 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site