| Message ID | 20200812125825.436733-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v2] crypto: algif_aead - fix uninitialized ctx->init | expand |
On Wed, Aug 12, 2020 at 02:58:25PM +0200, Ondrej Mosnacek wrote: > In skcipher_accept_parent_nokey() the whole af_alg_ctx structure is > cleared by memset() after allocation, so add such memset() also to > aead_accept_parent_nokey() so that the new "init" field is also > initialized to zero. Without that the initial ctx->init checks might > randomly return true and cause errors. > > While there, also remove the redundant zero assignments in both > functions. > > Found via libkcapi testsuite. > > Cc: Stephan Mueller <smueller@chronox.de> > Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when ctx->more is zero") > Suggested-by: Herbert Xu <herbert@gondor.apana.org.au> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > > v2: > - intead add missing memset() to algif_aead and remove the redundant > zero assignments (suggested by Herbert) > > crypto/algif_aead.c | 6 ------ > crypto/algif_skcipher.c | 7 +------ > 2 files changed, 1 insertion(+), 12 deletions(-) Patch applied. Thanks.
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c index d48d2156e6210..43c6aa784858b 100644 --- a/crypto/algif_aead.c +++ b/crypto/algif_aead.c @@ -558,12 +558,6 @@ static int aead_accept_parent_nokey(void *private, struct sock *sk) INIT_LIST_HEAD(&ctx->tsgl_list); ctx->len = len; - ctx->used = 0; - atomic_set(&ctx->rcvused, 0); - ctx->more = 0; - ctx->merge = 0; - ctx->enc = 0; - ctx->aead_assoclen = 0; crypto_init_wait(&ctx->wait); ask->private = ctx; diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index a51ba22fef58f..81c4022285a7c 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -333,6 +333,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk) ctx = sock_kmalloc(sk, len, GFP_KERNEL); if (!ctx) return -ENOMEM; + memset(ctx, 0, len); ctx->iv = sock_kmalloc(sk, crypto_skcipher_ivsize(tfm), GFP_KERNEL); @@ -340,16 +341,10 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk) sock_kfree_s(sk, ctx, len); return -ENOMEM; } - memset(ctx->iv, 0, crypto_skcipher_ivsize(tfm)); INIT_LIST_HEAD(&ctx->tsgl_list); ctx->len = len; - ctx->used = 0; - atomic_set(&ctx->rcvused, 0); - ctx->more = 0; - ctx->merge = 0; - ctx->enc = 0; crypto_init_wait(&ctx->wait); ask->private = ctx;
In skcipher_accept_parent_nokey() the whole af_alg_ctx structure is cleared by memset() after allocation, so add such memset() also to aead_accept_parent_nokey() so that the new "init" field is also initialized to zero. Without that the initial ctx->init checks might randomly return true and cause errors. While there, also remove the redundant zero assignments in both functions. Found via libkcapi testsuite. Cc: Stephan Mueller <smueller@chronox.de> Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when ctx->more is zero") Suggested-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- v2: - intead add missing memset() to algif_aead and remove the redundant zero assignments (suggested by Herbert) crypto/algif_aead.c | 6 ------ crypto/algif_skcipher.c | 7 +------ 2 files changed, 1 insertion(+), 12 deletions(-)