2020-07-23 16:01:40

by Nick Bowler

[permalink] [raw]
Subject: PROBLEM: cryptsetup fails to unlock drive in 5.8-rc6 (regression)

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 <[email protected]>
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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

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


2020-07-27 18:48:07

by Al Viro

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

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?

2020-07-27 18:49:44

by Al Viro

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

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 <[email protected]>
---
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);
}

/*

2020-07-27 18:49:56

by Nick Bowler

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

On 2020-07-27, Al Viro <[email protected]> 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

2020-07-27 19:03:31

by Nick Bowler

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

On 2020-07-27, Al Viro <[email protected]> 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 <[email protected]>
> ---
> 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

2020-07-27 19:14:08

by Al Viro

[permalink] [raw]
Subject: [PATCH net] fix a braino in cmsghdr_from_user_compat_to_kern()

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 <[email protected]>
Tested-by: Nick Bowler <[email protected]>
Fixes: 547ce4cfb34c ("switch cmsghdr_from_user_compat_to_kern() to copy_from_user()")

Signed-off-by: Al Viro <[email protected]>
---
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);
}

/*

2020-07-27 20:26:54

by David Miller

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

From: Al Viro <[email protected]>
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 <[email protected]>
> Tested-by: Nick Bowler <[email protected]>
> Fixes: 547ce4cfb34c ("switch cmsghdr_from_user_compat_to_kern() to copy_from_user()")
>
> Signed-off-by: Al Viro <[email protected]>

Applied, thanks Al.