linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] crypto: af_alg - add async support to algif_aead
@ 2016-01-15 19:21 Tadeusz Struk
  2016-01-17 15:07 ` Stephan Mueller
  0 siblings, 1 reply; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-15 19:21 UTC (permalink / raw)
  To: herbert; +Cc: smueller, linux-kernel, linux-crypto, tadeusz.struk

Following the async change for algif_skcipher
this patch adds similar async read to algif_aead.

Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 crypto/algif_aead.c |  196 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 189 insertions(+), 7 deletions(-)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index f70bcf8..6130759 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -44,6 +44,7 @@ struct aead_ctx {
 	struct af_alg_completion completion;
 
 	unsigned long used;
+	atomic_t inflight;
 
 	unsigned int len;
 	bool more;
@@ -54,6 +55,15 @@ struct aead_ctx {
 	struct aead_request aead_req;
 };
 
+struct aead_async_req {
+	struct kiocb *iocb;
+	struct af_alg_sgl rsgl[RSGL_MAX_ENTRIES];
+	struct scatterlist tsgl[ALG_MAX_PAGES];
+	unsigned int rsgls;
+	unsigned int tsgls;
+	char iv[];
+};
+
 static inline int aead_sndbuf(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -75,6 +85,17 @@ static inline bool aead_sufficient_data(struct aead_ctx *ctx)
 	return ctx->used >= ctx->aead_assoclen + as;
 }
 
+static void aead_reset_ctx(struct aead_ctx *ctx)
+{
+	struct aead_sg_list *sgl = &ctx->tsgl;
+
+	sg_init_table(sgl->sg, ALG_MAX_PAGES);
+	sgl->cur = 0;
+	ctx->used = 0;
+	ctx->more = 0;
+	ctx->merge = 0;
+}
+
 static void aead_put_sgl(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -90,11 +111,6 @@ static void aead_put_sgl(struct sock *sk)
 		put_page(sg_page(sg + i));
 		sg_assign_page(sg + i, NULL);
 	}
-	sg_init_table(sg, ALG_MAX_PAGES);
-	sgl->cur = 0;
-	ctx->used = 0;
-	ctx->more = 0;
-	ctx->merge = 0;
 }
 
 static void aead_wmem_wakeup(struct sock *sk)
@@ -240,6 +256,7 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		if (!aead_writable(sk)) {
 			/* user space sent too much data */
 			aead_put_sgl(sk);
+			aead_reset_ctx(ctx);
 			err = -EMSGSIZE;
 			goto unlock;
 		}
@@ -251,6 +268,7 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 
 			if (sgl->cur >= ALG_MAX_PAGES) {
 				aead_put_sgl(sk);
+				aead_reset_ctx(ctx);
 				err = -E2BIG;
 				goto unlock;
 			}
@@ -287,6 +305,7 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	ctx->more = msg->msg_flags & MSG_MORE;
 	if (!ctx->more && !aead_sufficient_data(ctx)) {
 		aead_put_sgl(sk);
+		aead_reset_ctx(ctx);
 		err = -EMSGSIZE;
 	}
 
@@ -322,6 +341,7 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 	if (!aead_writable(sk)) {
 		/* user space sent too much data */
 		aead_put_sgl(sk);
+		aead_reset_ctx(ctx);
 		err = -EMSGSIZE;
 		goto unlock;
 	}
@@ -339,6 +359,7 @@ done:
 	ctx->more = flags & MSG_MORE;
 	if (!ctx->more && !aead_sufficient_data(ctx)) {
 		aead_put_sgl(sk);
+		aead_reset_ctx(ctx);
 		err = -EMSGSIZE;
 	}
 
@@ -349,7 +370,144 @@ unlock:
 	return err ?: size;
 }
 
-static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored, int flags)
+#define GET_ASYM_REQ(req, tfm) (struct aead_async_req *) \
+		((char *)req + crypto_aead_reqsize(tfm))
+
+static void aead_async_cb(struct crypto_async_request *req, int err)
+{
+	struct sock *sk = req->data;
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
+	struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
+	struct scatterlist *sg = areq->tsgl;
+	struct kiocb *iocb = areq->iocb;
+	int i;
+
+	atomic_dec(&ctx->inflight);
+	for (i = 0; i < areq->tsgls; i++)
+		put_page(sg_page(sg + i));
+
+	for (i = 0; i < areq->rsgls; i++)
+		af_alg_free_sg(&areq->rsgl[i]);
+
+	kfree(req);
+	iocb->ki_complete(iocb, err, err);
+}
+
+static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
+			      int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
+	struct aead_async_req *areq;
+	struct aead_request *req = NULL;
+	struct aead_sg_list *sgl = &ctx->tsgl;
+	unsigned int as = crypto_aead_authsize(tfm);
+	unsigned int reqlen = sizeof(*areq) + crypto_aead_reqsize(tfm) +
+				crypto_aead_ivsize(tfm);
+	int err = -ENOMEM, i;
+	unsigned long used;
+	size_t outlen;
+	size_t usedpages = 0;
+	unsigned int cnt = 0;
+
+	/* Limit number of IOV blocks to be accessed below */
+	if (msg->msg_iter.nr_segs > RSGL_MAX_ENTRIES)
+		return -ENOMSG;
+
+	lock_sock(sk);
+
+	if (ctx->more) {
+		err = aead_wait_for_data(sk, flags);
+		if (err)
+			goto unlock;
+	}
+
+	used = ctx->used;
+	outlen = used;
+
+	if (!aead_sufficient_data(ctx))
+		goto unlock;
+
+	req = kmalloc(reqlen, GFP_KERNEL);
+	if (unlikely(!req))
+		goto unlock;
+
+	areq = GET_ASYM_REQ(req, tfm);
+	areq->iocb = msg->msg_iocb;
+	memcpy(areq->iv, ctx->iv, crypto_aead_ivsize(tfm));
+	aead_request_set_tfm(req, tfm);
+	aead_request_set_ad(req, ctx->aead_assoclen);
+	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				  aead_async_cb, sk);
+	used -= ctx->aead_assoclen + (ctx->enc ? as : 0);
+
+	/* take over all tx sgls from ctx */
+	for (i = 0; i < sgl->cur; i++)
+		areq->tsgl[i] = sgl->sg[i];
+
+	sg_mark_end(areq->tsgl + sgl->cur - 1);
+	areq->tsgls = sgl->cur;
+
+	/* create rx sgls */
+	while (iov_iter_count(&msg->msg_iter)) {
+		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
+				      (outlen - usedpages));
+
+		/* make one iovec available as scatterlist */
+		err = af_alg_make_sg(&areq->rsgl[cnt], &msg->msg_iter, seglen);
+		if (err < 0)
+			goto free;
+		usedpages += err;
+		/* chain the new scatterlist with previous one */
+		if (cnt)
+			af_alg_link_sg(&areq->rsgl[cnt - 1], &areq->rsgl[cnt]);
+
+		/* we do not need more iovecs as we have sufficient memory */
+		if (outlen <= usedpages)
+			break;
+		iov_iter_advance(&msg->msg_iter, err);
+		cnt++;
+	}
+	areq->rsgls = cnt;
+	err = -EINVAL;
+	/* ensure output buffer is sufficiently large */
+	if (usedpages < outlen)
+		goto free;
+
+	aead_request_set_crypt(req, areq->tsgl, areq->rsgl[0].sg, used,
+			       areq->iv);
+	err = ctx->enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+	if (err) {
+		if (err == -EINPROGRESS) {
+			atomic_inc(&ctx->inflight);
+			err = -EIOCBQUEUED;
+			aead_reset_ctx(ctx);
+			goto unlock;
+		} else if (err == -EBADMSG) {
+			aead_put_sgl(sk);
+			aead_reset_ctx(ctx);
+			goto free;
+		}
+		goto free;
+	}
+	aead_put_sgl(sk);
+	aead_reset_ctx(ctx);
+free:
+	for (i = 0; i < cnt; i++)
+		af_alg_free_sg(&areq->rsgl[i]);
+
+	kfree(req);
+unlock:
+	aead_wmem_wakeup(sk);
+	release_sock(sk);
+	return err ? err : outlen;
+}
+
+static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
@@ -452,12 +610,15 @@ static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
 
 	if (err) {
 		/* EBADMSG implies a valid cipher operation took place */
-		if (err == -EBADMSG)
+		if (err == -EBADMSG) {
 			aead_put_sgl(sk);
+			aead_reset_ctx(ctx);
+		}
 		goto unlock;
 	}
 
 	aead_put_sgl(sk);
+	aead_reset_ctx(ctx);
 
 	err = 0;
 
@@ -471,6 +632,14 @@ unlock:
 	return err ? err : outlen;
 }
 
+static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
+			int flags)
+{
+	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
+		aead_recvmsg_async(sock, msg, flags) :
+		aead_recvmsg_sync(sock, msg, flags);
+}
+
 static unsigned int aead_poll(struct file *file, struct socket *sock,
 			      poll_table *wait)
 {
@@ -533,6 +702,16 @@ static int aead_setkey(void *private, const u8 *key, unsigned int keylen)
 	return crypto_aead_setkey(private, key, keylen);
 }
 
+static void aead_wait(struct sock *sk)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	int ctr = 0;
+
+	while (atomic_read(&ctx->inflight) && ctr++ < 100)
+		msleep(100);
+}
+
 static void aead_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -540,6 +719,9 @@ static void aead_sock_destruct(struct sock *sk)
 	unsigned int ivlen = crypto_aead_ivsize(
 				crypto_aead_reqtfm(&ctx->aead_req));
 
+	if (atomic_read(&ctx->inflight))
+		aead_wait(sk);
+
 	aead_put_sgl(sk);
 	sock_kzfree_s(sk, ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx, ctx->len);

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-15 19:21 [PATCH] crypto: af_alg - add async support to algif_aead Tadeusz Struk
@ 2016-01-17 15:07 ` Stephan Mueller
  2016-01-18 15:22   ` Tadeusz Struk
  0 siblings, 1 reply; 14+ messages in thread
From: Stephan Mueller @ 2016-01-17 15:07 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: herbert, linux-kernel, linux-crypto

Am Freitag, 15. Januar 2016, 11:21:12 schrieb Tadeusz Struk:

Hi Tadeusz,

> Following the async change for algif_skcipher
> this patch adds similar async read to algif_aead.
> 
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
>  crypto/algif_aead.c |  196
> +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 189
> insertions(+), 7 deletions(-)
> 
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index f70bcf8..6130759 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -44,6 +44,7 @@ struct aead_ctx {
>  	struct af_alg_completion completion;
> 
>  	unsigned long used;
> +	atomic_t inflight;
> 
>  	unsigned int len;
>  	bool more;
> @@ -54,6 +55,15 @@ struct aead_ctx {
>  	struct aead_request aead_req;
>  };
> 
> +struct aead_async_req {
> +	struct kiocb *iocb;
> +	struct af_alg_sgl rsgl[RSGL_MAX_ENTRIES];
> +	struct scatterlist tsgl[ALG_MAX_PAGES];
> +	unsigned int rsgls;
> +	unsigned int tsgls;
> +	char iv[];
> +};
> +
>  static inline int aead_sndbuf(struct sock *sk)
>  {
>  	struct alg_sock *ask = alg_sk(sk);
> @@ -75,6 +85,17 @@ static inline bool aead_sufficient_data(struct aead_ctx
> *ctx) return ctx->used >= ctx->aead_assoclen + as;
>  }
> 
> +static void aead_reset_ctx(struct aead_ctx *ctx)
> +{
> +	struct aead_sg_list *sgl = &ctx->tsgl;
> +
> +	sg_init_table(sgl->sg, ALG_MAX_PAGES);
> +	sgl->cur = 0;
> +	ctx->used = 0;
> +	ctx->more = 0;
> +	ctx->merge = 0;
> +}
> +
>  static void aead_put_sgl(struct sock *sk)
>  {
>  	struct alg_sock *ask = alg_sk(sk);
> @@ -90,11 +111,6 @@ static void aead_put_sgl(struct sock *sk)
>  		put_page(sg_page(sg + i));
>  		sg_assign_page(sg + i, NULL);
>  	}
> -	sg_init_table(sg, ALG_MAX_PAGES);
> -	sgl->cur = 0;
> -	ctx->used = 0;
> -	ctx->more = 0;
> -	ctx->merge = 0;
>  }
> 
>  static void aead_wmem_wakeup(struct sock *sk)
> @@ -240,6 +256,7 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size) if (!aead_writable(sk)) {
>  			/* user space sent too much data */
>  			aead_put_sgl(sk);
> +			aead_reset_ctx(ctx);
>  			err = -EMSGSIZE;
>  			goto unlock;
>  		}
> @@ -251,6 +268,7 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size)
> 
>  			if (sgl->cur >= ALG_MAX_PAGES) {
>  				aead_put_sgl(sk);
> +				aead_reset_ctx(ctx);
>  				err = -E2BIG;
>  				goto unlock;
>  			}
> @@ -287,6 +305,7 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size) ctx->more = msg->msg_flags & MSG_MORE;
>  	if (!ctx->more && !aead_sufficient_data(ctx)) {
>  		aead_put_sgl(sk);
> +		aead_reset_ctx(ctx);
>  		err = -EMSGSIZE;
>  	}
> 
> @@ -322,6 +341,7 @@ static ssize_t aead_sendpage(struct socket *sock, struct
> page *page, if (!aead_writable(sk)) {
>  		/* user space sent too much data */
>  		aead_put_sgl(sk);
> +		aead_reset_ctx(ctx);
>  		err = -EMSGSIZE;
>  		goto unlock;
>  	}
> @@ -339,6 +359,7 @@ done:
>  	ctx->more = flags & MSG_MORE;
>  	if (!ctx->more && !aead_sufficient_data(ctx)) {
>  		aead_put_sgl(sk);
> +		aead_reset_ctx(ctx);
>  		err = -EMSGSIZE;
>  	}
> 
> @@ -349,7 +370,144 @@ unlock:
>  	return err ?: size;
>  }
> 
> -static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t
> ignored, int flags) +#define GET_ASYM_REQ(req, tfm) (struct aead_async_req
> *) \
> +		((char *)req + crypto_aead_reqsize(tfm))
> +
> +static void aead_async_cb(struct crypto_async_request *req, int err)
> +{
> +	struct sock *sk = req->data;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct aead_ctx *ctx = ask->private;
> +	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
> +	struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
> +	struct scatterlist *sg = areq->tsgl;
> +	struct kiocb *iocb = areq->iocb;
> +	int i;
> +
> +	atomic_dec(&ctx->inflight);
> +	for (i = 0; i < areq->tsgls; i++)
> +		put_page(sg_page(sg + i));
> +
> +	for (i = 0; i < areq->rsgls; i++)
> +		af_alg_free_sg(&areq->rsgl[i]);
> +
> +	kfree(req);
> +	iocb->ki_complete(iocb, err, err);
> +}
> +
> +static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> +			      int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct aead_ctx *ctx = ask->private;
> +	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
> +	struct aead_async_req *areq;
> +	struct aead_request *req = NULL;
> +	struct aead_sg_list *sgl = &ctx->tsgl;
> +	unsigned int as = crypto_aead_authsize(tfm);
> +	unsigned int reqlen = sizeof(*areq) + crypto_aead_reqsize(tfm) +
> +				crypto_aead_ivsize(tfm);
> +	int err = -ENOMEM, i;
> +	unsigned long used;
> +	size_t outlen;
> +	size_t usedpages = 0;
> +	unsigned int cnt = 0;
> +
> +	/* Limit number of IOV blocks to be accessed below */
> +	if (msg->msg_iter.nr_segs > RSGL_MAX_ENTRIES)
> +		return -ENOMSG;
> +
> +	lock_sock(sk);
> +
> +	if (ctx->more) {
> +		err = aead_wait_for_data(sk, flags);
> +		if (err)
> +			goto unlock;
> +	}
> +
> +	used = ctx->used;
> +	outlen = used;
> +
> +	if (!aead_sufficient_data(ctx))
> +		goto unlock;
> +
> +	req = kmalloc(reqlen, GFP_KERNEL);

Shouldn't that be sock_kmalloc? If yes, we also need to use sock_kfree_s 
above.

> +	if (unlikely(!req))
> +		goto unlock;
> +
> +	areq = GET_ASYM_REQ(req, tfm);
> +	areq->iocb = msg->msg_iocb;
> +	memcpy(areq->iv, ctx->iv, crypto_aead_ivsize(tfm));
> +	aead_request_set_tfm(req, tfm);
> +	aead_request_set_ad(req, ctx->aead_assoclen);
> +	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +				  aead_async_cb, sk);
> +	used -= ctx->aead_assoclen + (ctx->enc ? as : 0);
> +
> +	/* take over all tx sgls from ctx */
> +	for (i = 0; i < sgl->cur; i++)
> +		areq->tsgl[i] = sgl->sg[i];
> +
> +	sg_mark_end(areq->tsgl + sgl->cur - 1);
> +	areq->tsgls = sgl->cur;
> +
> +	/* create rx sgls */
> +	while (iov_iter_count(&msg->msg_iter)) {
> +		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
> +				      (outlen - usedpages));
> +
> +		/* make one iovec available as scatterlist */
> +		err = af_alg_make_sg(&areq->rsgl[cnt], &msg->msg_iter, 
seglen);
> +		if (err < 0)
> +			goto free;
> +		usedpages += err;
> +		/* chain the new scatterlist with previous one */
> +		if (cnt)
> +			af_alg_link_sg(&areq->rsgl[cnt - 1], &areq-
>rsgl[cnt]);
> +
> +		/* we do not need more iovecs as we have sufficient memory */
> +		if (outlen <= usedpages)
> +			break;
> +		iov_iter_advance(&msg->msg_iter, err);
> +		cnt++;
> +	}
> +	areq->rsgls = cnt;
> +	err = -EINVAL;
> +	/* ensure output buffer is sufficiently large */
> +	if (usedpages < outlen)
> +		goto free;
> +
> +	aead_request_set_crypt(req, areq->tsgl, areq->rsgl[0].sg, used,
> +			       areq->iv);
> +	err = ctx->enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
> +	if (err) {
> +		if (err == -EINPROGRESS) {
> +			atomic_inc(&ctx->inflight);
> +			err = -EIOCBQUEUED;
> +			aead_reset_ctx(ctx);
> +			goto unlock;
> +		} else if (err == -EBADMSG) {
> +			aead_put_sgl(sk);
> +			aead_reset_ctx(ctx);
> +			goto free;
> +		}
> +		goto free;
> +	}
> +	aead_put_sgl(sk);
> +	aead_reset_ctx(ctx);
> +free:
> +	for (i = 0; i < cnt; i++)
> +		af_alg_free_sg(&areq->rsgl[i]);
> +
> +	kfree(req);
> +unlock:
> +	aead_wmem_wakeup(sk);
> +	release_sock(sk);
> +	return err ? err : outlen;
> +}
> +
> +static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int
> flags) {
>  	struct sock *sk = sock->sk;
>  	struct alg_sock *ask = alg_sk(sk);
> @@ -452,12 +610,15 @@ static int aead_recvmsg(struct socket *sock, struct
> msghdr *msg, size_t ignored,
> 
>  	if (err) {
>  		/* EBADMSG implies a valid cipher operation took place */
> -		if (err == -EBADMSG)
> +		if (err == -EBADMSG) {
>  			aead_put_sgl(sk);
> +			aead_reset_ctx(ctx);
> +		}
>  		goto unlock;
>  	}
> 
>  	aead_put_sgl(sk);
> +	aead_reset_ctx(ctx);
> 
>  	err = 0;
> 
> @@ -471,6 +632,14 @@ unlock:
>  	return err ? err : outlen;
>  }
> 
> +static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t
> ignored, +			int flags)
> +{
> +	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
> +		aead_recvmsg_async(sock, msg, flags) :
> +		aead_recvmsg_sync(sock, msg, flags);
> +}

The code in the aead_recvmsg_sync and _async is very very similar with the 
exception of the areq handling.

What I am wondering, is it possible to consolidate both into one, considering 
that the real difference according to my understanding is the 
af_alg_wait_for_completion usage (in _sync) vs. the use of a self-written 
callback (_async)?

> +
>  static unsigned int aead_poll(struct file *file, struct socket *sock,
>  			      poll_table *wait)
>  {
> @@ -533,6 +702,16 @@ static int aead_setkey(void *private, const u8 *key,
> unsigned int keylen) return crypto_aead_setkey(private, key, keylen);
>  }
> 
> +static void aead_wait(struct sock *sk)
> +{
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct aead_ctx *ctx = ask->private;
> +	int ctr = 0;
> +
> +	while (atomic_read(&ctx->inflight) && ctr++ < 100)
> +		msleep(100);

I know that same logic is applied to algif_skcipher. But isn't that a kind of 
uninterruptible sleep? Do you see the potential that a process cannot 
terminate the socket? For example, if a process makes the error of sending 
asynchronously data to the kernel, but "forgets" all necessary recvmsg calls 
and we do not decrease the inflight any more. In this case, wouldn't a kill -9 
of that hanging process leave a zombie or not work at all?

> +}
> +
>  static void aead_sock_destruct(struct sock *sk)
>  {
>  	struct alg_sock *ask = alg_sk(sk);
> @@ -540,6 +719,9 @@ static void aead_sock_destruct(struct sock *sk)
>  	unsigned int ivlen = crypto_aead_ivsize(
>  				crypto_aead_reqtfm(&ctx->aead_req));
> 
> +	if (atomic_read(&ctx->inflight))
> +		aead_wait(sk);
> +
>  	aead_put_sgl(sk);
>  	sock_kzfree_s(sk, ctx->iv, ivlen);
>  	sock_kfree_s(sk, ctx, ctx->len);


-- 
Ciao
Stephan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-17 15:07 ` Stephan Mueller
@ 2016-01-18 15:22   ` Tadeusz Struk
  2016-01-19  0:34     ` Herbert Xu
  0 siblings, 1 reply; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-18 15:22 UTC (permalink / raw)
  To: Stephan Mueller, Tadeusz Struk; +Cc: herbert, linux-kernel, linux-crypto

