Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring

[Mikulas Patocka <mpatocka@redhat.com>]

On Fri, 2 Oct 2026, Lorenz Kofler wrote:

> Hi Mikulas,
> 
> thank you for the review!
> 
> Some notes below.
> 
> >> +#ifdef CONFIG_KEYS
> >> +
> >> +static bool contains_whitespace(const char *str)
> >> +{
> >> +	while (*str)
> >> +		if (isspace(*str++))
> >> +			return true;
> >> +	return false;
> >> +}
> >> +
> >> +static int set_key_user(struct alg_spec *a, struct key *key)
> >> +{
> >> +	const struct user_key_payload *ukp;
> >> +
> >> +	ukp = user_key_payload_locked(key);
> >> +	if (!ukp)
> >> +		return -EKEYREVOKED;
> >> +
> >> +	if (a->key_size != ukp->datalen)
> >> +		return -EINVAL;
> >> +
> >> +	memcpy(a->key, ukp->data, a->key_size);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int set_key_encrypted(struct alg_spec *a, struct key *key)
> >> +{
> >> +	const struct encrypted_key_payload *ekp;
> >> +
> >> +	ekp = key->payload.data[0];
> >> +	if (!ekp)
> >> +		return -EKEYREVOKED;
> >> +
> >> +	if (a->key_size != ekp->decrypted_datalen)
> >> +		return -EINVAL;
> >> +
> >> +	memcpy(a->key, ekp->decrypted_data, a->key_size);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int set_key_trusted(struct alg_spec *a, struct key *key)
> >> +{
> >> +	const struct trusted_key_payload *tkp;
> >> +
> >> +	tkp = key->payload.data[0];
> >> +	if (!tkp)
> >> +		return -EKEYREVOKED;
> >> +
> >> +	if (a->key_size != tkp->key_len)
> >> +		return -EINVAL;
> >> +
> >> +	memcpy(a->key, tkp->key, a->key_size);
> >> +
> >> +	return 0;
> >> +}
> > 
> > These four functions are copied from dm-crypt.c and dm-inlinecrypt.c.
> > Copying code is generally malpattern, they should be unified and moved to
> > an include file (that would be included in all three targets) or to the
> > key management code (that would be called from all three targets).
> > 
> 
> Yes that is the issue I described in the cover letters. But I don't
> actually know which way is the preferred one. Afaik there are now
> three options:
> 
> 1. static inline helpers in a drivers/md header, so dm-crypt and
>    dm-integrity each compile their own copy
> 2. a small library module, similar to dm-bufio, so there is one copy
>    that follows the value (y/m) of dm-crypt and dm-integrity
> 3. integration into key management code
> 
> Please tell me which option you prefer.

Try 3, if not possible then 1. I think that introducing a module with this 
would be overkill.

The "if (!strncmp(key_string, "logon:", key_desc - key_string + 1)) {" 
lines are duplicated as well, so I would refactor them and move them to 
the helper too.

I don't know why dm-inlinecrypt only uses the "logon:" key while dm-crypt 
uses "user:", "encrypted:", "trusted:" as well (Eric - could you 
explain?). So, perhaps, dm-inlinecrypt could be extended to use all four 
key types as well.

I CC'd keyrings maintainers - so, if they have some suggestions or 
objections regarding moving the code there, let them say.

Mikulas