PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

[Nick Bowler]

Hi,

After installing Linux 5.8-rc6, it seems cryptsetup can no longer
open LUKS volumes.  Regardless of the entered passphrase (correct
or otherwise), the result is a very unhelpful "Keyslot open failed."
message.

On the kernels which fail, I also noticed that the cryptsetup
benchmark command appears to not be able to determine that any
ciphers are available (output at end of message), possibly for
the same reason.

Bisected to the following commit, which suggests a problem specific
to compat userspace (this is amd64 kernel).  I tested both ia32 and
x32 userspace to confirm the problem.  Reverting this commit on top
of 5.8-rc6 resolves the issue.

Looking at strace output the failing syscall appears to be:

  sendmsg(8, {msg_name=NULL, msg_namelen=0, 
	     msg_iov=[{iov_base=..., iov_len=512}], msg_iovlen=1,
	     msg_control=[{cmsg_len=16, cmsg_level=SOL_ALG,
	     cmsg_type=0x3}, {cmsg_len=32, cmsg_level=SOL_ALG,
	     cmsg_type=0x2}], msg_controllen=48, msg_flags=0}, 0)
	     = -1 EINVAL (Invalid argument)

where fd 8 is the descriptor received after "accept" from the AF_ALG
socket bound to the skcipher algorithm.

  547ce4cfb34cdecfa0ee19c29a5510329a7ac802 is the first bad commit
  commit 547ce4cfb34cdecfa0ee19c29a5510329a7ac802
  Author: Al Viro <viro@zeniv.linux.org.uk>
  Date:   Sun May 31 02:06:55 2020 +0100

      switch cmsghdr_from_user_compat_to_kern() to copy_from_user()

      no point getting compat_cmsghdr field-by-field

      Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
      Signed-off-by: David S. Miller <davem@davemloft.net>

   net/compat.c | 15 ++++++++-------
   1 file changed, 8 insertions(+), 7 deletions(-)

  # cryptsetup open /dev/nvme0n1p2 test
  Enter passphrase for /dev/nvme0n1p2:
  Keyslot open failed.
  
  # cryptsetup benchmark
  # Tests are approximate using memory only (no storage IO).
  PBKDF2-sha1       362077 iterations per second for 256-bit key
  PBKDF2-sha256     503155 iterations per second for 256-bit key
  PBKDF2-sha512     396586 iterations per second for 256-bit key
  PBKDF2-ripemd160  283398 iterations per second for 256-bit key
  PBKDF2-whirlpool  159649 iterations per second for 256-bit key
  argon2i       4 iterations, 111601 memory, 4 parallel threads (CPUs) for 256-bit key (requested 2000 ms time)
  argon2id      4 iterations, 112215 memory, 4 parallel threads (CPUs) for 256-bit key (requested 2000 ms time)
  #     Algorithm |       Key |      Encryption |      Decryption
          aes-cbc        128b               N/A               N/A
      serpent-cbc        128b               N/A               N/A
      twofish-cbc        128b               N/A               N/A
          aes-cbc        256b               N/A               N/A
      serpent-cbc        256b               N/A               N/A
      twofish-cbc        256b               N/A               N/A
          aes-xts        256b               N/A               N/A
      serpent-xts        256b               N/A               N/A
      twofish-xts        256b               N/A               N/A
          aes-xts        512b               N/A               N/A
      serpent-xts        512b               N/A               N/A
      twofish-xts        512b               N/A               N/A

Cheers,
-- 
Nick Bowler

Re: PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

[Al Viro]

On Thu, Jul 23, 2020 at 11:51:01AM -0400, Nick Bowler wrote:
> Hi,
> 
> After installing Linux 5.8-rc6, it seems cryptsetup can no longer
> open LUKS volumes.  Regardless of the entered passphrase (correct
> or otherwise), the result is a very unhelpful "Keyslot open failed."
> message.
> 
> On the kernels which fail, I also noticed that the cryptsetup
> benchmark command appears to not be able to determine that any
> ciphers are available (output at end of message), possibly for
> the same reason.
> 
> Bisected to the following commit, which suggests a problem specific
> to compat userspace (this is amd64 kernel).  I tested both ia32 and
> x32 userspace to confirm the problem.  Reverting this commit on top
> of 5.8-rc6 resolves the issue.
> 
> Looking at strace output the failing syscall appears to be:
> 
>   sendmsg(8, {msg_name=NULL, msg_namelen=0, 
> 	     msg_iov=[{iov_base=..., iov_len=512}], msg_iovlen=1,
> 	     msg_control=[{cmsg_len=16, cmsg_level=SOL_ALG,
> 	     cmsg_type=0x3}, {cmsg_len=32, cmsg_level=SOL_ALG,
> 	     cmsg_type=0x2}], msg_controllen=48, msg_flags=0}, 0)
> 	     = -1 EINVAL (Invalid argument)

Huh?  Just in case - could you verify that on the kernel with that
commit reverted the same sendmsg() succeeds?

[PATCH] Re: PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

[Al Viro]

On Mon, Jul 27, 2020 at 05:05:54PM +0100, Al Viro wrote:
> On Thu, Jul 23, 2020 at 11:51:01AM -0400, Nick Bowler wrote:
> > Hi,
> > 
> > After installing Linux 5.8-rc6, it seems cryptsetup can no longer
> > open LUKS volumes.  Regardless of the entered passphrase (correct
> > or otherwise), the result is a very unhelpful "Keyslot open failed."
> > message.
> > 
> > On the kernels which fail, I also noticed that the cryptsetup
> > benchmark command appears to not be able to determine that any
> > ciphers are available (output at end of message), possibly for
> > the same reason.
> > 
> > Bisected to the following commit, which suggests a problem specific
> > to compat userspace (this is amd64 kernel).  I tested both ia32 and
> > x32 userspace to confirm the problem.  Reverting this commit on top
> > of 5.8-rc6 resolves the issue.
> > 
> > Looking at strace output the failing syscall appears to be:
> > 
> >   sendmsg(8, {msg_name=NULL, msg_namelen=0, 
> > 	     msg_iov=[{iov_base=..., iov_len=512}], msg_iovlen=1,
> > 	     msg_control=[{cmsg_len=16, cmsg_level=SOL_ALG,
> > 	     cmsg_type=0x3}, {cmsg_len=32, cmsg_level=SOL_ALG,
> > 	     cmsg_type=0x2}], msg_controllen=48, msg_flags=0}, 0)
> > 	     = -1 EINVAL (Invalid argument)
> 
> Huh?  Just in case - could you verify that on the kernel with that
> commit reverted the same sendmsg() succeeds?

Oh, fuck...  Please see if the following fixes your reproducer; the braino
is, of course, that instead of fetching ucmsg->cmsg_len into ucmlen we read
the entire thing into cmsg.  Other uses of ucmlen had been replaced with
cmsg.cmsg_len; this one was missed.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/net/compat.c b/net/compat.c
index 5e3041a2c37d..434838bef5f8 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -202,7 +202,7 @@ int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg, struct sock *sk,
 
 		/* Advance. */
 		kcmsg = (struct cmsghdr *)((char *)kcmsg + tmp);
