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
