* [PATCH] crypto: zero initialize memory allocated via sock_kmalloc
@ 2025-09-23 7:45 Shivani Agarwal
2025-09-23 9:57 ` Herbert Xu
0 siblings, 1 reply; 2+ messages in thread
From: Shivani Agarwal @ 2025-09-23 7:45 UTC (permalink / raw)
To: herbert, davem, smueller, linux-crypto, linux-kernel
Cc: bcm-kernel-feedback-list, ajay.kaher, alexey.makhalov,
tapas.kundu, vamsi-krishna.brahmajosyula, srinidhi.rao,
Shivani Agarwal, stable
Several crypto user API contexts and requests allocated with
sock_kmalloc() were left uninitialized, relying on callers to
set fields explicitly. This resulted in the use of uninitialized
data in certain error paths or when new fields are added in the
future.
The ACVP patches also contain two user-space interface files:
algif_kpp.c and algif_akcipher.c. These too rely on proper
initialization of their context structures.
A particular issue has been observed with the newly added
'inflight' variable introduced in af_alg_ctx by commit:
67b164a871af ("crypto: af_alg - Disallow multiple in-flight AIO requests")
Because the context is not memset to zero after allocation,
the inflight variable has contained garbage values. As a result,
af_alg_alloc_areq() has incorrectly returned -EBUSY randomly when
the garbage value was interpreted as true:
https://github.com/gregkh/linux/blame/master/crypto/af_alg.c#L1209
The check directly tests ctx->inflight without explicitly
comparing against true/false. Since inflight is only ever set to
true or false later, an uninitialized value has triggered
-EBUSY failures. Zero-initializing memory allocated with
sock_kmalloc() ensures inflight and other fields start in a known
state, removing random issues caused by uninitialized data.
Fixes: fe869cdb89c9 ("crypto: algif_hash - User-space interface for hash operations")
Fixes: 5afdfd22e6ba ("crypto: algif_rng - add random number generator support")
Fixes: 2d97591ef43d ("crypto: af_alg - consolidation of duplicate code")
Fixes: 99bd99d3e3a7 ("crypto: algif_skcipher - Fix stream cipher chaining")
Fixes: 67b164a871af ("crypto: af_alg - Disallow multiple in-flight AIO requests")
Cc: stable@vger.kernel.org
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
---
crypto/af_alg.c | 5 ++---
crypto/algif_hash.c | 3 +--
crypto/algif_rng.c | 3 +--
crypto/algif_skcipher.c | 1 +
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index ca6fdcc6c54a..6c271e55f44d 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1212,15 +1212,14 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,
if (unlikely(!areq))
return ERR_PTR(-ENOMEM);
+ memset(areq, 0, areqlen);
+
ctx->inflight = true;
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;
- areq->tsgl_entries = 0;
return areq;
}
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index e3f1a4852737..4d3dfc60a16a 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -416,9 +416,8 @@ static int hash_accept_parent_nokey(void *private, struct sock *sk)
if (!ctx)
return -ENOMEM;
- ctx->result = NULL;
+ memset(ctx, 0, len);
ctx->len = len;
- ctx->more = false;
crypto_init_wait(&ctx->wait);
ask->private = ctx;
diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
index 10c41adac3b1..1a86e40c8372 100644
--- a/crypto/algif_rng.c
+++ b/crypto/algif_rng.c
@@ -248,9 +248,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
if (!ctx)
return -ENOMEM;
+ memset(ctx, 0, len);
ctx->len = len;
- ctx->addtl = NULL;
- ctx->addtl_len = 0;
/*
* No seeding done at that point -- if multiple accepts are
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 125d395c5e00..f4ce5473324f 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -70,6 +70,7 @@ static int algif_skcipher_export(struct sock *sk, struct skcipher_request *req)
if (!ctx->state)
return -ENOMEM;
+ memset(ctx->state, 0, statesize);
err = crypto_skcipher_export(req, ctx->state);
if (err) {
sock_kzfree_s(sk, ctx->state, statesize);
--
2.40.4
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] crypto: zero initialize memory allocated via sock_kmalloc
2025-09-23 7:45 [PATCH] crypto: zero initialize memory allocated via sock_kmalloc Shivani Agarwal
@ 2025-09-23 9:57 ` Herbert Xu
0 siblings, 0 replies; 2+ messages in thread
From: Herbert Xu @ 2025-09-23 9:57 UTC (permalink / raw)
To: Shivani Agarwal
Cc: davem, smueller, linux-crypto, linux-kernel,
bcm-kernel-feedback-list, ajay.kaher, alexey.makhalov,
tapas.kundu, vamsi-krishna.brahmajosyula, srinidhi.rao, stable
On Tue, Sep 23, 2025 at 12:45:15AM -0700, Shivani Agarwal wrote:
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index ca6fdcc6c54a..6c271e55f44d 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -1212,15 +1212,14 @@ struct af_alg_async_req *af_alg_alloc_areq(struct sock *sk,
> if (unlikely(!areq))
> return ERR_PTR(-ENOMEM);
>
> + memset(areq, 0, areqlen);
> +
> ctx->inflight = true;
>
> 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;
> - areq->tsgl_entries = 0;
>
> return areq;
> }
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index e3f1a4852737..4d3dfc60a16a 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -416,9 +416,8 @@ static int hash_accept_parent_nokey(void *private, struct sock *sk)
> if (!ctx)
> return -ENOMEM;
>
> - ctx->result = NULL;
> + memset(ctx, 0, len);
> ctx->len = len;
> - ctx->more = false;
> crypto_init_wait(&ctx->wait);
>
> ask->private = ctx;
> diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> index 10c41adac3b1..1a86e40c8372 100644
> --- a/crypto/algif_rng.c
> +++ b/crypto/algif_rng.c
> @@ -248,9 +248,8 @@ static int rng_accept_parent(void *private, struct sock *sk)
> if (!ctx)
> return -ENOMEM;
>
> + memset(ctx, 0, len);
> ctx->len = len;
> - ctx->addtl = NULL;
> - ctx->addtl_len = 0;
>
> /*
> * No seeding done at that point -- if multiple accepts are
These changes look good.
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 125d395c5e00..f4ce5473324f 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -70,6 +70,7 @@ static int algif_skcipher_export(struct sock *sk, struct skcipher_request *req)
> if (!ctx->state)
> return -ENOMEM;
>
> + memset(ctx->state, 0, statesize);
> err = crypto_skcipher_export(req, ctx->state);
> if (err) {
> sock_kzfree_s(sk, ctx->state, statesize);
But this one should be dropped. The ctx->state will immediately
be overwritten by crypto_skcipher_export. Even if it fails, the
memory is immediately freed so no harm is done.
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] 2+ messages in thread
end of thread, other threads:[~2025-09-23 9:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-23 7:45 [PATCH] crypto: zero initialize memory allocated via sock_kmalloc Shivani Agarwal
2025-09-23 9:57 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®