Hi Stephan,

On 01/17/2016 07:07 AM, Stephan Mueller wrote:
>> +static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
>> > +			      int flags)
>> > +{
>> > +	struct sock *sk = sock->sk;
>> > +	struct alg_sock *ask = alg_sk(sk);
>> > +	struct aead_ctx *ctx = ask->private;
>> > +	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
>> > +	struct aead_async_req *areq;
>> > +	struct aead_request *req = NULL;
>> > +	struct aead_sg_list *sgl = &ctx->tsgl;
>> > +	unsigned int as = crypto_aead_authsize(tfm);
>> > +	unsigned int reqlen = sizeof(*areq) + crypto_aead_reqsize(tfm) +
>> > +				crypto_aead_ivsize(tfm);
>> > +	int err = -ENOMEM, i;
>> > +	unsigned long used;
>> > +	size_t outlen;
>> > +	size_t usedpages = 0;
>> > +	unsigned int cnt = 0;
>> > +
>> > +	/* Limit number of IOV blocks to be accessed below */
>> > +	if (msg->msg_iter.nr_segs > RSGL_MAX_ENTRIES)
>> > +		return -ENOMSG;
>> > +
>> > +	lock_sock(sk);
>> > +
>> > +	if (ctx->more) {
>> > +		err = aead_wait_for_data(sk, flags);
>> > +		if (err)
>> > +			goto unlock;
>> > +	}
>> > +
>> > +	used = ctx->used;
>> > +	outlen = used;
>> > +
>> > +	if (!aead_sufficient_data(ctx))
>> > +		goto unlock;
>> > +
>> > +	req = kmalloc(reqlen, GFP_KERNEL);
> Shouldn't that be sock_kmalloc? If yes, we also need to use sock_kfree_s 
> above.

My understanding is that the sock_kmalloc is mainly used for allocations
of the user provided  data, because it keeps tracks of how much memory
is allocated by a socket, and makes sure that is will not exceed the
sysctl_optmem_max limit. Usually the internal structures, with fixed
size are allocated simply with kmalloc. I don't think that using
sock_kmalloc will give us any benefit here.

> 
>> +static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t
>> > ignored, +			int flags)
>> > +{
>> > +	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
>> > +		aead_recvmsg_async(sock, msg, flags) :
>> > +		aead_recvmsg_sync(sock, msg, flags);
>> > +}
> The code in the aead_recvmsg_sync and _async is very very similar with the 
> exception of the areq handling.
> 
> What I am wondering, is it possible to consolidate both into one, considering 
> that the real difference according to my understanding is the 
> af_alg_wait_for_completion usage (in _sync) vs. the use of a self-written 
> callback (_async)?
> 

I agree that they are very similar, but I found it much easier to debug
when they are separate functions. I would prefer to keep them separate.
They are also separate in algif_skcipher. It makes it also easier to
read and understand.

>> +static void aead_wait(struct sock *sk)
>> > +{
>> > +	struct alg_sock *ask = alg_sk(sk);
>> > +	struct aead_ctx *ctx = ask->private;
>> > +	int ctr = 0;
>> > +
>> > +	while (atomic_read(&ctx->inflight) && ctr++ < 100)
>> > +		msleep(100);
> I know that same logic is applied to algif_skcipher. But isn't that a kind of 
> uninterruptible sleep? Do you see the potential that a process cannot 
> terminate the socket? For example, if a process makes the error of sending 
> asynchronously data to the kernel, but "forgets" all necessary recvmsg calls 
> and we do not decrease the inflight any more. In this case, wouldn't a kill -9 
> of that hanging process leave a zombie or not work at all?
> 

The inflight ctr is incremented only if an asynchronous request has been
successfully en-queued for processing. If a user forges to call recvmsg
then the function that increments it won't be even called.
>From the other hand we don't want to give the option to interrupt the
wait, because in a case, when we do have request being processed by some
hardware, and the user kills the process, causing the socket to be
freed, then we will get an Oops in the callback.
Thanks,

--
TS

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-18 15:22   ` Tadeusz Struk
@ 2016-01-19  0:34     ` Herbert Xu
  2016-01-19 15:18       ` Tadeusz Struk
  2016-01-20 20:18       ` Tadeusz Struk
  0 siblings, 2 replies; 14+ messages in thread
From: Herbert Xu @ 2016-01-19  0:34 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Stephan Mueller, Tadeusz Struk, linux-kernel, linux-crypto

On Mon, Jan 18, 2016 at 07:22:55AM -0800, Tadeusz Struk wrote:
>
> My understanding is that the sock_kmalloc is mainly used for allocations
> of the user provided  data, because it keeps tracks of how much memory
> is allocated by a socket, and makes sure that is will not exceed the
> sysctl_optmem_max limit. Usually the internal structures, with fixed
> size are allocated simply with kmalloc. I don't think that using
> sock_kmalloc will give us any benefit here.

If there is only ever one of them per-socket then kmalloc is fine,
otherwise you should use sock_kmalloc.

> > The code in the aead_recvmsg_sync and _async is very very similar with the 
> > exception of the areq handling.
> > 
> > What I am wondering, is it possible to consolidate both into one, considering 
> > that the real difference according to my understanding is the 
> > af_alg_wait_for_completion usage (in _sync) vs. the use of a self-written 
> > callback (_async)?
> 
> I agree that they are very similar, but I found it much easier to debug
> when they are separate functions. I would prefer to keep them separate.
> They are also separate in algif_skcipher. It makes it also easier to
> read and understand.

I too would prefer a common function.  However we can do this
later if we wish.
 
> The inflight ctr is incremented only if an asynchronous request has been
> successfully en-queued for processing. If a user forges to call recvmsg
> then the function that increments it won't be even called.
> >From the other hand we don't want to give the option to interrupt the
> wait, because in a case, when we do have request being processed by some
> hardware, and the user kills the process, causing the socket to be
> freed, then we will get an Oops in the callback.

This should be replaced with a sock_hold.

Cheers,
-- 
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] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-19  0:34     ` Herbert Xu
@ 2016-01-19 15:18       ` Tadeusz Struk
  2016-01-20 20:18       ` Tadeusz Struk
  1 sibling, 0 replies; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-19 15:18 UTC (permalink / raw)
  To: Herbert Xu, Tadeusz Struk; +Cc: Stephan Mueller, linux-kernel, linux-crypto

On 01/18/2016 04:34 PM, Herbert Xu wrote:
>> My understanding is that the sock_kmalloc is mainly used for allocations
>> > of the user provided  data, because it keeps tracks of how much memory
>> > is allocated by a socket, and makes sure that is will not exceed the
>> > sysctl_optmem_max limit. Usually the internal structures, with fixed
>> > size are allocated simply with kmalloc. I don't think that using
>> > sock_kmalloc will give us any benefit here.
> If there is only ever one of them per-socket then kmalloc is fine,
> otherwise you should use sock_kmalloc.

There is one per request. There can be a few of them at a given time.
We have the same thing in skcipher and we use kmalloc there. 

> 
>> I agree that they are very similar, but I found it much easier to debug
>> > when they are separate functions. I would prefer to keep them separate.
>> > They are also separate in algif_skcipher. It makes it also easier to
>> > read and understand.
> I too would prefer a common function.  However we can do this
> later if we wish.
>  

lets do this later then.

> 
>> > The inflight ctr is incremented only if an asynchronous request has been
>> > successfully en-queued for processing. If a user forges to call recvmsg
>> > then the function that increments it won't be even called.
>> > >From the other hand we don't want to give the option to interrupt the
>> > wait, because in a case, when we do have request being processed by some
>> > hardware, and the user kills the process, causing the socket to be
>> > freed, then we will get an Oops in the callback.
> This should be replaced with a sock_hold.

Ok, I will try sock_hold.
Thanks,

-- 
TS

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-19  0:34     ` Herbert Xu
  2016-01-19 15:18       ` Tadeusz Struk
@ 2016-01-20 20:18       ` Tadeusz Struk
  2016-01-21  5:00         ` Herbert Xu
  1 sibling, 1 reply; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-20 20:18 UTC (permalink / raw)
  To: Herbert Xu, Tadeusz Struk; +Cc: Stephan Mueller, linux-kernel, linux-crypto

Hi Herbert,
On 01/18/2016 04:34 PM, Herbert Xu wrote:
>> My understanding is that the sock_kmalloc is mainly used for allocations
>> > of the user provided  data, because it keeps tracks of how much memory
>> > is allocated by a socket, and makes sure that is will not exceed the
>> > sysctl_optmem_max limit. Usually the internal structures, with fixed
>> > size are allocated simply with kmalloc. I don't think that using
>> > sock_kmalloc will give us any benefit here.
> If there is only ever one of them per-socket then kmalloc is fine,
> otherwise you should use sock_kmalloc.
> 

I tried sock_kmalloc and it will not work. The sysctl_optmem_max by
default is 20480 bytes. The aead ctx by itself takes more than half of
it (11832 bytes). A single async request takes 11408 bytes.
It means we need to use kmalloc or no async request could be allocated.
I would opt to go with this version and I'll convert both algif_aead
and algif_skcipher to use sock_hold later.
Thanks,

-- 
TS

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-20 20:18       ` Tadeusz Struk
@ 2016-01-21  5:00         ` Herbert Xu
  0 siblings, 0 replies; 14+ messages in thread
From: Herbert Xu @ 2016-01-21  5:00 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: Tadeusz Struk, Stephan Mueller, linux-kernel, linux-crypto

On Wed, Jan 20, 2016 at 12:18:24PM -0800, Tadeusz Struk wrote:
>
> I tried sock_kmalloc and it will not work. The sysctl_optmem_max by
> default is 20480 bytes. The aead ctx by itself takes more than half of
> it (11832 bytes). A single async request takes 11408 bytes.
> It means we need to use kmalloc or no async request could be allocated.
> I would opt to go with this version and I'll convert both algif_aead
> and algif_skcipher to use sock_hold later.

Then we have to make it smaller.  The whole point of this is to
stop the user from allocating too much kernel memory, and if you
are allocating too much memory to start with...

So instead of allocating RSGL_MAX_ENTRIES you should turn it into
a linked list like algif_skcipher and then allocate it on demand
using sock_kmalloc.

Cheers,
-- 
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] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-28 17:09     ` Stephan Mueller
@ 2016-01-28 17:30       ` Tadeusz Struk
  0 siblings, 0 replies; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-28 17:30 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: herbert, linux-crypto, linux-kernel

On 01/28/2016 09:09 AM, Stephan Mueller wrote:
> Am Donnerstag, 28. Januar 2016, 08:00:25 schrieb Tadeusz Struk:
> 
> Hi Tadeusz,
> 
>> Hi Stephan,
>>
>> On 01/27/2016 10:26 PM, Stephan Mueller wrote:
>>>> +	for (i = 0; i < areq->tsgls; i++)
>>>>
>>>>> +		put_page(sg_page(sg + i));
>>>
>>> Shouldn't here be the same logic as in put_sgl? I.e.
>>>
>>>         for (i = 0; i < sgl->cur; i++) {
>>>         
>>>                 if (!sg_page(sg + i))
>>>                 
>>>                         continue;
>>>                 
>>>                 put_page(sg_page(sg + i));
>>>                 sg_assign_page(sg + i, NULL);
>>>         
>>>         }
>>
>> Thanks for reviewing.
>> I don't think it is possible that there ever will be any gaps in the tsgl.
>> In fact if there is such a possibility then it is a serious problem, because
>> it would mean that we are sending NULL ptrs to the ciphers (see line 640):
>>
>> 	sg_mark_end(sgl->sg + sgl->cur - 1);
>> 	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx-
>> first_rsgl.sgl.sg,
>> 			       used, ctx->iv);
>>
>> I don't see any implementation checking for null in sgls. Most of them just
>> do:
>>
>> 	for_each_sg(sgl, sg, nents, i)
>> 		sg_virt(sg)...
>>
>> So it would Oops there. I think this check in put_sgl is redundant.
>> Thanks,
> 
> algif_skcipher does a similar check in skcipher_pull_sgl:
> 
> ...
>                         if (!sg_page(sg + i))
>                                 continue;
> ...
>                         if (put)
>                                 put_page(sg_page(sg + i));
>                         sg_assign_page(sg + i, NULL);
> ...
> 

Yes, that's true, but this is because if you look at the skcipher_recvmsg_async()
function, it invokes crypt operation for each recv segment separately, and after each
iteration advances the tsgl forward and marks the sgs that are already processed with NULL.
This is completely different to the aead use case, which always sends everything
in one go.
Thanks,

-- 
TS

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-28 16:00   ` Tadeusz Struk
@ 2016-01-28 17:09     ` Stephan Mueller
  2016-01-28 17:30       ` Tadeusz Struk
  0 siblings, 1 reply; 14+ messages in thread
From: Stephan Mueller @ 2016-01-28 17:09 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: herbert, linux-crypto, linux-kernel

Am Donnerstag, 28. Januar 2016, 08:00:25 schrieb Tadeusz Struk:

Hi Tadeusz,

>Hi Stephan,
>
>On 01/27/2016 10:26 PM, Stephan Mueller wrote:
>>> +	for (i = 0; i < areq->tsgls; i++)
>>> 
>>> > +		put_page(sg_page(sg + i));
>> 
>> Shouldn't here be the same logic as in put_sgl? I.e.
>> 
>>         for (i = 0; i < sgl->cur; i++) {
>>         
>>                 if (!sg_page(sg + i))
>>                 
>>                         continue;
>>                 
>>                 put_page(sg_page(sg + i));
>>                 sg_assign_page(sg + i, NULL);
>>         
>>         }
>
>Thanks for reviewing.
>I don't think it is possible that there ever will be any gaps in the tsgl.
>In fact if there is such a possibility then it is a serious problem, because
>it would mean that we are sending NULL ptrs to the ciphers (see line 640):
>
>	sg_mark_end(sgl->sg + sgl->cur - 1);
>	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx-
>first_rsgl.sgl.sg,
>			       used, ctx->iv);
>
>I don't see any implementation checking for null in sgls. Most of them just
>do:
>
>	for_each_sg(sgl, sg, nents, i)
>		sg_virt(sg)...
>
>So it would Oops there. I think this check in put_sgl is redundant.
>Thanks,

algif_skcipher does a similar check in skcipher_pull_sgl:

...
                        if (!sg_page(sg + i))
                                continue;
...
                        if (put)
                                put_page(sg_page(sg + i));
                        sg_assign_page(sg + i, NULL);
...


Ciao
Stephan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-28  6:26 ` Stephan Mueller
@ 2016-01-28 16:00   ` Tadeusz Struk
  2016-01-28 17:09     ` Stephan Mueller
  0 siblings, 1 reply; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-28 16:00 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: herbert, linux-crypto, linux-kernel

Hi Stephan,
On 01/27/2016 10:26 PM, Stephan Mueller wrote:
>> +	for (i = 0; i < areq->tsgls; i++)
>> > +		put_page(sg_page(sg + i));
> Shouldn't here be the same logic as in put_sgl? I.e.
> 
>         for (i = 0; i < sgl->cur; i++) {
>                 if (!sg_page(sg + i))
>                         continue;
> 
>                 put_page(sg_page(sg + i));
>                 sg_assign_page(sg + i, NULL);
>         }
> 

Thanks for reviewing.
I don't think it is possible that there ever will be any gaps in the tsgl.
In fact if there is such a possibility then it is a serious problem, because
it would mean that we are sending NULL ptrs to the ciphers (see line 640):

	sg_mark_end(sgl->sg + sgl->cur - 1);
	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx->first_rsgl.sgl.sg,
			       used, ctx->iv);

I don't see any implementation checking for null in sgls. Most of them just do:

	for_each_sg(sgl, sg, nents, i)
		sg_virt(sg)...

So it would Oops there. I think this check in put_sgl is redundant.
Thanks,
-- 
TS

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-27 22:10 Tadeusz Struk
  2016-01-27 22:29 ` kbuild test robot
@ 2016-01-28  6:26 ` Stephan Mueller
  2016-01-28 16:00   ` Tadeusz Struk
  1 sibling, 1 reply; 14+ messages in thread
From: Stephan Mueller @ 2016-01-28  6:26 UTC (permalink / raw)
  To: Tadeusz Struk; +Cc: herbert, linux-crypto, linux-kernel

Am Mittwoch, 27. Januar 2016, 14:10:31 schrieb Tadeusz Struk:

Hi Tadeusz,

> Following the async change for algif_skcipher
> this patch adds similar async read to algif_aead.
> 
> changes in v2:
> - change internal data structures from fixed size arrays, limited to
>   RSGL_MAX_ENTRIES, to linked list model with no artificial limitation.

Thank you for the dynamic allocation support, but I do have a question.

> - use sock_kmalloc instead of kmalloc for memory allocation
> - use sock_hold instead of separate atomic ctr to wait for outstanding
> request
> 
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
>  crypto/algif_aead.c |  278
> +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 248
> insertions(+), 30 deletions(-)
> 
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index 147069c..3fa1a95 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -29,15 +29,24 @@ struct aead_sg_list {
>  	struct scatterlist sg[ALG_MAX_PAGES];
>  };
> 
> +struct aead_async_rsgl {
> +	struct af_alg_sgl sgl;
> +	struct list_head list;
> +};
> +
> +struct aead_async_req {
> +	struct scatterlist *tsgl;
> +	struct aead_async_rsgl first_rsgl;
> +	struct list_head list;
> +	struct kiocb *iocb;
> +	unsigned int tsgls;
> +	char iv[];
> +};
> +
>  struct aead_ctx {
>  	struct aead_sg_list tsgl;
> -	/*
> -	 * RSGL_MAX_ENTRIES is an artificial limit where user space at maximum
> -	 * can cause the kernel to allocate RSGL_MAX_ENTRIES * ALG_MAX_PAGES
> -	 * pages
> -	 */
> -#define RSGL_MAX_ENTRIES ALG_MAX_PAGES
> -	struct af_alg_sgl rsgl[RSGL_MAX_ENTRIES];
> +	struct aead_async_rsgl first_rsgl;
> +	struct list_head list;
> 
>  	void *iv;
> 
> @@ -75,6 +84,17 @@ static inline bool aead_sufficient_data(struct aead_ctx
> *ctx) return ctx->used >= ctx->aead_assoclen + as;
>  }
> 
> +static void aead_reset_ctx(struct aead_ctx *ctx)
> +{
> +	struct aead_sg_list *sgl = &ctx->tsgl;
> +
> +	sg_init_table(sgl->sg, ALG_MAX_PAGES);
> +	sgl->cur = 0;
> +	ctx->used = 0;
> +	ctx->more = 0;
> +	ctx->merge = 0;
> +}
> +
>  static void aead_put_sgl(struct sock *sk)
>  {
>  	struct alg_sock *ask = alg_sk(sk);
> @@ -90,11 +110,6 @@ static void aead_put_sgl(struct sock *sk)
>  		put_page(sg_page(sg + i));
>  		sg_assign_page(sg + i, NULL);
>  	}
> -	sg_init_table(sg, ALG_MAX_PAGES);
> -	sgl->cur = 0;
> -	ctx->used = 0;
> -	ctx->more = 0;
> -	ctx->merge = 0;
>  }
> 
>  static void aead_wmem_wakeup(struct sock *sk)
> @@ -240,6 +255,7 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size) if (!aead_writable(sk)) {
>  			/* user space sent too much data */
>  			aead_put_sgl(sk);
> +			aead_reset_ctx(ctx);
>  			err = -EMSGSIZE;
>  			goto unlock;
>  		}
> @@ -251,6 +267,7 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size)
> 
>  			if (sgl->cur >= ALG_MAX_PAGES) {
>  				aead_put_sgl(sk);
> +				aead_reset_ctx(ctx);
>  				err = -E2BIG;
>  				goto unlock;
>  			}
> @@ -287,6 +304,7 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size) ctx->more = msg->msg_flags & MSG_MORE;
>  	if (!ctx->more && !aead_sufficient_data(ctx)) {
>  		aead_put_sgl(sk);
> +		aead_reset_ctx(ctx);
>  		err = -EMSGSIZE;
>  	}
> 
> @@ -322,6 +340,7 @@ static ssize_t aead_sendpage(struct socket *sock, struct
> page *page, if (!aead_writable(sk)) {
>  		/* user space sent too much data */
>  		aead_put_sgl(sk);
> +		aead_reset_ctx(ctx);
>  		err = -EMSGSIZE;
>  		goto unlock;
>  	}
> @@ -339,6 +358,7 @@ done:
>  	ctx->more = flags & MSG_MORE;
>  	if (!ctx->more && !aead_sufficient_data(ctx)) {
>  		aead_put_sgl(sk);
> +		aead_reset_ctx(ctx);
>  		err = -EMSGSIZE;
>  	}
> 
> @@ -349,23 +369,189 @@ unlock:
>  	return err ?: size;
>  }
> 
> -static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t
> ignored, int flags) +#define GET_ASYM_REQ(req, tfm) (struct aead_async_req
> *) \
> +		((char *)req + sizeof(struct aead_request) + \
> +		 crypto_aead_reqsize(tfm))
> +
> + #define GET_REQ_SIZE(tfm) sizeof(struct aead_async_req) + \
> +	crypto_aead_reqsize(tfm) + crypto_aead_ivsize(tfm) + \
> +	sizeof(struct aead_request)
> +
> +static void aead_async_cb(struct crypto_async_request *_req, int err)
> +{
> +	struct sock *sk = _req->data;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct aead_ctx *ctx = ask->private;
> +	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
> +	struct aead_request *req = aead_request_cast(_req);
> +	struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
> +	struct scatterlist *sg = areq->tsgl;
> +	struct aead_async_rsgl *rsgl;
> +	struct kiocb *iocb = areq->iocb;
> +	unsigned int i, reqlen = GET_REQ_SIZE(tfm);
> +
> +	list_for_each_entry(rsgl, &areq->list, list) {
> +		af_alg_free_sg(&rsgl->sgl);
> +		if (rsgl != &areq->first_rsgl)
> +			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
> +	}
> +
> +	for (i = 0; i < areq->tsgls; i++)
> +		put_page(sg_page(sg + i));

Shouldn't here be the same logic as in put_sgl? I.e.

        for (i = 0; i < sgl->cur; i++) {
                if (!sg_page(sg + i))
                        continue;

                put_page(sg_page(sg + i));
                sg_assign_page(sg + i, NULL);
        }

> +
> +	sock_kfree_s(sk, areq->tsgl, sizeof(*areq->tsgl) * areq->tsgls);
> +	sock_kfree_s(sk, req, reqlen);
> +	__sock_put(sk);
> +	iocb->ki_complete(iocb, err, err);
> +}
> +
> +static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> +			      int flags)
> +{
> +	struct sock *sk = sock->sk;
> +	struct alg_sock *ask = alg_sk(sk);
> +	struct aead_ctx *ctx = ask->private;
> +	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
> +	struct aead_async_req *areq;
> +	struct aead_request *req = NULL;
> +	struct aead_sg_list *sgl = &ctx->tsgl;
> +	struct aead_async_rsgl *last_rsgl = NULL, *rsgl;
> +	unsigned int as = crypto_aead_authsize(tfm);
> +	unsigned int i, reqlen = GET_REQ_SIZE(tfm);
> +	int err = -ENOMEM;
> +	unsigned long used;
> +	size_t outlen;
> +	size_t usedpages = 0;
> +
> +	lock_sock(sk);
> +	if (ctx->more) {
> +		err = aead_wait_for_data(sk, flags);
> +		if (err)
> +			goto unlock;
> +	}
> +
> +	used = ctx->used;
> +	outlen = used;
> +
> +	if (!aead_sufficient_data(ctx))
> +		goto unlock;
> +
> +	req = sock_kmalloc(sk, reqlen, GFP_KERNEL);
> +	if (unlikely(!req))
> +		goto unlock;
> +
> +	areq = GET_ASYM_REQ(req, tfm);
> +	memset(&areq->first_rsgl, '\0', sizeof(areq->first_rsgl));
> +	INIT_LIST_HEAD(&areq->list);
> +	areq->iocb = msg->msg_iocb;
> +	memcpy(areq->iv, ctx->iv, crypto_aead_ivsize(tfm));
> +	aead_request_set_tfm(req, tfm);
> +	aead_request_set_ad(req, ctx->aead_assoclen);
> +	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> +				  aead_async_cb, sk);
> +	used -= ctx->aead_assoclen + (ctx->enc ? as : 0);
> +
> +	/* take over all tx sgls from ctx */
> +	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * sgl->cur,
> +				  GFP_KERNEL);
> +	if (unlikely(!areq->tsgl))
> +		goto free;
> +
> +	sg_init_table(areq->tsgl, sgl->cur);
> +	for (i = 0; i < sgl->cur; i++)
> +		sg_set_page(&areq->tsgl[i], sg_page(&sgl->sg[i]),
> +			    sgl->sg[i].length, sgl->sg[i].offset);
> +
> +	areq->tsgls = sgl->cur;
> +
> +	/* create rx sgls */
> +	while (iov_iter_count(&msg->msg_iter)) {
> +		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
> +				      (outlen - usedpages));
> +
> +		if (list_empty(&areq->list))
> +			rsgl = &areq->first_rsgl;
> +		else {
> +			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
> +			if (unlikely(!rsgl)) {
> +				err = -ENOMEM;
> +				goto free;
> +			}
> +		}
> +		rsgl->sgl.npages = 0;
> +		list_add_tail(&rsgl->list, &areq->list);
> +
> +		/* make one iovec available as scatterlist */
> +		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
> +		if (err < 0)
> +			goto free;
> +
> +		usedpages += err;
> +
> +		/* chain the new scatterlist with previous one */
> +		if (last_rsgl)
> +			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
> +
> +		last_rsgl = rsgl;
> +
> +		/* we do not need more iovecs as we have sufficient memory */
> +		if (outlen <= usedpages)
> +			break;
> +
> +		iov_iter_advance(&msg->msg_iter, err);
> +	}
> +	err = -EINVAL;
> +	/* ensure output buffer is sufficiently large */
> +	if (usedpages < outlen)
> +		goto free;
> +
> +	aead_request_set_crypt(req, areq->tsgl, areq->first_rsgl.sgl.sg, used,
> +			       areq->iv);
> +	err = ctx->enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
> +	if (err) {
> +		if (err == -EINPROGRESS) {
> +			sock_hold(sk);
> +			err = -EIOCBQUEUED;
> +			aead_reset_ctx(ctx);
> +			goto unlock;
> +		} else if (err == -EBADMSG) {
> +			aead_put_sgl(sk);
> +			aead_reset_ctx(ctx);
> +		}
> +		goto free;
> +	}
> +	aead_put_sgl(sk);
> +	aead_reset_ctx(ctx);
> +
> +free:
> +	list_for_each_entry(rsgl, &areq->list, list) {
> +		af_alg_free_sg(&rsgl->sgl);
> +		if (rsgl != &areq->first_rsgl)
> +			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
> +	}
> +	if (areq->tsgl)
> +		sock_kfree_s(sk, areq->tsgl, sizeof(*areq->tsgl) * areq-
>tsgls);
> +	if (req)
> +		sock_kfree_s(sk, req, reqlen);
> +unlock:
> +	aead_wmem_wakeup(sk);
> +	release_sock(sk);
> +	return err ? err : outlen;
> +}
> +
> +static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int
> flags) {
>  	struct sock *sk = sock->sk;
>  	struct alg_sock *ask = alg_sk(sk);
>  	struct aead_ctx *ctx = ask->private;
>  	unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx-
>aead_req));
>  	struct aead_sg_list *sgl = &ctx->tsgl;
> -	unsigned int i = 0;
> +	struct aead_async_rsgl *last_rsgl = NULL;
> +	struct aead_async_rsgl *rsgl, *tmp;
>  	int err = -EINVAL;
>  	unsigned long used = 0;
>  	size_t outlen = 0;
>  	size_t usedpages = 0;
> -	unsigned int cnt = 0;
> -
> -	/* Limit number of IOV blocks to be accessed below */
> -	if (msg->msg_iter.nr_segs > RSGL_MAX_ENTRIES)
> -		return -ENOMSG;
> 
>  	lock_sock(sk);
> 
> @@ -417,21 +603,33 @@ static int aead_recvmsg(struct socket *sock, struct
> msghdr *msg, size_t ignored, size_t seglen = min_t(size_t,
> iov_iter_count(&msg->msg_iter),
>  				      (outlen - usedpages));
> 
> +		if (list_empty(&ctx->list))
> +			rsgl = &ctx->first_rsgl;
> +		else {
> +			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
> +			if (unlikely(!rsgl)) {
> +				err = -ENOMEM;
> +				goto unlock;
> +			}
> +		}
> +		rsgl->sgl.npages = 0;
> +		list_add_tail(&rsgl->list, &ctx->list);
> +
>  		/* make one iovec available as scatterlist */
> -		err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
> -				     seglen);
> +		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
>  		if (err < 0)
>  			goto unlock;
>  		usedpages += err;
>  		/* chain the new scatterlist with previous one */
> -		if (cnt)
> -			af_alg_link_sg(&ctx->rsgl[cnt-1], &ctx->rsgl[cnt]);
> +		if (last_rsgl)
> +			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
> +
> +		last_rsgl = rsgl;
> 
>  		/* we do not need more iovecs as we have sufficient memory */
>  		if (outlen <= usedpages)
>  			break;
>  		iov_iter_advance(&msg->msg_iter, err);
> -		cnt++;
>  	}
> 
>  	err = -EINVAL;
> @@ -440,8 +638,7 @@ static int aead_recvmsg(struct socket *sock, struct
> msghdr *msg, size_t ignored, goto unlock;
> 
>  	sg_mark_end(sgl->sg + sgl->cur - 1);
> -
> -	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx->rsgl[0].sg,
> +	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx-
>first_rsgl.sgl.sg,
>  			       used, ctx->iv);
>  	aead_request_set_ad(&ctx->aead_req, ctx->aead_assoclen);
> 
> @@ -452,25 +649,40 @@ static int aead_recvmsg(struct socket *sock, struct
> msghdr *msg, size_t ignored,
> 
>  	if (err) {
>  		/* EBADMSG implies a valid cipher operation took place */
> -		if (err == -EBADMSG)
> +		if (err == -EBADMSG) {
>  			aead_put_sgl(sk);
> +			aead_reset_ctx(ctx);
> +		}
>  		goto unlock;
>  	}
> 
>  	aead_put_sgl(sk);
> +	aead_reset_ctx(ctx);
> 
>  	err = 0;
> 
>  unlock:
> -	for (i = 0; i < cnt; i++)
> -		af_alg_free_sg(&ctx->rsgl[i]);
> -
> +	list_for_each_entry_safe(rsgl, tmp, &ctx->list, list) {
> +		af_alg_free_sg(&rsgl->sgl);
> +		if (rsgl != &ctx->first_rsgl)
> +			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
> +		list_del(&rsgl->list);
> +	}
> +	INIT_LIST_HEAD(&ctx->list);
>  	aead_wmem_wakeup(sk);
>  	release_sock(sk);
> 
>  	return err ? err : outlen;
>  }
> 
> +static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t
> ignored, +			int flags)
> +{
> +	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
> +		aead_recvmsg_async(sock, msg, flags) :
> +		aead_recvmsg_sync(sock, msg, flags);
> +}
> +
>  static unsigned int aead_poll(struct file *file, struct socket *sock,
>  			      poll_table *wait)
>  {
> @@ -539,7 +751,12 @@ static void aead_sock_destruct(struct sock *sk)
>  	struct aead_ctx *ctx = ask->private;
>  	unsigned int ivlen = crypto_aead_ivsize(
>  				crypto_aead_reqtfm(&ctx->aead_req));
> +	int ctr = 0;
> +
> +	while (atomic_read(&sk->sk_refcnt) != 0 && ctr++ < 10)
> +		msleep(100);
> 
> +	WARN_ON(atomic_read(&sk->sk_refcnt) != 0);
>  	aead_put_sgl(sk);
>  	sock_kzfree_s(sk, ctx->iv, ivlen);
>  	sock_kfree_s(sk, ctx, ctx->len);
> @@ -574,6 +791,7 @@ static int aead_accept_parent(void *private, struct sock
> *sk) ctx->aead_assoclen = 0;
>  	af_alg_init_completion(&ctx->completion);
>  	sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
> +	INIT_LIST_HEAD(&ctx->list);
> 
>  	ask->private = ctx;


Ciao
Stephan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-27 22:29 ` kbuild test robot
@ 2016-01-27 22:41   ` Tadeusz Struk
  0 siblings, 0 replies; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-27 22:41 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, herbert, smueller, linux-crypto, linux-kernel

On 01/27/2016 02:29 PM, kbuild test robot wrote:
> Hi Tadeusz,
> 
> [auto build test ERROR on cryptodev/master]
> [also build test ERROR on v4.5-rc1 next-20160127]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Tadeusz-Struk/crypto-af_alg-add-async-support-to-algif_aead/20160128-061818
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
> config: x86_64-randconfig-x011-01270835 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    crypto/algif_aead.c: In function 'aead_async_cb':
>>> crypto/algif_aead.c:386:29: error: implicit declaration of function 'aead_request_cast' [-Werror=implicit-function-declaration]
>      struct aead_request *req = aead_request_cast(_req);
>                                 ^
>>> crypto/algif_aead.c:386:29: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>    cc1: some warnings being treated as errors
> 
> vim +/aead_request_cast +386 crypto/algif_aead.c
> 
>    380	static void aead_async_cb(struct crypto_async_request *_req, int err)
>    381	{
>    382		struct sock *sk = _req->data;
>    383		struct alg_sock *ask = alg_sk(sk);
>    384		struct aead_ctx *ctx = ask->private;
>    385		struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
>  > 386		struct aead_request *req = aead_request_cast(_req);
>    387		struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
>    388		struct scatterlist *sg = areq->tsgl;
>    389		struct aead_async_rsgl *rsgl;
> 

Thanks for the report.
It has dependency on this one https://patchwork.kernel.org/patch/8144731/
which has been sent one minute earlier.
I do not want to squash them into one patch, because they are not really related.
Herbert, how should this be handled?
Thanks,
 
-- 
TS

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] crypto: af_alg - add async support to algif_aead
  2016-01-27 22:10 Tadeusz Struk
@ 2016-01-27 22:29 ` kbuild test robot
  2016-01-27 22:41   ` Tadeusz Struk
  2016-01-28  6:26 ` Stephan Mueller
  1 sibling, 1 reply; 14+ messages in thread
From: kbuild test robot @ 2016-01-27 22:29 UTC (permalink / raw)
  To: Tadeusz Struk
  Cc: kbuild-all, herbert, smueller, tadeusz.struk, linux-crypto, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

Hi Tadeusz,

[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.5-rc1 next-20160127]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Tadeusz-Struk/crypto-af_alg-add-async-support-to-algif_aead/20160128-061818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: x86_64-randconfig-x011-01270835 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   crypto/algif_aead.c: In function 'aead_async_cb':
>> crypto/algif_aead.c:386:29: error: implicit declaration of function 'aead_request_cast' [-Werror=implicit-function-declaration]
     struct aead_request *req = aead_request_cast(_req);
                                ^
>> crypto/algif_aead.c:386:29: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   cc1: some warnings being treated as errors

vim +/aead_request_cast +386 crypto/algif_aead.c

   380	static void aead_async_cb(struct crypto_async_request *_req, int err)
   381	{
   382		struct sock *sk = _req->data;
   383		struct alg_sock *ask = alg_sk(sk);
   384		struct aead_ctx *ctx = ask->private;
   385		struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
 > 386		struct aead_request *req = aead_request_cast(_req);
   387		struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
   388		struct scatterlist *sg = areq->tsgl;
   389		struct aead_async_rsgl *rsgl;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 28726 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] crypto: af_alg - add async support to algif_aead
@ 2016-01-27 22:10 Tadeusz Struk
  2016-01-27 22:29 ` kbuild test robot
  2016-01-28  6:26 ` Stephan Mueller
  0 siblings, 2 replies; 14+ messages in thread
From: Tadeusz Struk @ 2016-01-27 22:10 UTC (permalink / raw)
  To: herbert; +Cc: smueller, tadeusz.struk, linux-crypto, linux-kernel

Following the async change for algif_skcipher
this patch adds similar async read to algif_aead.

changes in v2:
- change internal data structures from fixed size arrays, limited to
  RSGL_MAX_ENTRIES, to linked list model with no artificial limitation.
- use sock_kmalloc instead of kmalloc for memory allocation
- use sock_hold instead of separate atomic ctr to wait for outstanding request

Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 crypto/algif_aead.c |  278 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 248 insertions(+), 30 deletions(-)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 147069c..3fa1a95 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -29,15 +29,24 @@ struct aead_sg_list {
 	struct scatterlist sg[ALG_MAX_PAGES];
 };
 
+struct aead_async_rsgl {
+	struct af_alg_sgl sgl;
+	struct list_head list;
+};
+
+struct aead_async_req {
+	struct scatterlist *tsgl;
+	struct aead_async_rsgl first_rsgl;
+	struct list_head list;
+	struct kiocb *iocb;
+	unsigned int tsgls;
+	char iv[];
+};
+
 struct aead_ctx {
 	struct aead_sg_list tsgl;
-	/*
-	 * RSGL_MAX_ENTRIES is an artificial limit where user space at maximum
-	 * can cause the kernel to allocate RSGL_MAX_ENTRIES * ALG_MAX_PAGES
-	 * pages
-	 */
-#define RSGL_MAX_ENTRIES ALG_MAX_PAGES
-	struct af_alg_sgl rsgl[RSGL_MAX_ENTRIES];
+	struct aead_async_rsgl first_rsgl;
+	struct list_head list;
 
 	void *iv;
 
@@ -75,6 +84,17 @@ static inline bool aead_sufficient_data(struct aead_ctx *ctx)
 	return ctx->used >= ctx->aead_assoclen + as;
 }
 
+static void aead_reset_ctx(struct aead_ctx *ctx)
+{
+	struct aead_sg_list *sgl = &ctx->tsgl;
+
+	sg_init_table(sgl->sg, ALG_MAX_PAGES);
+	sgl->cur = 0;
+	ctx->used = 0;
+	ctx->more = 0;
+	ctx->merge = 0;
+}
+
 static void aead_put_sgl(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -90,11 +110,6 @@ static void aead_put_sgl(struct sock *sk)
 		put_page(sg_page(sg + i));
 		sg_assign_page(sg + i, NULL);
 	}
-	sg_init_table(sg, ALG_MAX_PAGES);
-	sgl->cur = 0;
-	ctx->used = 0;
-	ctx->more = 0;
-	ctx->merge = 0;
 }
 
 static void aead_wmem_wakeup(struct sock *sk)
@@ -240,6 +255,7 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 		if (!aead_writable(sk)) {
 			/* user space sent too much data */
 			aead_put_sgl(sk);
+			aead_reset_ctx(ctx);
 			err = -EMSGSIZE;
 			goto unlock;
 		}
@@ -251,6 +267,7 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 
 			if (sgl->cur >= ALG_MAX_PAGES) {
 				aead_put_sgl(sk);
+				aead_reset_ctx(ctx);
 				err = -E2BIG;
 				goto unlock;
 			}
@@ -287,6 +304,7 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	ctx->more = msg->msg_flags & MSG_MORE;
 	if (!ctx->more && !aead_sufficient_data(ctx)) {
 		aead_put_sgl(sk);
+		aead_reset_ctx(ctx);
 		err = -EMSGSIZE;
 	}
 
@@ -322,6 +340,7 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 	if (!aead_writable(sk)) {
 		/* user space sent too much data */
 		aead_put_sgl(sk);
+		aead_reset_ctx(ctx);
 		err = -EMSGSIZE;
 		goto unlock;
 	}
@@ -339,6 +358,7 @@ done:
 	ctx->more = flags & MSG_MORE;
 	if (!ctx->more && !aead_sufficient_data(ctx)) {
 		aead_put_sgl(sk);
+		aead_reset_ctx(ctx);
 		err = -EMSGSIZE;
 	}
 
@@ -349,23 +369,189 @@ unlock:
 	return err ?: size;
 }
 
-static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored, int flags)
+#define GET_ASYM_REQ(req, tfm) (struct aead_async_req *) \
+		((char *)req + sizeof(struct aead_request) + \
+		 crypto_aead_reqsize(tfm))
+
+ #define GET_REQ_SIZE(tfm) sizeof(struct aead_async_req) + \
+	crypto_aead_reqsize(tfm) + crypto_aead_ivsize(tfm) + \
+	sizeof(struct aead_request)
+
+static void aead_async_cb(struct crypto_async_request *_req, int err)
+{
+	struct sock *sk = _req->data;
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
+	struct aead_request *req = aead_request_cast(_req);
+	struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
+	struct scatterlist *sg = areq->tsgl;
+	struct aead_async_rsgl *rsgl;
+	struct kiocb *iocb = areq->iocb;
+	unsigned int i, reqlen = GET_REQ_SIZE(tfm);
+
+	list_for_each_entry(rsgl, &areq->list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		if (rsgl != &areq->first_rsgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+
+	for (i = 0; i < areq->tsgls; i++)
+		put_page(sg_page(sg + i));
+
+	sock_kfree_s(sk, areq->tsgl, sizeof(*areq->tsgl) * areq->tsgls);
+	sock_kfree_s(sk, req, reqlen);
+	__sock_put(sk);
+	iocb->ki_complete(iocb, err, err);
+}
+
+static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
+			      int flags)
+{
+	struct sock *sk = sock->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
+	struct aead_async_req *areq;
+	struct aead_request *req = NULL;
+	struct aead_sg_list *sgl = &ctx->tsgl;
+	struct aead_async_rsgl *last_rsgl = NULL, *rsgl;
+	unsigned int as = crypto_aead_authsize(tfm);
+	unsigned int i, reqlen = GET_REQ_SIZE(tfm);
+	int err = -ENOMEM;
+	unsigned long used;
+	size_t outlen;
+	size_t usedpages = 0;
+
+	lock_sock(sk);
+	if (ctx->more) {
+		err = aead_wait_for_data(sk, flags);
+		if (err)
+			goto unlock;
+	}
+
+	used = ctx->used;
+	outlen = used;
+
+	if (!aead_sufficient_data(ctx))
+		goto unlock;
+
+	req = sock_kmalloc(sk, reqlen, GFP_KERNEL);
+	if (unlikely(!req))
+		goto unlock;
+
+	areq = GET_ASYM_REQ(req, tfm);
+	memset(&areq->first_rsgl, '\0', sizeof(areq->first_rsgl));
+	INIT_LIST_HEAD(&areq->list);
+	areq->iocb = msg->msg_iocb;
+	memcpy(areq->iv, ctx->iv, crypto_aead_ivsize(tfm));
+	aead_request_set_tfm(req, tfm);
+	aead_request_set_ad(req, ctx->aead_assoclen);
+	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				  aead_async_cb, sk);
+	used -= ctx->aead_assoclen + (ctx->enc ? as : 0);
+
+	/* take over all tx sgls from ctx */
+	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * sgl->cur,
+				  GFP_KERNEL);
+	if (unlikely(!areq->tsgl))
+		goto free;
+
+	sg_init_table(areq->tsgl, sgl->cur);
+	for (i = 0; i < sgl->cur; i++)
+		sg_set_page(&areq->tsgl[i], sg_page(&sgl->sg[i]),
+			    sgl->sg[i].length, sgl->sg[i].offset);
+
+	areq->tsgls = sgl->cur;
+
+	/* create rx sgls */
+	while (iov_iter_count(&msg->msg_iter)) {
+		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
+				      (outlen - usedpages));
+
+		if (list_empty(&areq->list))
+			rsgl = &areq->first_rsgl;
+		else {
+			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
+			if (unlikely(!rsgl)) {
+				err = -ENOMEM;
+				goto free;
+			}
+		}
+		rsgl->sgl.npages = 0;
+		list_add_tail(&rsgl->list, &areq->list);
+
+		/* make one iovec available as scatterlist */
+		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
+		if (err < 0)
+			goto free;
+
+		usedpages += err;
+
+		/* chain the new scatterlist with previous one */
+		if (last_rsgl)
+			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
+
+		last_rsgl = rsgl;
+
+		/* we do not need more iovecs as we have sufficient memory */
+		if (outlen <= usedpages)
+			break;
+
+		iov_iter_advance(&msg->msg_iter, err);
+	}
+	err = -EINVAL;
+	/* ensure output buffer is sufficiently large */
+	if (usedpages < outlen)
+		goto free;
+
+	aead_request_set_crypt(req, areq->tsgl, areq->first_rsgl.sgl.sg, used,
+			       areq->iv);
+	err = ctx->enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+	if (err) {
+		if (err == -EINPROGRESS) {
+			sock_hold(sk);
+			err = -EIOCBQUEUED;
+			aead_reset_ctx(ctx);
+			goto unlock;
+		} else if (err == -EBADMSG) {
+			aead_put_sgl(sk);
+			aead_reset_ctx(ctx);
+		}
+		goto free;
+	}
+	aead_put_sgl(sk);
+	aead_reset_ctx(ctx);
+
+free:
+	list_for_each_entry(rsgl, &areq->list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		if (rsgl != &areq->first_rsgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+	if (areq->tsgl)
+		sock_kfree_s(sk, areq->tsgl, sizeof(*areq->tsgl) * areq->tsgls);
+	if (req)
+		sock_kfree_s(sk, req, reqlen);
+unlock:
+	aead_wmem_wakeup(sk);
+	release_sock(sk);
+	return err ? err : outlen;
+}
+
+static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
 	unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
 	struct aead_sg_list *sgl = &ctx->tsgl;
-	unsigned int i = 0;
+	struct aead_async_rsgl *last_rsgl = NULL;
+	struct aead_async_rsgl *rsgl, *tmp;
 	int err = -EINVAL;
 	unsigned long used = 0;
 	size_t outlen = 0;
 	size_t usedpages = 0;
-	unsigned int cnt = 0;
-
-	/* Limit number of IOV blocks to be accessed below */
-	if (msg->msg_iter.nr_segs > RSGL_MAX_ENTRIES)
-		return -ENOMSG;
 
 	lock_sock(sk);
 
@@ -417,21 +603,33 @@ static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
 		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
 				      (outlen - usedpages));
 
+		if (list_empty(&ctx->list))
+			rsgl = &ctx->first_rsgl;
+		else {
+			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
+			if (unlikely(!rsgl)) {
+				err = -ENOMEM;
+				goto unlock;
+			}
+		}
+		rsgl->sgl.npages = 0;
+		list_add_tail(&rsgl->list, &ctx->list);
+
 		/* make one iovec available as scatterlist */
-		err = af_alg_make_sg(&ctx->rsgl[cnt], &msg->msg_iter,
-				     seglen);
+		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
 		if (err < 0)
 			goto unlock;
 		usedpages += err;
 		/* chain the new scatterlist with previous one */
-		if (cnt)
-			af_alg_link_sg(&ctx->rsgl[cnt-1], &ctx->rsgl[cnt]);
+		if (last_rsgl)
+			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
+
+		last_rsgl = rsgl;
 
 		/* we do not need more iovecs as we have sufficient memory */
 		if (outlen <= usedpages)
 			break;
 		iov_iter_advance(&msg->msg_iter, err);
-		cnt++;
 	}
 
 	err = -EINVAL;
@@ -440,8 +638,7 @@ static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
 		goto unlock;
 
 	sg_mark_end(sgl->sg + sgl->cur - 1);
-
-	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx->rsgl[0].sg,
+	aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx->first_rsgl.sgl.sg,
 			       used, ctx->iv);
 	aead_request_set_ad(&ctx->aead_req, ctx->aead_assoclen);
 
@@ -452,25 +649,40 @@ static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
 
 	if (err) {
 		/* EBADMSG implies a valid cipher operation took place */
-		if (err == -EBADMSG)
+		if (err == -EBADMSG) {
 			aead_put_sgl(sk);
+			aead_reset_ctx(ctx);
+		}
 		goto unlock;
 	}
 
 	aead_put_sgl(sk);
+	aead_reset_ctx(ctx);
 
 	err = 0;
 
 unlock:
-	for (i = 0; i < cnt; i++)
-		af_alg_free_sg(&ctx->rsgl[i]);
-
+	list_for_each_entry_safe(rsgl, tmp, &ctx->list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		if (rsgl != &ctx->first_rsgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+		list_del(&rsgl->list);
+	}
+	INIT_LIST_HEAD(&ctx->list);
 	aead_wmem_wakeup(sk);
 	release_sock(sk);
 
 	return err ? err : outlen;
 }
 
+static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
+			int flags)
+{
+	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
+		aead_recvmsg_async(sock, msg, flags) :
+		aead_recvmsg_sync(sock, msg, flags);
+}
+
 static unsigned int aead_poll(struct file *file, struct socket *sock,
 			      poll_table *wait)
 {
@@ -539,7 +751,12 @@ static void aead_sock_destruct(struct sock *sk)
 	struct aead_ctx *ctx = ask->private;
 	unsigned int ivlen = crypto_aead_ivsize(
 				crypto_aead_reqtfm(&ctx->aead_req));
+	int ctr = 0;
+
+	while (atomic_read(&sk->sk_refcnt) != 0 && ctr++ < 10)
+		msleep(100);
 
+	WARN_ON(atomic_read(&sk->sk_refcnt) != 0);
 	aead_put_sgl(sk);
 	sock_kzfree_s(sk, ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx, ctx->len);
@@ -574,6 +791,7 @@ static int aead_accept_parent(void *private, struct sock *sk)
 	ctx->aead_assoclen = 0;
 	af_alg_init_completion(&ctx->completion);
 	sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
+	INIT_LIST_HEAD(&ctx->list);
 
 	ask->private = ctx;
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-01-28 17:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-15 19:21 [PATCH] crypto: af_alg - add async support to algif_aead Tadeusz Struk
2016-01-17 15:07 ` Stephan Mueller
2016-01-18 15:22   ` Tadeusz Struk
2016-01-19  0:34     ` Herbert Xu
2016-01-19 15:18       ` Tadeusz Struk
2016-01-20 20:18       ` Tadeusz Struk
2016-01-21  5:00         ` Herbert Xu
2016-01-27 22:10 Tadeusz Struk
2016-01-27 22:29 ` kbuild test robot
2016-01-27 22:41   ` Tadeusz Struk
2016-01-28  6:26 ` Stephan Mueller
2016-01-28 16:00   ` Tadeusz Struk
2016-01-28 17:09     ` Stephan Mueller
2016-01-28 17:30       ` Tadeusz Struk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).