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

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

[Thorsten Blum]

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().

Fixes: e944e03e336f ("dm crypt: replace custom implementation of hex2bin()")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/md/dm-crypt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 608b617fb817..b7aa3d893303 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2615,6 +2615,9 @@ static int crypt_set_key(struct crypt_config *cc, char *key)
 	kfree_sensitive(cc->key_string);
 	cc->key_string = NULL;
 
+	if (cc->key_size && key_string_len != cc->key_size * 2)
+		goto out;
+
 	/* Decode key from its hex representation. */
 	if (cc->key_size && hex2bin(cc->key, key, cc->key_size) < 0)
 		goto out;

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

[Andy Shevchenko]

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?

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



-- 
With Best Regards,
Andy Shevchenko

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

[Thorsten Blum]

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;