-		ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, ucmlen);
+		ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, cmsg.cmsg_len);
 	}
 
 	/*

Re: PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

[Nick Bowler]

On 2020-07-27, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Jul 23, 2020 at 11:51:01AM -0400, Nick Bowler wrote:
>> Hi,
>>
>> After installing Linux 5.8-rc6, it seems cryptsetup can no longer
>> open LUKS volumes.  Regardless of the entered passphrase (correct
>> or otherwise), the result is a very unhelpful "Keyslot open failed."
>> message.
>>
>> On the kernels which fail, I also noticed that the cryptsetup
>> benchmark command appears to not be able to determine that any
>> ciphers are available (output at end of message), possibly for
>> the same reason.
>>
>> Bisected to the following commit, which suggests a problem specific
>> to compat userspace (this is amd64 kernel).  I tested both ia32 and
>> x32 userspace to confirm the problem.  Reverting this commit on top
>> of 5.8-rc6 resolves the issue.
>>
>> Looking at strace output the failing syscall appears to be:
>>
>>   sendmsg(8, {msg_name=NULL, msg_namelen=0,
>> 	     msg_iov=[{iov_base=..., iov_len=512}], msg_iovlen=1,
>> 	     msg_control=[{cmsg_len=16, cmsg_level=SOL_ALG,
>> 	     cmsg_type=0x3}, {cmsg_len=32, cmsg_level=SOL_ALG,
>> 	     cmsg_type=0x2}], msg_controllen=48, msg_flags=0}, 0)
>> 	     = -1 EINVAL (Invalid argument)
>
> Huh?  Just in case - could you verify that on the kernel with that
> commit reverted the same sendmsg() succeeds?

Seems so; with commit 547ce4cfb34c reverted on top of 5.8-rc6 there is
no such error in the strace output.  This particular syscall seems
to be succeeding:

  sendmsg(8, {msg_name=NULL, msg_namelen=0,
	  msg_iov=[{iov_base=..., iov_len=512}], msg_iovlen=1,
	  msg_control=[{cmsg_len=16, cmsg_level=SOL_ALG,
	  cmsg_type=0x3}, {cmsg_len=32, cmsg_level=SOL_ALG,
	  cmsg_type=0x2}], msg_controllen=48, msg_flags=0}, 0) = 512

Cheers,
  Nick

Re: [PATCH] Re: PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

[Nick Bowler]

On 2020-07-27, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Jul 27, 2020 at 05:05:54PM +0100, Al Viro wrote:
>> On Thu, Jul 23, 2020 at 11:51:01AM -0400, Nick Bowler wrote:
>> > After installing Linux 5.8-rc6, it seems cryptsetup can no longer
>> > open LUKS volumes.  Regardless of the entered passphrase (correct
>> > or otherwise), the result is a very unhelpful "Keyslot open failed."
>> > message.
[...]
> Oh, fuck...  Please see if the following fixes your reproducer; the braino
> is, of course, that instead of fetching ucmsg->cmsg_len into ucmlen we read
> the entire thing into cmsg.  Other uses of ucmlen had been replaced with
> cmsg.cmsg_len; this one was missed.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/net/compat.c b/net/compat.c
> index 5e3041a2c37d..434838bef5f8 100644
> --- a/net/compat.c
> +++ b/net/compat.c
> @@ -202,7 +202,7 @@ int cmsghdr_from_user_compat_to_kern(struct msghdr
> *kmsg, struct sock *sk,
>
>  		/* Advance. */
>  		kcmsg = (struct cmsghdr *)((char *)kcmsg + tmp);
> -		ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, ucmlen);
> +		ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, cmsg.cmsg_len);
>  	}
>
>  	/*

This patch appears to resolve the problem when applied on top of 5.8-rc7.

Thanks,
  Nick

[PATCH net] fix a braino in cmsghdr_from_user_compat_to_kern()

[Al Viro]

	commit 547ce4cfb34c ("switch cmsghdr_from_user_compat_to_kern() to
copy_from_user()") missed one of the places where ucmlen should've been
replaced with cmsg.cmsg_len, now that we are fetching the entire struct
rather than doing it field-by-field.

	As the result, compat sendmsg() with several different-sized cmsg
attached started to fail with EINVAL.  Trivial to fix, fortunately.

Reported-by: Nick Bowler <nbowler@draconx.ca>
Tested-by: Nick Bowler <nbowler@draconx.ca>
Fixes: 547ce4cfb34c ("switch cmsghdr_from_user_compat_to_kern() to copy_from_user()")

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/net/compat.c b/net/compat.c
index 5e3041a2c37d..434838bef5f8 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -202,7 +202,7 @@ int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg, struct sock *sk,
 
 		/* Advance. */
 		kcmsg = (struct cmsghdr *)((char *)kcmsg + tmp);
-		ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, ucmlen);
+		ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, cmsg.cmsg_len);
 	}
 
 	/*

Re: [PATCH net] fix a braino in cmsghdr_from_user_compat_to_kern()

[David Miller]

From: Al Viro <viro@zeniv.linux.org.uk>
Date: Mon, 27 Jul 2020 19:22:20 +0100

> 	commit 547ce4cfb34c ("switch cmsghdr_from_user_compat_to_kern() to
> copy_from_user()") missed one of the places where ucmlen should've been
> replaced with cmsg.cmsg_len, now that we are fetching the entire struct
> rather than doing it field-by-field.
> 
> 	As the result, compat sendmsg() with several different-sized cmsg
> attached started to fail with EINVAL.  Trivial to fix, fortunately.
> 
> Reported-by: Nick Bowler <nbowler@draconx.ca>
> Tested-by: Nick Bowler <nbowler@draconx.ca>
> Fixes: 547ce4cfb34c ("switch cmsghdr_from_user_compat_to_kern() to copy_from_user()")
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied, thanks Al.