2023-07-13 08:10:38

by Ondrej Mosnáček

[permalink] [raw]
Subject: Another regression in the af_alg series (s390x-specific)

Hi,

It turns out that beneath the first bug [1] there was another one
hiding. It seems to happen only on the s390x architecture when running
the following libkcapi [2] reproducer:

kcapi -x 2 -s -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 \
-k f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c \
-a "" -t 5f24c68cbe6f32c29652442bf5d483ad -q ""

Frequently (but not always) it triggers an oops like this one:

[ 3986.766763] Unable to handle kernel pointer dereference in virtual
kernel address space
[ 3986.766774] Failing address: 0000000a00000000 TEID: 0000000a00000803
[ 3986.766776] Fault in home space mode while using kernel ASCE.
[ 3986.766778] AS:00000000a43a0007 R3:0000000000000024
[ 3986.766825] Oops: 003b ilc:2 [#1] SMP
<snip>
[ 3986.766877] CPU: 0 PID: 271064 Comm: kcapi Tainted: G W
6.5.0-rc1 #1
[ 3986.767070] Hardware name: IBM 8561 LT1 400 (z/VM 7.2.0)
<snip>
[ 3986.767151] Call Trace:
[ 3986.767153] [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
[ 3986.767160] [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
[ 3986.767166] [<00000000a2a60888>] aead_recvmsg+0x478/0x698
[ 3986.767169] [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
[ 3986.767175] [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
[ 3986.767177] [<00000000a273e066>] vfs_read+0x26e/0x2a8
[ 3986.767182] [<00000000a273e8c4>] ksys_read+0xbc/0x100
[ 3986.767184] [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
[ 3986.767189] [<00000000a312ff30>] system_call+0x70/0x98
[ 3986.767193] Last Breaking-Event-Address:
[ 3986.767193] [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
[ 3986.767198] Kernel panic - not syncing: Fatal exception: panic_on_oops

This time the regression was bisected to:

commit c1abe6f570aff4b6d396dc551e60570d2f50bd79
Author: David Howells <[email protected]>
Date: Tue Jun 6 14:08:52 2023 +0100

crypto: af_alg: Use extract_iter_to_sg() to create scatterlists

I can't see what the problem is with the commit, so I'm reporting here
hoping that David or someone else can pick it up from here.

Cheers,
Ondrej

[1] https://lore.kernel.org/linux-crypto/CAAUqJDvFuvms55Td1c=XKv6epfRnnP78438nZQ-JKyuCptGBiQ@mail.gmail.com/T/
[2] https://github.com/smuellerDD/libkcapi/


2023-07-14 02:14:44

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: Another regression in the af_alg series (s390x-specific)

On Thu, Jul 13, 2023 at 10:03:45AM +0200, Ondrej Mosnáček wrote:
> Hi,
>
> It turns out that beneath the first bug [1] there was another one
> hiding. It seems to happen only on the s390x architecture when running
> the following libkcapi [2] reproducer:
>
> kcapi -x 2 -s -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 \
> -k f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c \
> -a "" -t 5f24c68cbe6f32c29652442bf5d483ad -q ""
>
> Frequently (but not always) it triggers an oops like this one:
>
> [ 3986.766763] Unable to handle kernel pointer dereference in virtual
> kernel address space
> [ 3986.766774] Failing address: 0000000a00000000 TEID: 0000000a00000803
> [ 3986.766776] Fault in home space mode while using kernel ASCE.
> [ 3986.766778] AS:00000000a43a0007 R3:0000000000000024
> [ 3986.766825] Oops: 003b ilc:2 [#1] SMP
> <snip>
> [ 3986.766877] CPU: 0 PID: 271064 Comm: kcapi Tainted: G W
> 6.5.0-rc1 #1
> [ 3986.767070] Hardware name: IBM 8561 LT1 400 (z/VM 7.2.0)
> <snip>
> [ 3986.767151] Call Trace:
> [ 3986.767153] [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
> [ 3986.767160] [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
> [ 3986.767166] [<00000000a2a60888>] aead_recvmsg+0x478/0x698
> [ 3986.767169] [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
> [ 3986.767175] [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
> [ 3986.767177] [<00000000a273e066>] vfs_read+0x26e/0x2a8
> [ 3986.767182] [<00000000a273e8c4>] ksys_read+0xbc/0x100
> [ 3986.767184] [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
> [ 3986.767189] [<00000000a312ff30>] system_call+0x70/0x98
> [ 3986.767193] Last Breaking-Event-Address:
> [ 3986.767193] [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
> [ 3986.767198] Kernel panic - not syncing: Fatal exception: panic_on_oops
>
> This time the regression was bisected to:
>
> commit c1abe6f570aff4b6d396dc551e60570d2f50bd79
> Author: David Howells <[email protected]>
> Date: Tue Jun 6 14:08:52 2023 +0100
>
> crypto: af_alg: Use extract_iter_to_sg() to create scatterlists
>
> I can't see what the problem is with the commit, so I'm reporting here
> hoping that David or someone else can pick it up from here.
>

Thanks for the regression report. I'm adding it to regzbot:

#regzbot ^introduced: c1abe6f570aff4b
#regzbot title: kernel pointer dereference regression due to extract_iter_to_sg()

--
An old man doll... just what I always wanted! - Clara


Attachments:
(No filename) (2.47 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-26 10:29:34

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: Another regression in the af_alg series (s390x-specific)

Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.

What's the status wrt to this regression (caused by c1abe6f570af from
David)? It looks like there never was a real reply and the regression
still is unresolved. But maybe I missed something, which can easily
happen in my position.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

On 13.07.23 10:03, Ondrej Mosnáček wrote:
> Hi,
>
> It turns out that beneath the first bug [1] there was another one
> hiding. It seems to happen only on the s390x architecture when running
> the following libkcapi [2] reproducer:
>
> kcapi -x 2 -s -c "gcm(aes)" -i 0d92aa861746b324f20ee6b7 \
> -k f4a6a5e5f2066f6dd9ec6fc5169c29043560ef595c9e81e76f42d29212cc581c \
> -a "" -t 5f24c68cbe6f32c29652442bf5d483ad -q ""
>
> Frequently (but not always) it triggers an oops like this one:
>
> [ 3986.766763] Unable to handle kernel pointer dereference in virtual
> kernel address space
> [ 3986.766774] Failing address: 0000000a00000000 TEID: 0000000a00000803
> [ 3986.766776] Fault in home space mode while using kernel ASCE.
> [ 3986.766778] AS:00000000a43a0007 R3:0000000000000024
> [ 3986.766825] Oops: 003b ilc:2 [#1] SMP
> <snip>
> [ 3986.766877] CPU: 0 PID: 271064 Comm: kcapi Tainted: G W
> 6.5.0-rc1 #1
> [ 3986.767070] Hardware name: IBM 8561 LT1 400 (z/VM 7.2.0)
> <snip>
> [ 3986.767151] Call Trace:
> [ 3986.767153] [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
> [ 3986.767160] [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
> [ 3986.767166] [<00000000a2a60888>] aead_recvmsg+0x478/0x698
> [ 3986.767169] [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
> [ 3986.767175] [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
> [ 3986.767177] [<00000000a273e066>] vfs_read+0x26e/0x2a8
> [ 3986.767182] [<00000000a273e8c4>] ksys_read+0xbc/0x100
> [ 3986.767184] [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
> [ 3986.767189] [<00000000a312ff30>] system_call+0x70/0x98
> [ 3986.767193] Last Breaking-Event-Address:
> [ 3986.767193] [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
> [ 3986.767198] Kernel panic - not syncing: Fatal exception: panic_on_oops
>
> This time the regression was bisected to:
>
> commit c1abe6f570aff4b6d396dc551e60570d2f50bd79
> Author: David Howells <[email protected]>
> Date: Tue Jun 6 14:08:52 2023 +0100
>
> crypto: af_alg: Use extract_iter_to_sg() to create scatterlists
>
> I can't see what the problem is with the commit, so I'm reporting here
> hoping that David or someone else can pick it up from here.
>
> Cheers,
> Ondrej
>
> [1] https://lore.kernel.org/linux-crypto/CAAUqJDvFuvms55Td1c=XKv6epfRnnP78438nZQ-JKyuCptGBiQ@mail.gmail.com/T/
> [2] https://github.com/smuellerDD/libkcapi/

2023-07-26 11:20:19

by David Howells

[permalink] [raw]
Subject: Re: Another regression in the af_alg series (s390x-specific)

"Linux regression tracking (Thorsten Leemhuis)" wrote:

> What's the status wrt to this regression (caused by c1abe6f570af from
> David)? It looks like there never was a real reply and the regression
> still is unresolved. But maybe I missed something, which can easily
> happen in my position.

I was on holiday when the regression was posted. This week I've been working
through various things raised during the last couple of weeks whilst fighting
an intermittent apparent bug on my desktop kernel somewhere in ext4, the mm
subsys, md or dm-crypt.

I'll get round to it, but I'll I don't have s390x h/w immediately to hand.

David


2023-07-26 11:28:50

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: Another regression in the af_alg series (s390x-specific)

On 26.07.23 12:43, David Howells wrote:
> "Linux regression tracking (Thorsten Leemhuis)" wrote:
>
>> What's the status wrt to this regression (caused by c1abe6f570af from
>> David)? It looks like there never was a real reply and the regression
>> still is unresolved. But maybe I missed something, which can easily
>> happen in my position.
>
> I was on holiday when the regression was posted.

Welcome back. And, no worries, I was just wondering what was up here.

> This week I've been working
> through various things raised during the last couple of weeks whilst fighting
> an intermittent apparent bug on my desktop kernel somewhere in ext4, the mm
> subsys, md or dm-crypt.

Good luck with that!

> I'll get round to it, but I'll I don't have s390x h/w immediately to hand.

thx! Ciao, Thorsten

2023-07-26 15:59:40

by David Howells

[permalink] [raw]
Subject: Re: Another regression in the af_alg series (s390x-specific)

Well, I can reproduce it fairly easily. It seems to be:

static inline void scatterwalk_start(struct scatter_walk *walk,
struct scatterlist *sg)
{
walk->sg = sg;
walk->offset = sg->offset; <----
}

Presumably sg is rubbish.

Dump of assembler code for function gcm_walk_start:
0x0000000000000038 <+0>: jgnop 0x38 <gcm_walk_start>
0x000000000000003e <+6>: xc 8(64,%r2),8(%r2)
0x0000000000000044 <+12>: st %r4,32(%r2)
0x0000000000000048 <+16>: stg %r3,0(%r2)
0x000000000000004e <+22>: l %r1,8(%r3)
0x0000000000000052 <+26>: st %r1,8(%r2)
0x0000000000000056 <+30>: jg 0x56 <gcm_walk_start+30>

I'm don't know much about s390x assembly, but I'm guessing %r2 has "walk" and
%r3 has "sg".

AS:0000000116d50007 R3:0000000000000024
Fault in home space mode while using kernel ASCE.
Failing address: 0026070200000000 TEID: 0026070200000803
Unable to handle kernel pointer dereference in virtual kernel address space

Krnl GPRS: 000000000000000c 0000038000000310 00000380002a7938 0026070200000000
0000000000000000 0000000115593cb4 0000000000000000 0000000000000010
0000000100000000 000000017e984690 000000000000000c 0000000000000000
000003ffaf12cf98 0000000000000000 000003ff7fc536ba 00000380002a77e0

I'm not sure what to make of the 0026070200000000.

David


2023-07-26 19:39:46

by Sven Schnelle

[permalink] [raw]
Subject: Re: Another regression in the af_alg series (s390x-specific)

David Howells <[email protected]> writes:

> Well, I can reproduce it fairly easily. It seems to be:
>
> static inline void scatterwalk_start(struct scatter_walk *walk,
> struct scatterlist *sg)
> {
> walk->sg = sg;
> walk->offset = sg->offset; <----
> }
>
> Presumably sg is rubbish.
>
> Dump of assembler code for function gcm_walk_start:
> 0x0000000000000038 <+0>: jgnop 0x38 <gcm_walk_start>
> 0x000000000000003e <+6>: xc 8(64,%r2),8(%r2)
> 0x0000000000000044 <+12>: st %r4,32(%r2)
> 0x0000000000000048 <+16>: stg %r3,0(%r2)
> 0x000000000000004e <+22>: l %r1,8(%r3)
> 0x0000000000000052 <+26>: st %r1,8(%r2)
> 0x0000000000000056 <+30>: jg 0x56 <gcm_walk_start+30>
>
> I'm don't know much about s390x assembly, but I'm guessing %r2 has "walk" and
> %r3 has "sg".

Correct. I looked into this today, and it happens with c1abe6f570af
("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists"),
but not with the commit before. It also only happens with
arch/s390/crypto/aes_s390.c, but not with a generic aes implementation.
I also see the s390 aes driver returning EBADMSG even when it's not
crashing the kernel, so i wonder wether it's another problem in some
error path.

I tried to understand the patch mentioned above, but i never worked with
the crypto API in recent years, so that would require some learning on
my side. Adding Harald, maybe he has some more insight.

> AS:0000000116d50007 R3:0000000000000024
> Fault in home space mode while using kernel ASCE.
> Failing address: 0026070200000000 TEID: 0026070200000803
> Unable to handle kernel pointer dereference in virtual kernel address space
>
> Krnl GPRS: 000000000000000c 0000038000000310 00000380002a7938 0026070200000000
> 0000000000000000 0000000115593cb4 0000000000000000 0000000000000010
> 0000000100000000 000000017e984690 000000000000000c 0000000000000000
> 000003ffaf12cf98 0000000000000000 000003ff7fc536ba 00000380002a77e0
>
> I'm not sure what to make of the 0026070200000000.

Well, propbably just an arbitry value loaded from corrupted memory.

2023-07-26 22:15:51

by David Howells

[permalink] [raw]
Subject: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390


Fix af_alg_alloc_areq() to initialise areq->first_rsgl.sgl.sgt.sgl to point
to the scatterlist array in areq->first_rsgl.sgl.sgl.

Without this, the gcm-aes-s390 driver will oops when it tries to do
gcm_walk_start() on req->dst because req->dst is set to the value of
areq->first_rsgl.sgl.sgl by _aead_recvmsg() calling
aead_request_set_crypt().

The problem comes if an empty ciphertext is passed: the loop in
af_alg_get_rsgl() just passes straight out and doesn't set areq->first_rsgl
up.

This isn't a problem on x86_64 using gcmaes_crypt_by_sg() because, as far
as I can tell, that ignores req->dst and only uses req->src[*].

[*] Is this a bug in aesni-intel_glue.c?

The s390x oops looks something like:

Unable to handle kernel pointer dereference in virtual kernel address space
Failing address: 0000000a00000000 TEID: 0000000a00000803
Fault in home space mode while using kernel ASCE.
AS:00000000a43a0007 R3:0000000000000024
Oops: 003b ilc:2 [#1] SMP
...
Call Trace:
[<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
[<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
[<00000000a2a60888>] aead_recvmsg+0x478/0x698
[<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
[<00000000a2e51a56>] sock_read_iter+0x76/0xa0
[<00000000a273e066>] vfs_read+0x26e/0x2a8
[<00000000a273e8c4>] ksys_read+0xbc/0x100
[<00000000a311d808>] __do_syscall+0x1d0/0x1f8
[<00000000a312ff30>] system_call+0x70/0x98
Last Breaking-Event-Address:
[<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]

Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
Reported-by: Ondrej Mosnáček <[email protected]>
Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
Signed-off-by: David Howells <[email protected]>
cc: Herbert Xu <[email protected]>
cc: Sven Schnelle <[email protected]>
cc: Harald Freudenberger <[email protected]>
cc: "David S. Miller" <[email protected]>
cc: Paolo Abeni <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
crypto/af_alg.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 06b15b9f661c..9ee8575d3b1a 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1192,6 +1192,7 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,

areq->areqlen = areqlen;
areq->sk = sk;
+ areq->first_rsgl.sgl.sgt.sgl = areq->first_rsgl.sgl.sgl;
areq->last_rsgl = NULL;
INIT_LIST_HEAD(&areq->rsgl_list);
areq->tsgl = NULL;


2023-07-27 06:19:19

by Sven Schnelle

[permalink] [raw]
Subject: Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390

David Howells <[email protected]> writes:

>
> Fix af_alg_alloc_areq() to initialise areq->first_rsgl.sgl.sgt.sgl to point
> to the scatterlist array in areq->first_rsgl.sgl.sgl.
>
> Without this, the gcm-aes-s390 driver will oops when it tries to do
> gcm_walk_start() on req->dst because req->dst is set to the value of
> areq->first_rsgl.sgl.sgl by _aead_recvmsg() calling
> aead_request_set_crypt().
>
> The problem comes if an empty ciphertext is passed: the loop in
> af_alg_get_rsgl() just passes straight out and doesn't set areq->first_rsgl
> up.
>
> This isn't a problem on x86_64 using gcmaes_crypt_by_sg() because, as far
> as I can tell, that ignores req->dst and only uses req->src[*].
>
> [*] Is this a bug in aesni-intel_glue.c?
>
> The s390x oops looks something like:
>
> Unable to handle kernel pointer dereference in virtual kernel address space
> Failing address: 0000000a00000000 TEID: 0000000a00000803
> Fault in home space mode while using kernel ASCE.
> AS:00000000a43a0007 R3:0000000000000024
> Oops: 003b ilc:2 [#1] SMP
> ...
> Call Trace:
> [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
> [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
> [<00000000a2a60888>] aead_recvmsg+0x478/0x698
> [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
> [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
> [<00000000a273e066>] vfs_read+0x26e/0x2a8
> [<00000000a273e8c4>] ksys_read+0xbc/0x100
> [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
> [<00000000a312ff30>] system_call+0x70/0x98
> Last Breaking-Event-Address:
> [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
>
> Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
> Reported-by: Ondrej Mosnáček <[email protected]>
> Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> Signed-off-by: David Howells <[email protected]>
> cc: Herbert Xu <[email protected]>
> cc: Sven Schnelle <[email protected]>
> cc: Harald Freudenberger <[email protected]>
> cc: "David S. Miller" <[email protected]>
> cc: Paolo Abeni <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> ---
> crypto/af_alg.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 06b15b9f661c..9ee8575d3b1a 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -1192,6 +1192,7 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,
>
> areq->areqlen = areqlen;
> areq->sk = sk;
> + areq->first_rsgl.sgl.sgt.sgl = areq->first_rsgl.sgl.sgl;
> areq->last_rsgl = NULL;
> INIT_LIST_HEAD(&areq->rsgl_list);
> areq->tsgl = NULL;

Just tested, with this fix the kernel no longer crashes. Thanks!

Tested-by: Sven Schnelle <[email protected]>

2023-07-31 13:54:49

by Ondrej Mosnáček

[permalink] [raw]
Subject: Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390

On Thu, Jul 27, 2023 at 7:55 AM Sven Schnelle <[email protected]> wrote:
>
> David Howells <[email protected]> writes:
>
> >
> > Fix af_alg_alloc_areq() to initialise areq->first_rsgl.sgl.sgt.sgl to point
> > to the scatterlist array in areq->first_rsgl.sgl.sgl.
> >
> > Without this, the gcm-aes-s390 driver will oops when it tries to do
> > gcm_walk_start() on req->dst because req->dst is set to the value of
> > areq->first_rsgl.sgl.sgl by _aead_recvmsg() calling
> > aead_request_set_crypt().
> >
> > The problem comes if an empty ciphertext is passed: the loop in
> > af_alg_get_rsgl() just passes straight out and doesn't set areq->first_rsgl
> > up.
> >
> > This isn't a problem on x86_64 using gcmaes_crypt_by_sg() because, as far
> > as I can tell, that ignores req->dst and only uses req->src[*].
> >
> > [*] Is this a bug in aesni-intel_glue.c?
> >
> > The s390x oops looks something like:
> >
> > Unable to handle kernel pointer dereference in virtual kernel address space
> > Failing address: 0000000a00000000 TEID: 0000000a00000803
> > Fault in home space mode while using kernel ASCE.
> > AS:00000000a43a0007 R3:0000000000000024
> > Oops: 003b ilc:2 [#1] SMP
> > ...
> > Call Trace:
> > [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
> > [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
> > [<00000000a2a60888>] aead_recvmsg+0x478/0x698
> > [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
> > [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
> > [<00000000a273e066>] vfs_read+0x26e/0x2a8
> > [<00000000a273e8c4>] ksys_read+0xbc/0x100
> > [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
> > [<00000000a312ff30>] system_call+0x70/0x98
> > Last Breaking-Event-Address:
> > [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
> >
> > Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
> > Reported-by: Ondrej Mosnáček <[email protected]>
> > Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> > Signed-off-by: David Howells <[email protected]>
> > cc: Herbert Xu <[email protected]>
> > cc: Sven Schnelle <[email protected]>
> > cc: Harald Freudenberger <[email protected]>
> > cc: "David S. Miller" <[email protected]>
> > cc: Paolo Abeni <[email protected]>
> > cc: [email protected]
> > cc: [email protected]
> > cc: [email protected]
> > ---
> > crypto/af_alg.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> > index 06b15b9f661c..9ee8575d3b1a 100644
> > --- a/crypto/af_alg.c
> > +++ b/crypto/af_alg.c
> > @@ -1192,6 +1192,7 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,
> >
> > areq->areqlen = areqlen;
> > areq->sk = sk;
> > + areq->first_rsgl.sgl.sgt.sgl = areq->first_rsgl.sgl.sgl;
> > areq->last_rsgl = NULL;
> > INIT_LIST_HEAD(&areq->rsgl_list);
> > areq->tsgl = NULL;
>
> Just tested, with this fix the kernel no longer crashes. Thanks!
>
> Tested-by: Sven Schnelle <[email protected]>

Same here. Thanks for the fix!

Tested-by: Ondrej Mosnáček <[email protected]>

2023-07-31 14:41:54

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390

On Wed, 26 Jul 2023 at 23:54, David Howells <[email protected]> wrote:
>
>
> Fix af_alg_alloc_areq() to initialise areq->first_rsgl.sgl.sgt.sgl to point
> to the scatterlist array in areq->first_rsgl.sgl.sgl.
>
> Without this, the gcm-aes-s390 driver will oops when it tries to do
> gcm_walk_start() on req->dst because req->dst is set to the value of
> areq->first_rsgl.sgl.sgl by _aead_recvmsg() calling
> aead_request_set_crypt().
>
> The problem comes if an empty ciphertext is passed: the loop in
> af_alg_get_rsgl() just passes straight out and doesn't set areq->first_rsgl
> up.
>
> This isn't a problem on x86_64 using gcmaes_crypt_by_sg() because, as far
> as I can tell, that ignores req->dst and only uses req->src[*].
>
> [*] Is this a bug in aesni-intel_glue.c?
>

It uses req->src directly only for processing the additional
authenticated data (AAD) which contributes to the MAC but not to the
ciphertext. Conceptually, there is no dst only src for this part, and
only the IPsec specific encapsulations of GCM and CCM etc do a plain
memcpy of src to dst (if src and dst do not refer to the same
scatterlist already). Otherwise, the AAD is not considered to be part
of the output.

The actual encryption logic does use both src and dst, but under the
hood (inside the skcipher walk helpers)



> The s390x oops looks something like:
>
> Unable to handle kernel pointer dereference in virtual kernel address space
> Failing address: 0000000a00000000 TEID: 0000000a00000803
> Fault in home space mode while using kernel ASCE.
> AS:00000000a43a0007 R3:0000000000000024
> Oops: 003b ilc:2 [#1] SMP
> ...
> Call Trace:
> [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
> [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
> [<00000000a2a60888>] aead_recvmsg+0x478/0x698
> [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
> [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
> [<00000000a273e066>] vfs_read+0x26e/0x2a8
> [<00000000a273e8c4>] ksys_read+0xbc/0x100
> [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
> [<00000000a312ff30>] system_call+0x70/0x98
> Last Breaking-Event-Address:
> [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
>
> Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
> Reported-by: Ondrej Mosnáček <[email protected]>
> Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> Signed-off-by: David Howells <[email protected]>
> cc: Herbert Xu <[email protected]>
> cc: Sven Schnelle <[email protected]>
> cc: Harald Freudenberger <[email protected]>
> cc: "David S. Miller" <[email protected]>
> cc: Paolo Abeni <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> ---
> crypto/af_alg.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 06b15b9f661c..9ee8575d3b1a 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -1192,6 +1192,7 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,
>
> areq->areqlen = areqlen;
> areq->sk = sk;
> + areq->first_rsgl.sgl.sgt.sgl = areq->first_rsgl.sgl.sgl;
> areq->last_rsgl = NULL;
> INIT_LIST_HEAD(&areq->rsgl_list);
> areq->tsgl = NULL;
>

2023-08-04 09:37:08

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390

On Wed, Jul 26, 2023 at 10:53:19PM +0100, David Howells wrote:
>
> Fix af_alg_alloc_areq() to initialise areq->first_rsgl.sgl.sgt.sgl to point
> to the scatterlist array in areq->first_rsgl.sgl.sgl.
>
> Without this, the gcm-aes-s390 driver will oops when it tries to do
> gcm_walk_start() on req->dst because req->dst is set to the value of
> areq->first_rsgl.sgl.sgl by _aead_recvmsg() calling
> aead_request_set_crypt().
>
> The problem comes if an empty ciphertext is passed: the loop in
> af_alg_get_rsgl() just passes straight out and doesn't set areq->first_rsgl
> up.
>
> This isn't a problem on x86_64 using gcmaes_crypt_by_sg() because, as far
> as I can tell, that ignores req->dst and only uses req->src[*].
>
> [*] Is this a bug in aesni-intel_glue.c?
>
> The s390x oops looks something like:
>
> Unable to handle kernel pointer dereference in virtual kernel address space
> Failing address: 0000000a00000000 TEID: 0000000a00000803
> Fault in home space mode while using kernel ASCE.
> AS:00000000a43a0007 R3:0000000000000024
> Oops: 003b ilc:2 [#1] SMP
> ...
> Call Trace:
> [<000003ff7fc3d47e>] gcm_walk_start+0x16/0x28 [aes_s390]
> [<00000000a2a342f2>] crypto_aead_decrypt+0x9a/0xb8
> [<00000000a2a60888>] aead_recvmsg+0x478/0x698
> [<00000000a2e519a0>] sock_recvmsg+0x70/0xb0
> [<00000000a2e51a56>] sock_read_iter+0x76/0xa0
> [<00000000a273e066>] vfs_read+0x26e/0x2a8
> [<00000000a273e8c4>] ksys_read+0xbc/0x100
> [<00000000a311d808>] __do_syscall+0x1d0/0x1f8
> [<00000000a312ff30>] system_call+0x70/0x98
> Last Breaking-Event-Address:
> [<000003ff7fc3e6b4>] gcm_aes_crypt+0x104/0xa68 [aes_s390]
>
> Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
> Reported-by: Ondrej Mosnáček <[email protected]>
> Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> Signed-off-by: David Howells <[email protected]>
> cc: Herbert Xu <[email protected]>
> cc: Sven Schnelle <[email protected]>
> cc: Harald Freudenberger <[email protected]>
> cc: "David S. Miller" <[email protected]>
> cc: Paolo Abeni <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> ---
> crypto/af_alg.c | 1 +
> 1 file changed, 1 insertion(+)

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt