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

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

[David Howells]

    
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;

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

[Sven Schnelle]

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>

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

[Ondrej Mosnáček]

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>

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

[Ard Biesheuvel]

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;
>

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

[Herbert Xu]

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