From 1acbf36cd3585776cdc2abe203648fbefdd13c3c Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Thu, 1 Sep 2016 17:16:44 +0800 Subject: crypto: algif_hash - Handle NULL hashes correctly Right now attempting to read an empty hash simply returns zeroed bytes, this patch corrects this by calling the digest function using an empty input. Reported-by: Russell King - ARM Linux Signed-off-by: Herbert Xu --- crypto/algif_hash.c | 73 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 12 deletions(-) (limited to 'crypto/algif_hash.c') diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c index 68a5ceaa..2d8466f9 100644 --- a/crypto/algif_hash.c +++ b/crypto/algif_hash.c @@ -39,6 +39,37 @@ struct algif_hash_tfm { bool has_key; }; +static int hash_alloc_result(struct sock *sk, struct hash_ctx *ctx) +{ + unsigned ds; + + if (ctx->result) + return 0; + + ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)); + + ctx->result = sock_kmalloc(sk, ds, GFP_KERNEL); + if (!ctx->result) + return -ENOMEM; + + memset(ctx->result, 0, ds); + + return 0; +} + +static void hash_free_result(struct sock *sk, struct hash_ctx *ctx) +{ + unsigned ds; + + if (!ctx->result) + return; + + ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)); + + sock_kzfree_s(sk, ctx->result, ds); + ctx->result = NULL; +} + static int hash_sendmsg(struct socket *sock, struct msghdr *msg, size_t ignored) { @@ -54,6 +85,9 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg, lock_sock(sk); if (!ctx->more) { + if ((msg->msg_flags & MSG_MORE)) + hash_free_result(sk, ctx); + err = af_alg_wait_for_completion(crypto_ahash_init(&ctx->req), &ctx->completion); if (err) @@ -90,6 +124,10 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg, ctx->more = msg->msg_flags & MSG_MORE; if (!ctx->more) { + err = hash_alloc_result(sk, ctx); + if (err) + goto unlock; + ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0); err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req), &ctx->completion); @@ -116,6 +154,13 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page, sg_init_table(ctx->sgl.sg, 1); sg_set_page(ctx->sgl.sg, page, size, offset); + if (!(flags & MSG_MORE)) { + err = hash_alloc_result(sk, ctx); + if (err) + goto unlock; + } else if (!ctx->more) + hash_free_result(sk, ctx); + ahash_request_set_crypt(&ctx->req, ctx->sgl.sg, ctx->result, size); if (!(flags & MSG_MORE)) { @@ -153,6 +198,7 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, struct alg_sock *ask = alg_sk(sk); struct hash_ctx *ctx = ask->private; unsigned ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)); + bool result; int err; if (len > ds) @@ -161,17 +207,29 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, msg->msg_flags |= MSG_TRUNC; lock_sock(sk); + result = ctx->result; + err = hash_alloc_result(sk, ctx); + if (err) + goto unlock; + + ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0); + if (ctx->more) { ctx->more = 0; - ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0); err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req), &ctx->completion); if (err) goto unlock; + } else if (!result) { + err = af_alg_wait_for_completion( + crypto_ahash_digest(&ctx->req), + &ctx->completion); } err = memcpy_to_msg(msg, ctx->result, len); + hash_free_result(sk, ctx); + unlock: release_sock(sk); @@ -394,8 +452,7 @@ static void hash_sock_destruct(struct sock *sk) struct alg_sock *ask = alg_sk(sk); struct hash_ctx *ctx = ask->private; - sock_kzfree_s(sk, ctx->result, - crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req))); + hash_free_result(sk, ctx); sock_kfree_s(sk, ctx, ctx->len); af_alg_release_parent(sk); } @@ -407,20 +464,12 @@ static int hash_accept_parent_nokey(void *private, struct sock *sk) struct algif_hash_tfm *tfm = private; struct crypto_ahash *hash = tfm->hash; unsigned len = sizeof(*ctx) + crypto_ahash_reqsize(hash); - unsigned ds = crypto_ahash_digestsize(hash); ctx = sock_kmalloc(sk, len, GFP_KERNEL); if (!ctx) return -ENOMEM; - ctx->result = sock_kmalloc(sk, ds, GFP_KERNEL); - if (!ctx->result) { - sock_kfree_s(sk, ctx, len); - return -ENOMEM; - } - - memset(ctx->result, 0, ds); - + ctx->result = NULL; ctx->len = len; ctx->more = 0; af_alg_init_completion(&ctx->completion); -- cgit v1.2.3 From 19db05fdfca2b8b655b9d6d7a93387c1dc0d1fd5 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Thu, 17 Nov 2016 22:07:58 +0800 Subject: crypto: algif_hash - Fix NULL hash crash with shash Recently algif_hash has been changed to allow null hashes. This triggers a bug when used with an shash algorithm whereby it will cause a crash during the digest operation. This patch fixes it by avoiding the digest operation and instead doing an init followed by a final which avoids the buggy code in shash. This patch also ensures that the result buffer is freed after an error so that it is not returned as a genuine hash result on the next recv call. The shash/ahash wrapper code will be fixed later to handle this case correctly. Fixes: 1acbf36cd358 ("crypto: algif_hash - Handle NULL hashes correctly") Signed-off-by: Herbert Xu Tested-by: Laura Abbott --- crypto/algif_hash.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'crypto/algif_hash.c') diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c index 2d8466f9..05e21b46 100644 --- a/crypto/algif_hash.c +++ b/crypto/algif_hash.c @@ -214,23 +214,26 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0); - if (ctx->more) { + if (!result) { + err = af_alg_wait_for_completion( + crypto_ahash_init(&ctx->req), + &ctx->completion); + if (err) + goto unlock; + } + + if (!result || ctx->more) { ctx->more = 0; err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req), &ctx->completion); if (err) goto unlock; - } else if (!result) { - err = af_alg_wait_for_completion( - crypto_ahash_digest(&ctx->req), - &ctx->completion); } err = memcpy_to_msg(msg, ctx->result, len); - hash_free_result(sk, ctx); - unlock: + hash_free_result(sk, ctx); release_sock(sk); return err ?: len; -- cgit v1.2.3 From 82b0bae9ba1105ce315d45d8436dfd03b4853d23 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Mon, 21 Nov 2016 15:34:00 +0800 Subject: crypto: algif_hash - Fix result clobbering in recvmsg Recently an init call was added to hash_recvmsg so as to reset the hash state in case a sendmsg call was never made. Unfortunately this ended up clobbering the result if the previous sendmsg was done with a MSG_MORE flag. This patch fixes it by excluding that case when we make the init call. Fixes: 19db05fdfca2 ("algif_hash - Fix NULL hash crash with shash") Reported-by: Patrick Steinhardt Signed-off-by: Herbert Xu --- crypto/algif_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crypto/algif_hash.c') diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c index 05e21b46..d19b09cd 100644 --- a/crypto/algif_hash.c +++ b/crypto/algif_hash.c @@ -214,7 +214,7 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0); - if (!result) { + if (!result && !ctx->more) { err = af_alg_wait_for_completion( crypto_ahash_init(&ctx->req), &ctx->completion); -- cgit v1.2.3