[PATCH 06/13] nvme-keyring: restrict match length for version '1' identifiers

[Sagi Grimberg]

On 07/03/2024 13:35, Hannes Reinecke wrote:
> On 3/7/24 11:49, Sagi Grimberg wrote:
>>
>>
>> On 27/01/2024 11:30, hare at kernel.org wrote:
>>> From: Hannes Reinecke <hare at suse.de>
>>>
>>> TP8018 changed the TLS PSK identifiers to append a PSK hash value,
>>> so to lookup any version '1' identifiers we need to restrict the
>>> match length to exclude the PSK hash value (which we don't have
>>> when looking up keys).
>>>
>>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>>> ---
>>>   drivers/nvme/common/keyring.c | 21 +++++++++++++++++++++
>>>   1 file changed, 21 insertions(+)
>>>
>>> diff --git a/drivers/nvme/common/keyring.c 
>>> b/drivers/nvme/common/keyring.c
>>> index c16f9496643f..51b99b34e100 100644
>>> --- a/drivers/nvme/common/keyring.c
>>> +++ b/drivers/nvme/common/keyring.c
>>> @@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key 
>>> *key,
>>>           return false;
>>>       }
>>>       match_id = match_data->raw_data;
>>> +    if (memcmp(match_id, "NVMe1", 5)) {
>>> +        char *e = (char *)match_id;
>>> +        size_t offset = 0;
>>> +        int n = 0;
>>> +
>>> +        while (*e != ' ' && offset < match_len) {
>>> +            if (*e == ' ') {
>>> +                n++;
>>> +                if (n == 3)
>>> +                    break;
>>> +            }
>>> +            e++;
>>> +            offset++;
>>> +        }
>>> +        if (n != 3) {
>>> +            pr_debug("%s: error parsing '%s'\n",
>>> +                 __func__, match_id);
>>> +            return false;
>>> +        }
>>> +        match_len = offset;
>>> +    }
>>
>> My eyes hurt... I am failing to understand what this even does...
>>
>> You have a concrete example?
>
> Yeah, it's ugly. What I'm trying to do here is to lookup TLS PSK 
> identities which are either in the original format:
>
> NVMe0[RG]0[12] <hostnqn> <subsysnqn>
>
> _or_ the TP8018 TLS PSK format:
>
> NVMe1[RG]0[12] <hostnqn> <subsysnqn> <digest>
>
> given that I only have <hostnqn> and <subsysnqn> as lookup keys.
> So for the memcmp() I'll have to exclude the ' <digest>' bit, which
> happens to the after the third blank space in the identity.
> I'll have a slightly better version queued, but parsing remains ugly.

Shouldn't strtok/strsep simplify what you are trying to do here?