On Thu, Aug 13, 2026 at 11:39:27PM +0300, Andy Shevchenko wrote: > On Thu, Aug 13, 2026 at 11:15 AM Thorsten Blum <[email protected]> > 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;
