Re: [PATCH 02/10] nvme: add nvme_auth_generate_psk()

[Hannes Reinecke <hare@suse.de>]

On 1/28/25 09:53, Christoph Hellwig wrote:
>> + * Generate a PSK for TLS as specified in NVMe base specification, section 8.13.5.9:
> 
> Please fix your comment to stay in 80 characters.  The line wrapping
> here makes the text almost unreadable.
> 
>> +int nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len,
>> +		u8 *c1, u8 *c2, size_t hash_len, u8 **ret_psk,size_t *ret_len)
> 
> Missing whitespace before the last size_t.
> 
>> +{
>> +	struct crypto_shash *tfm;
>> +	struct shash_desc *shash;
>> +	u8 *psk;
>> +	const char *hmac_name;
>> +	int ret, psk_len;
>> +
>> +	if (!c1 || !c2) {
>> +		pr_warn("%s: invalid parameter\n", __func__);
>> +		return -EINVAL;
>> +	}
> 
> No a very useful error message.  Given that this is a violation of
> the API contract either a WARN_ON_ONCE or just letting the crypto
> algorithm fail should be fine.
> 
Okay, will just let it fail.

>> +	tfm = crypto_alloc_shash(hmac_name, 0, 0);
>> +	if (IS_ERR(tfm))
>> +		return PTR_ERR(tfm);
>> +
>> +	psk_len = crypto_shash_digestsize(tfm);
>> +	psk = kzalloc(psk_len, GFP_KERNEL);
>> +	if (!psk) {
>> +		ret = -ENOMEM;
>> +		goto out_free_tfm;
>> +	}
>> +
>> +	shash = kmalloc(sizeof(struct shash_desc) +
>> +			crypto_shash_descsize(tfm),
>> +			GFP_KERNEL);
>> +	if (!shash) {
>> +		ret = -ENOMEM;
>> +		goto out_free_psk;
>> +	}
> 
> Is there a good reason this can't use SHASH_DESC_ON_STACK()?
> 
I'll check.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich