Thread (13 messages) 13 messages, 2 authors, 2016-02-03

Re: [v3 PATCH 1/3] crypto: algif_skcipher - Do not assume that req is unchanged

From: Tadeusz Struk <hidden>
Date: 2016-02-01 21:07:55
Subsystem: crypto api, the rest · Maintainers: Herbert Xu, "David S. Miller", Linus Torvalds

Hi Herbert,
On 02/01/2016 05:08 AM, Herbert Xu wrote:
quoted hunk ↗ jump to hunk
@@ -509,37 +498,42 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
 	struct skcipher_ctx *ctx = ask->private;
+	struct skcipher_tfm *skc = pask->private;
+	struct crypto_skcipher *tfm = skc->skcipher;
 	struct skcipher_sg_list *sgl;
 	struct scatterlist *sg;
 	struct skcipher_async_req *sreq;
 	struct skcipher_request *req;
 	struct skcipher_async_rsgl *last_rsgl = NULL;
 	unsigned int txbufs = 0, len = 0, tx_nents = skcipher_all_sg_nents(ctx);
-	unsigned int reqlen = sizeof(struct skcipher_async_req) +
-				GET_REQ_SIZE(ctx) + GET_IV_SIZE(ctx);
+	unsigned int reqsize = crypto_skcipher_reqsize(tfm);
+	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
 	int err = -ENOMEM;
 	bool mark = false;
+	char *iv;
 
-	lock_sock(sk);
-	req = kmalloc(reqlen, GFP_KERNEL);
-	if (unlikely(!req))
-		goto unlock;
+	sreq = kzalloc(sizeof(*req) + reqsize + ivsize, GFP_KERNEL);
I think this should be:
sreq = kzalloc(sizeof(*sreq) + reqsize + ivsize, GFP_KERNEL);
quoted hunk ↗ jump to hunk
+	if (unlikely(!sreq))
+		goto out;
 
-	sreq = GET_SREQ(req, ctx);
+	req = &sreq->req;
+	iv = (char *)(req + 1) + reqsize;
 	sreq->iocb = msg->msg_iocb;
-	memset(&sreq->first_sgl, '\0', sizeof(struct skcipher_async_rsgl));
 	INIT_LIST_HEAD(&sreq->list);
+	sreq->inflight = &ctx->inflight;
+
+	lock_sock(sk);
 	sreq->tsg = kcalloc(tx_nents, sizeof(*sg), GFP_KERNEL);
-	if (unlikely(!sreq->tsg)) {
-		kfree(req);
+	if (unlikely(!sreq->tsg))
 		goto unlock;
-	}
 	sg_init_table(sreq->tsg, tx_nents);
-	memcpy(sreq->iv, ctx->iv, GET_IV_SIZE(ctx));
-	skcipher_request_set_tfm(req, crypto_skcipher_reqtfm(&ctx->req));
+	memcpy(iv, ctx->iv, ivsize);
+	skcipher_request_set_tfm(req, tfm);
 	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      skcipher_async_cb, sk);
+				      skcipher_async_cb, sreq);
 
 	while (iov_iter_count(&msg->msg_iter)) {
 		struct skcipher_async_rsgl *rsgl;
@@ -615,7 +609,7 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 		sg_mark_end(sreq->tsg + txbufs - 1);
 
 	skcipher_request_set_crypt(req, sreq->tsg, sreq->first_sgl.sgl.sg,
-				   len, sreq->iv);
+				   len, iv);
 	err = ctx->enc ? crypto_skcipher_encrypt(req) :
 			 crypto_skcipher_decrypt(req);
 	if (err == -EINPROGRESS) {
@@ -625,10 +619,11 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 	}
 free:
 	skcipher_free_async_sgls(sreq);
-	kfree(req);
 unlock:
 	skcipher_wmem_wakeup(sk);
 	release_sock(sk);
+	kzfree(sreq);
we can't free sreq here because in case if (err == -EINPROGRESS)
we goto unlock and then we crash in the callback.
+out:
 	return err;
 }
With the following changes on top, Tested-by: Tadeusz Struk [off-list ref]
---
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a692e06..322891a 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -515,7 +515,7 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 	bool mark = false;
 	char *iv;
 
-	sreq = kzalloc(sizeof(*req) + reqsize + ivsize, GFP_KERNEL);
+	sreq = kzalloc(sizeof(*sreq) + reqsize + ivsize, GFP_KERNEL);
 	if (unlikely(!sreq))
 		goto out;
 
@@ -528,7 +528,7 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 	lock_sock(sk);
 	sreq->tsg = kcalloc(tx_nents, sizeof(*sg), GFP_KERNEL);
 	if (unlikely(!sreq->tsg))
-		goto unlock;
+		goto free;
 	sg_init_table(sreq->tsg, tx_nents);
 	memcpy(iv, ctx->iv, ivsize);
 	skcipher_request_set_tfm(req, tfm);
@@ -619,10 +619,10 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 	}
 free:
 	skcipher_free_async_sgls(sreq);
+	kzfree(sreq);
 unlock:
 	skcipher_wmem_wakeup(sk);
 	release_sock(sk);
-	kzfree(sreq);
 out:
 	return err;
 }
Thanks,

-- 
TS
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help