On 09/21/2018 04:20 PM, David Howells wrote:
> Dave Jiang <[email protected]> wrote:
>
>> + depends on KEYS
>
> That needs to be in patch 2 where you create a keyring.
>
>> + char desc[NVDIMM_KEY_DESC_LEN + strlen(NVDIMM_PREFIX)];
>
> You should be using sizeof() not strlen() and do you need + 1 for the NUL
> char?
>
>> + sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
>
> NVDIMM_PREFIX is a constant string. I would recommend either declaring it as
> a const char[] or just sticking it in the format string in place of the %s:
>
> sprintf(desc, NVDIMM_PREFIX "%s", nvdimm->dimm_id);
>
>> + if (!cached_key) {
>> + key_link(nvdimm_keyring, key);
>> + nvdimm->key = key;
>> + key->perm |= KEY_USR_SEARCH;
>> + }
>
> Ummm... You're altering the key permission? That's not really yours to
> change.
What can I do to allow the user app to look up the right key in order to
pass the key id to sysfs? Without the KEY_USR_SEARCH I am not able to
search for that key in the keyring.
>
>> +static int nvdimm_check_key_len(unsigned short len, bool update)
>> +{
>> + if (len == (NVDIMM_PASSPHRASE_LEN * 2) && update)
>> + return 0;
>
> In the cover you said:
>
> - Modified "update" to take two key ids and and use lookup_user_key()
> in order to improve security. (David)
>
> is this a holdover?
No. We are passing in two key ids to sysfs. The new one and the existing
one. I think that was what you suggested to do. With the above issue,
should I only just pass in the new key id since the specific sysfs node
should know which key to update already?
>
> David
>
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
