Re: [PATCH] dm-crypt: Reject malformed raw keys in crypt_set_key()

[Thorsten Blum <thorsten.blum@linux.dev>]

On Thu, Aug 13, 2026 at 11:39:27PM +0300, Andy Shevchenko wrote:
> On Thu, Aug 13, 2026 at 11:15 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
> >
> > dm-crypt calculates the raw key size by dividing the hex string length
> > by two. If the string has an odd number of characters, the key size is
> > rounded down and hex2bin() only decodes complete byte pairs.
> >
> > For example, a 33-character raw key string is malformed, but dm-crypt
> > treats it as a 16-byte key and ignores the last character.
> >
> > Reject raw key strings whose length does not match the expected number
> > of hex characters. This restores the trailing character check that was
> > lost when the open-coded decoder was replaced with hex2bin().
> 
> Thanks for the report and the fix.
> 
> ...
> 
> > +       if (cc->key_size && key_string_len != cc->key_size * 2)
> > +               goto out;
> 
> Okay, the original code did two things (differently to the
> implementation with hex2bin() call):
> - if key length is odd and the last character is not NUL, it failed with -EINVAL
> - if the key length is even and the last characters are \n\0, the
> string was parsed normally with the exception that the H\n part
> becomes 0x0H instead of 0xH0 if follow the order
> 
> I dunno if the second was ever supported and not theoretical (so a
> user can forge that one), but this change is only about the first. So
> if we don't care about the second part, the expectation is that the
> key always ends up with the NUL having an odd number of hex digits in
> it. So, why not simply restore that NUL-check?

The result would be the same, but I prefer the length check because it
validates before calling hex2bin() and rejects before modifying cc->key.

> >         /* Decode key from its hex representation. */
> >         if (cc->key_size && hex2bin(cc->key, key, cc->key_size) < 0)
> >                 goto out;