* Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390
2023-07-26 21:53 ` [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390 David Howells
@ 2023-07-27 5:55 ` Sven Schnelle
2023-07-31 13:35 ` Ondrej Mosnáček
2023-07-31 14:18 ` Ard Biesheuvel
2023-08-04 9:11 ` Herbert Xu
2 siblings, 1 reply; 5+ messages in thread
From: Sven Schnelle @ 2023-07-27 5:55 UTC (permalink / raw)
To: David Howells
Cc: =?UTF-8?B?T25kcmVqIE1vc27DocSNZWs=?=, Herbert Xu, Paolo Abeni,
Harald Freudenberger, Bagas Sanjaya, linux-crypto, linux-s390,
netdev, regressions, linux-kernel
David Howells <dhowells@redhat.com> 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 <omosnacek@gmail.com>
> Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: Sven Schnelle <svens@linux.ibm.com>
> cc: Harald Freudenberger <freude@linux.vnet.ibm.com>
> cc: "David S. Miller" <davem@davemloft.net>
> cc: Paolo Abeni <pabeni@redhat.com>
> cc: linux-crypto@vger.kernel.org
> cc: linux-s390@vger.kernel.org
> cc: regressions@lists.linux.dev
> ---
> 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 <svens@linux.ibm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390
2023-07-27 5:55 ` Sven Schnelle
@ 2023-07-31 13:35 ` Ondrej Mosnáček
0 siblings, 0 replies; 5+ messages in thread
From: Ondrej Mosnáček @ 2023-07-31 13:35 UTC (permalink / raw)
To: Sven Schnelle
Cc: David Howells, Herbert Xu, Paolo Abeni, Harald Freudenberger,
Bagas Sanjaya, linux-crypto, linux-s390, netdev, regressions,
linux-kernel
On Thu, Jul 27, 2023 at 7:55 AM Sven Schnelle <svens@linux.ibm.com> wrote:
>
> David Howells <dhowells@redhat.com> 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 <omosnacek@gmail.com>
> > Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: Herbert Xu <herbert@gondor.apana.org.au>
> > cc: Sven Schnelle <svens@linux.ibm.com>
> > cc: Harald Freudenberger <freude@linux.vnet.ibm.com>
> > cc: "David S. Miller" <davem@davemloft.net>
> > cc: Paolo Abeni <pabeni@redhat.com>
> > cc: linux-crypto@vger.kernel.org
> > cc: linux-s390@vger.kernel.org
> > cc: regressions@lists.linux.dev
> > ---
> > 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 <svens@linux.ibm.com>
Same here. Thanks for the fix!
Tested-by: Ondrej Mosnáček <omosnacek@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390
2023-07-26 21:53 ` [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390 David Howells
2023-07-27 5:55 ` Sven Schnelle
@ 2023-07-31 14:18 ` Ard Biesheuvel
2023-08-04 9:11 ` Herbert Xu
2 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2023-07-31 14:18 UTC (permalink / raw)
To: David Howells
Cc: Ondrej Mosnáček, Herbert Xu, Paolo Abeni, Sven Schnelle,
Harald Freudenberger, Bagas Sanjaya, linux-crypto, linux-s390,
netdev, regressions, linux-kernel
On Wed, 26 Jul 2023 at 23:54, David Howells <dhowells@redhat.com> 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 <omosnacek@gmail.com>
> Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: Sven Schnelle <svens@linux.ibm.com>
> cc: Harald Freudenberger <freude@linux.vnet.ibm.com>
> cc: "David S. Miller" <davem@davemloft.net>
> cc: Paolo Abeni <pabeni@redhat.com>
> cc: linux-crypto@vger.kernel.org
> cc: linux-s390@vger.kernel.org
> cc: regressions@lists.linux.dev
> ---
> 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;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390
2023-07-26 21:53 ` [PATCH] crypto: Fix missing initialisation affecting gcm-aes-s390 David Howells
2023-07-27 5:55 ` Sven Schnelle
2023-07-31 14:18 ` Ard Biesheuvel
@ 2023-08-04 9:11 ` Herbert Xu
2 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2023-08-04 9:11 UTC (permalink / raw)
To: David Howells
Cc: =?UTF-8?B?T25kcmVqIE1vc27DocSNZWs=?=, Paolo Abeni, Sven Schnelle,
Harald Freudenberger, Bagas Sanjaya, linux-crypto, linux-s390,
netdev, regressions, linux-kernel
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 <omosnacek@gmail.com>
> Link: https://lore.kernel.org/r/CAAUqJDuRkHE8fPgZJGaKjUjd3QfGwzfumuJBmStPqBhubxyk_A@mail.gmail.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: Sven Schnelle <svens@linux.ibm.com>
> cc: Harald Freudenberger <freude@linux.vnet.ibm.com>
> cc: "David S. Miller" <davem@davemloft.net>
> cc: Paolo Abeni <pabeni@redhat.com>
> cc: linux-crypto@vger.kernel.org
> cc: linux-s390@vger.kernel.org
> cc: regressions@lists.linux.dev
> ---
> crypto/af_alg.c | 1 +
> 1 file changed, 1 insertion(+)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread