Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH v5 2/2] crypto: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-17 22:32 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <2523592.mde6d2a8Lg@positron.chronox.de>

The updated memory management is described in the top part of the code.
As one benefit of the changed memory management, the AIO and synchronous
operation is now implemented in one common function. The AF_ALG
operation uses the async kernel crypto API interface for each cipher
operation. Thus, the only difference between the AIO and sync operation
types visible from user space is:

1. the callback function to be invoked when the asynchronous operation
   is completed

2. whether to wait for the completion of the kernel crypto API operation
   or not

The change includes the overhaul of the TX and RX SGL handling. The TX
SGL holding the data sent from user space to the kernel is now dynamic
similar to algif_skcipher. This dynamic nature allows a continuous
operation of a thread sending data and a second thread receiving the
data. These threads do not need to synchronize as the kernel processes
as much data from the TX SGL to fill the RX SGL.

The caller reading the data from the kernel defines the amount of data
to be processed. Considering that the interface covers AEAD
authenticating ciphers, the reader must provide the buffer in the
correct size. Thus the reader defines the encryption size.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_aead.c | 702 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 386 insertions(+), 316 deletions(-)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 533265f..050a866 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -5,12 +5,26 @@
  *
  * This file provides the user-space API for AEAD ciphers.
  *
- * This file is derived from algif_skcipher.c.
- *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free
  * Software Foundation; either version 2 of the License, or (at your option)
  * any later version.
+ *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
  */
 
 #include <crypto/internal/aead.h>
@@ -24,45 +38,57 @@
 #include <linux/net.h>
 #include <net/sock.h>
 
-struct aead_sg_list {
-	unsigned int cur;
-	struct scatterlist sg[ALG_MAX_PAGES];
+struct aead_tsgl {
+	struct list_head list;
+	unsigned int cur;		/* Last processed SG entry */
+	struct scatterlist sg[0];	/* Array of SGs forming the SGL */
 };
 
-struct aead_async_rsgl {
+struct aead_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 sock *sk;
+
+	struct aead_rsgl first_rsgl;	/* First RX SG */
+	struct list_head rsgl_list;	/* Track RX SGs */
+
+	struct scatterlist *tsgl;	/* priv. TX SGL of buffers to process */
+	unsigned int tsgl_entries;	/* number of entries in priv. TX SGL */
+
+	unsigned int outlen;		/* Filled output buf length */
+
+	unsigned int areqlen;		/* Length of this data struct */
+	struct aead_request aead_req;	/* req ctx trails this struct */
 };
 
 struct aead_ctx {
-	struct aead_sg_list tsgl;
-	struct aead_async_rsgl first_rsgl;
-	struct list_head list;
+	struct list_head tsgl_list;	/* Link to TX SGL */
 
 	void *iv;
+	size_t aead_assoclen;
 
-	struct af_alg_completion completion;
+	struct af_alg_completion completion;	/* sync work queue */
 
-	unsigned long used;
+	unsigned int inflight;	/* Outstanding AIO ops */
+	size_t used;		/* TX bytes sent to kernel */
 
-	unsigned int len;
-	bool more;
-	bool merge;
-	bool enc;
+	bool more;		/* More data to be expected? */
+	bool merge;		/* Merge new data into existing SG */
+	bool enc;		/* Crypto operation: enc, dec */
 
-	size_t aead_assoclen;
-	struct aead_request aead_req;
+	unsigned int len;	/* Length of allocated memory for this struct */
+	struct crypto_aead *aead_tfm;
 };
 
+static DECLARE_WAIT_QUEUE_HEAD(aead_aio_finish_wait);
+
+#define MAX_SGL_ENTS ((4096 - sizeof(struct aead_tsgl)) / \
+		      sizeof(struct scatterlist) - 1)
+
 static inline int aead_sndbuf(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -79,7 +105,7 @@ static inline bool aead_writable(struct sock *sk)
 
 static inline bool aead_sufficient_data(struct aead_ctx *ctx)
 {
-	unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
+	unsigned int as = crypto_aead_authsize(ctx->aead_tfm);
 
 	/*
 	 * The minimum amount of memory needed for an AEAD cipher is
@@ -88,33 +114,163 @@ static inline bool aead_sufficient_data(struct aead_ctx *ctx)
 	return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as);
 }
 
-static void aead_reset_ctx(struct aead_ctx *ctx)
+static int aead_alloc_tsgl(struct sock *sk)
 {
-	struct aead_sg_list *sgl = &ctx->tsgl;
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct aead_tsgl *sgl;
+	struct scatterlist *sg = NULL;
 
-	sg_init_table(sgl->sg, ALG_MAX_PAGES);
-	sgl->cur = 0;
-	ctx->used = 0;
-	ctx->more = 0;
-	ctx->merge = 0;
+	sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl, list);
+	if (!list_empty(&ctx->tsgl_list))
+		sg = sgl->sg;
+
+	if (!sg || sgl->cur >= MAX_SGL_ENTS) {
+		sgl = sock_kmalloc(sk, sizeof(*sgl) +
+				       sizeof(sgl->sg[0]) * (MAX_SGL_ENTS + 1),
+				   GFP_KERNEL);
+		if (!sgl)
+			return -ENOMEM;
+
+		sg_init_table(sgl->sg, MAX_SGL_ENTS + 1);
+		sgl->cur = 0;
+
+		if (sg)
+			sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
+
+		list_add_tail(&sgl->list, &ctx->tsgl_list);
+	}
+
+	return 0;
+}
+
+static unsigned int aead_count_tsgl(struct sock *sk, size_t bytes)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct aead_tsgl *sgl, *tmp;
+	unsigned int i;
+	unsigned int sgl_count = 0;
+
+	if (!bytes)
+		return 0;
+
+	list_for_each_entry_safe(sgl, tmp, &ctx->tsgl_list, list) {
+		struct scatterlist *sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			sgl_count++;
+			if (sg[i].length >= bytes)
+				return sgl_count;
+
+			bytes -= sg[i].length;
+		}
+	}
+
+	return sgl_count;
 }
 
-static void aead_put_sgl(struct sock *sk)
+static void aead_pull_tsgl(struct sock *sk, size_t used,
+			   struct scatterlist *dst)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	struct aead_sg_list *sgl = &ctx->tsgl;
-	struct scatterlist *sg = sgl->sg;
+	struct aead_tsgl *sgl;
+	struct scatterlist *sg;
 	unsigned int i;
 
-	for (i = 0; i < sgl->cur; i++) {
-		if (!sg_page(sg + i))
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct aead_tsgl,
+				       list);
+		sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			size_t plen = min_t(size_t, used, sg[i].length);
+			struct page *page = sg_page(sg + i);
+
+			if (!page)
+				continue;
+
+			/*
+			 * Assumption: caller created aead_count_tsgl(len)
+			 * SG entries in dst.
+			 */
+			if (dst)
+				sg_set_page(dst + i, page, plen, sg[i].offset);
+
+			sg[i].length -= plen;
+			sg[i].offset += plen;
+
+			used -= plen;
+			ctx->used -= plen;
+
+			if (sg[i].length)
+				return;
+
+			if (!dst)
+				put_page(page);
+			sg_assign_page(sg + i, NULL);
+		}
+
+		list_del(&sgl->list);
+		sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+						     (MAX_SGL_ENTS + 1));
+	}
+
+	if (!ctx->used)
+		ctx->merge = 0;
+}
+
+static void aead_free_areq_sgls(struct aead_async_req *areq)
+{
+	struct sock *sk = areq->sk;
+	struct aead_rsgl *rsgl, *tmp;
+	struct scatterlist *tsgl;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		list_del(&rsgl->list);
+		if (rsgl != &areq->first_rsgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+
+	tsgl = areq->tsgl;
+	for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
+		if (!sg_page(sg))
 			continue;
+		put_page(sg_page(sg));
+	}
+
+	if (areq->tsgl && areq->tsgl_entries)
+		sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
+}
 
-		put_page(sg_page(sg + i));
-		sg_assign_page(sg + i, NULL);
+static int aead_wait_for_wmem(struct sock *sk, unsigned flags)
+{
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	int err = -ERESTARTSYS;
+	long timeout;
+
+	if (flags & MSG_DONTWAIT)
+		return -EAGAIN;
+
+	sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+
+	add_wait_queue(sk_sleep(sk), &wait);
+	for (;;) {
+		if (signal_pending(current))
+			break;
+		timeout = MAX_SCHEDULE_TIMEOUT;
+		if (sk_wait_event(sk, &timeout, aead_writable(sk), &wait)) {
+			err = 0;
+			break;
+		}
 	}
-	aead_reset_ctx(ctx);
+	remove_wait_queue(sk_sleep(sk), &wait);
+
+	return err;
 }
 
 static void aead_wmem_wakeup(struct sock *sk)
@@ -146,6 +302,7 @@ static int aead_wait_for_data(struct sock *sk, unsigned flags)
 		return -EAGAIN;
 
 	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+
 	add_wait_queue(sk_sleep(sk), &wait);
 	for (;;) {
 		if (signal_pending(current))
@@ -169,8 +326,6 @@ static void aead_data_wakeup(struct sock *sk)
 	struct aead_ctx *ctx = ask->private;
 	struct socket_wq *wq;
 
-	if (ctx->more)
-		return;
 	if (!ctx->used)
 		return;
 
@@ -189,14 +344,13 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	unsigned ivsize =
-		crypto_aead_ivsize(crypto_aead_reqtfm(&ctx->aead_req));
-	struct aead_sg_list *sgl = &ctx->tsgl;
+	unsigned int ivsize = crypto_aead_ivsize(ctx->aead_tfm);
+	struct aead_tsgl *sgl;
 	struct af_alg_control con = {};
 	long copied = 0;
 	bool enc = 0;
 	bool init = 0;
-	int err = -EINVAL;
+	int err = 0;
 
 	if (msg->msg_controllen) {
 		err = af_alg_cmsg_send(msg, &con);
@@ -220,8 +374,10 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	}
 
 	lock_sock(sk);
-	if (!ctx->more && ctx->used)
+	if (!ctx->more && ctx->used) {
+		err = -EINVAL;
 		goto unlock;
+	}
 
 	if (init) {
 		ctx->enc = enc;
@@ -232,11 +388,14 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	}
 
 	while (size) {
+		struct scatterlist *sg;
 		size_t len = size;
-		struct scatterlist *sg = NULL;
+		size_t plen;
 
 		/* use the existing memory in an allocated page */
 		if (ctx->merge) {
+			sgl = list_entry(ctx->tsgl_list.prev,
+					 struct aead_tsgl, list);
 			sg = sgl->sg + sgl->cur - 1;
 			len = min_t(unsigned long, len,
 				    PAGE_SIZE - sg->offset - sg->length);
@@ -257,57 +416,60 @@ 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);
-			err = -EMSGSIZE;
-			goto unlock;
+			err = aead_wait_for_wmem(sk, msg->msg_flags);
+			if (err)
+				goto unlock;
 		}
 
 		/* allocate a new page */
 		len = min_t(unsigned long, size, aead_sndbuf(sk));
-		while (len) {
-			size_t plen = 0;
 
-			if (sgl->cur >= ALG_MAX_PAGES) {
-				aead_put_sgl(sk);
-				err = -E2BIG;
-				goto unlock;
-			}
+		err = aead_alloc_tsgl(sk);
+		if (err)
+			goto unlock;
+
+		sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl,
+				 list);
+		sg = sgl->sg;
+		if (sgl->cur)
+			sg_unmark_end(sg + sgl->cur - 1);
+
+		do {
+			unsigned int i = sgl->cur;
 
-			sg = sgl->sg + sgl->cur;
 			plen = min_t(size_t, len, PAGE_SIZE);
 
-			sg_assign_page(sg, alloc_page(GFP_KERNEL));
-			err = -ENOMEM;
-			if (!sg_page(sg))
+			sg_assign_page(sg + i, alloc_page(GFP_KERNEL));
+			if (!sg_page(sg + i)) {
+				err = -ENOMEM;
 				goto unlock;
+			}
 
-			err = memcpy_from_msg(page_address(sg_page(sg)),
+			err = memcpy_from_msg(page_address(sg_page(sg + i)),
 					      msg, plen);
 			if (err) {
-				__free_page(sg_page(sg));
-				sg_assign_page(sg, NULL);
+				__free_page(sg_page(sg + i));
+				sg_assign_page(sg + i, NULL);
 				goto unlock;
 			}
 
-			sg->offset = 0;
-			sg->length = plen;
+			sg[i].length = plen;
 			len -= plen;
 			ctx->used += plen;
 			copied += plen;
-			sgl->cur++;
 			size -= plen;
-			ctx->merge = plen & (PAGE_SIZE - 1);
-		}
+			sgl->cur++;
+		} while (len && sgl->cur < MAX_SGL_ENTS);
+
+		if (!size)
+			sg_mark_end(sg + sgl->cur - 1);
+
+		ctx->merge = plen & (PAGE_SIZE - 1);
 	}
 
 	err = 0;
 
 	ctx->more = msg->msg_flags & MSG_MORE;
-	if (!ctx->more && !aead_sufficient_data(ctx)) {
-		aead_put_sgl(sk);
-		err = -EMSGSIZE;
-	}
 
 unlock:
 	aead_data_wakeup(sk);
@@ -322,15 +484,12 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	struct aead_sg_list *sgl = &ctx->tsgl;
+	struct aead_tsgl *sgl;
 	int err = -EINVAL;
 
 	if (flags & MSG_SENDPAGE_NOTLAST)
 		flags |= MSG_MORE;
 
-	if (sgl->cur >= ALG_MAX_PAGES)
-		return -E2BIG;
-
 	lock_sock(sk);
 	if (!ctx->more && ctx->used)
 		goto unlock;
@@ -339,13 +498,22 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 		goto done;
 
 	if (!aead_writable(sk)) {
-		/* user space sent too much data */
-		aead_put_sgl(sk);
-		err = -EMSGSIZE;
-		goto unlock;
+		err = aead_wait_for_wmem(sk, flags);
+		if (err)
+			goto unlock;
 	}
 
+	err = aead_alloc_tsgl(sk);
+	if (err)
+		goto unlock;
+
 	ctx->merge = 0;
+	sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl, list);
+
+	if (sgl->cur)
+		sg_unmark_end(sgl->sg + sgl->cur - 1);
+
+	sg_mark_end(sgl->sg + sgl->cur);
 
 	get_page(page);
 	sg_set_page(sgl->sg + sgl->cur, page, size, offset);
@@ -356,11 +524,6 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 
 done:
 	ctx->more = flags & MSG_MORE;
-	if (!ctx->more && !aead_sufficient_data(ctx)) {
-		aead_put_sgl(sk);
-		err = -EMSGSIZE;
-	}
-
 unlock:
 	aead_data_wakeup(sk);
 	release_sock(sk);
@@ -368,205 +531,58 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 	return err ?: size;
 }
 
-#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 aead_async_req *areq = _req->data;
+	struct sock *sk = areq->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_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 = 0;
-	size_t usedpages = 0;
+	unsigned int resultlen;
 
 	lock_sock(sk);
-	if (ctx->more) {
-		err = aead_wait_for_data(sk, flags);
-		if (err)
-			goto unlock;
-	}
-
-	if (!aead_sufficient_data(ctx))
-		goto unlock;
-
-	used = ctx->used;
-	if (ctx->enc)
-		outlen = used + as;
-	else
-		outlen = used - as;
-
-	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;
-
-	/* take over all tx sgls from ctx */
-	areq->tsgl = sock_kmalloc(sk,
-				  sizeof(*areq->tsgl) * max_t(u32, sgl->cur, 1),
-				  GFP_KERNEL);
-	if (unlikely(!areq->tsgl))
-		goto free;
-
-	sg_init_table(areq->tsgl, max_t(u32, sgl->cur, 1));
-	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 (outlen > usedpages && 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);
+	BUG_ON(!ctx->inflight);
 
-		/* make one iovec available as scatterlist */
-		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
-		if (err < 0)
-			goto free;
-
-		usedpages += err;
+	/* Buffer size written by crypto operation. */
+	resultlen = areq->outlen;
 
-		/* chain the new scatterlist with previous one */
-		if (last_rsgl)
-			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
-
-		last_rsgl = rsgl;
-
-		iov_iter_advance(&msg->msg_iter, err);
-	}
-
-	/* ensure output buffer is sufficiently large */
-	if (usedpages < outlen) {
-		err = -EINVAL;
-		goto unlock;
-	}
+	aead_free_areq_sgls(areq);
+	sock_kfree_s(sk, areq, areq->areqlen);
+	__sock_put(sk);
+	ctx->inflight--;
 
-	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);
-		}
-		goto free;
-	}
-	aead_put_sgl(sk);
+	iocb->ki_complete(iocb, err ? err : resultlen, 0);
 
-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;
+
+	wake_up_interruptible(&aead_aio_finish_wait);
 }
 
-static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
+static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
+			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;
-	struct aead_async_rsgl *last_rsgl = NULL;
-	struct aead_async_rsgl *rsgl, *tmp;
+	struct crypto_aead *tfm = ctx->aead_tfm;
+	unsigned int as = crypto_aead_authsize(tfm);
+	unsigned int areqlen =
+		sizeof(struct aead_async_req) + crypto_aead_reqsize(tfm);
+	struct aead_async_req *areq;
+	struct aead_rsgl *last_rsgl = NULL;
 	int err = -EINVAL;
-	unsigned long used = 0;
-	size_t outlen = 0;
-	size_t usedpages = 0;
+	size_t used = 0;		/* [in]  TX bufs to be en/decrypted */
+	size_t outlen = 0;		/* [out] RX bufs produced by kernel */
+	size_t usedpages = 0;		/* [in]  RX bufs to be used from user */
+	size_t processed = 0;		/* [in]  TX bufs to be consumed */
 
 	lock_sock(sk);
 
 	/*
-	 * Please see documentation of aead_request_set_crypt for the
-	 * description of the AEAD memory structure expected from the caller.
+	 * Data length provided by caller via sendmsg/sendpage that has not
+	 * yet been processed.
 	 */
-
-	if (ctx->more) {
-		err = aead_wait_for_data(sk, flags);
-		if (err)
-			goto unlock;
-	}
-
-	/* data length provided by caller via sendmsg/sendpage */
 	used = ctx->used;
 
 	/*
@@ -600,96 +616,151 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
 	 */
 	used -= ctx->aead_assoclen;
 
-	/* convert iovecs of output buffers into scatterlists */
+	/* Allocate cipher request for current operation. */
+	areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+	if (unlikely(!areq)) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+	areq->areqlen = areqlen;
+	areq->sk = sk;
+	INIT_LIST_HEAD(&areq->rsgl_list);
+	areq->tsgl = NULL;
+	areq->tsgl_entries = 0;
+
+	/* convert iovecs of output buffers into RX SGL */
 	while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
-		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
-				      (outlen - usedpages));
+		struct aead_rsgl *rsgl;
+		size_t seglen;
+
+		if (!ctx->used) {
+			err = aead_wait_for_data(sk, flags);
+			if (err)
+				goto free;
+		}
+
+		seglen = min_t(size_t, (outlen - usedpages),
+			       iov_iter_count(&msg->msg_iter));
 
-		if (list_empty(&ctx->list)) {
-			rsgl = &ctx->first_rsgl;
+		if (list_empty(&areq->rsgl_list)) {
+			rsgl = &areq->first_rsgl;
 		} else {
 			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
 			if (unlikely(!rsgl)) {
 				err = -ENOMEM;
-				goto unlock;
+				goto free;
 			}
 		}
+
 		rsgl->sgl.npages = 0;
-		list_add_tail(&rsgl->list, &ctx->list);
+		list_add_tail(&rsgl->list, &areq->rsgl_list);
 
 		/* make one iovec available as scatterlist */
 		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
 		if (err < 0)
-			goto unlock;
-		usedpages += err;
+			goto free;
+
 		/* chain the new scatterlist with previous one */
 		if (last_rsgl)
 			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
 
 		last_rsgl = rsgl;
-
+		usedpages += err;
 		iov_iter_advance(&msg->msg_iter, err);
 	}
 
-	/* ensure output buffer is sufficiently large */
+	/*
+	 * Ensure output buffer is sufficiently large. If the caller provides
+	 * less buffer space, only use the relative required input size. This
+	 * allows AIO operation where the caller sent all data to be processed
+	 * and the AIO operation performs the operation on the different chunks
+	 * of the input data.
+	 */
 	if (usedpages < outlen) {
-		err = -EINVAL;
-		goto unlock;
-	}
+		size_t less = outlen - usedpages;
 
-	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);
-	aead_request_set_ad(&ctx->aead_req, ctx->aead_assoclen);
+		if (used < less) {
+			err = -EINVAL;
+			goto free;
+		}
+		used -= less;
+		outlen -= less;
+	}
 
-	err = af_alg_wait_for_completion(ctx->enc ?
-					 crypto_aead_encrypt(&ctx->aead_req) :
-					 crypto_aead_decrypt(&ctx->aead_req),
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
+	processed = used + ctx->aead_assoclen;
+	areq->tsgl_entries = aead_count_tsgl(sk, processed);
+	if (!areq->tsgl_entries)
+		areq->tsgl_entries = 1;
+	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
+				  GFP_KERNEL);
+	if (!areq->tsgl) {
+		err = -ENOMEM;
+		goto free;
+	}
+	sg_init_table(areq->tsgl, areq->tsgl_entries);
+	aead_pull_tsgl(sk, processed, areq->tsgl);
+
+	/* Initialize the crypto operation */
+	aead_request_set_crypt(&areq->aead_req, areq->tsgl,
+			       areq->first_rsgl.sgl.sg, used, ctx->iv);
+	aead_request_set_ad(&areq->aead_req, ctx->aead_assoclen);
+	aead_request_set_tfm(&areq->aead_req, tfm);
+
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+		/* AIO operation */
+		areq->iocb = msg->msg_iocb;
+		aead_request_set_callback(&areq->aead_req,
+					  CRYPTO_TFM_REQ_MAY_BACKLOG,
+					  aead_async_cb, areq);
+		err = ctx->enc ? crypto_aead_encrypt(&areq->aead_req) :
+				 crypto_aead_decrypt(&areq->aead_req);
+	} else {
+		/* Synchronous operation */
+		aead_request_set_callback(&areq->aead_req,
+					  CRYPTO_TFM_REQ_MAY_BACKLOG,
+					  af_alg_complete, &ctx->completion);
+		err = af_alg_wait_for_completion(ctx->enc ?
+					 crypto_aead_encrypt(&areq->aead_req) :
+					 crypto_aead_decrypt(&areq->aead_req),
 					 &ctx->completion);
+	}
 
-	if (err) {
-		/* EBADMSG implies a valid cipher operation took place */
-		if (err == -EBADMSG)
-			aead_put_sgl(sk);
+	/* AIO operation in progress */
+	if (err == -EINPROGRESS) {
+		sock_hold(sk);
+		err = -EIOCBQUEUED;
+		ctx->inflight++;
+
+		/* Remember output size that will be generated. */
+		areq->outlen = outlen;
 
 		goto unlock;
 	}
 
-	aead_put_sgl(sk);
-	err = 0;
+free:
+	aead_free_areq_sgls(areq);
+	if (areq)
+		sock_kfree_s(sk, areq, areqlen);
 
 unlock:
-	list_for_each_entry_safe(rsgl, tmp, &ctx->list, list) {
-		af_alg_free_sg(&rsgl->sgl);
-		list_del(&rsgl->list);
-		if (rsgl != &ctx->first_rsgl)
-			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
-	}
-	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)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	unsigned int mask;
+	unsigned int mask = 0;
 
 	sock_poll_wait(file, sk_sleep(sk), wait);
-	mask = 0;
 
 	if (!ctx->more)
 		mask |= POLLIN | POLLRDNORM;
@@ -746,11 +817,14 @@ static void aead_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	unsigned int ivlen = crypto_aead_ivsize(
-				crypto_aead_reqtfm(&ctx->aead_req));
+	unsigned int ivlen = crypto_aead_ivsize(ctx->aead_tfm);
 
 	WARN_ON(atomic_read(&sk->sk_refcnt) != 0);
-	aead_put_sgl(sk);
+
+	/* Suspend caller if AIO operations are in flight. */
+	wait_event_interruptible(aead_aio_finish_wait, (ctx->inflight == 0));
+
+	aead_pull_tsgl(sk, ctx->used, NULL);
 	sock_kzfree_s(sk, ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
@@ -760,7 +834,7 @@ static int aead_accept_parent(void *private, struct sock *sk)
 {
 	struct aead_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
-	unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(private);
+	unsigned int len = sizeof(*ctx);
 	unsigned int ivlen = crypto_aead_ivsize(private);
 
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
@@ -775,23 +849,19 @@ static int aead_accept_parent(void *private, struct sock *sk)
 	}
 	memset(ctx->iv, 0, ivlen);
 
+	INIT_LIST_HEAD(&ctx->tsgl_list);
 	ctx->len = len;
 	ctx->used = 0;
 	ctx->more = 0;
 	ctx->merge = 0;
 	ctx->enc = 0;
-	ctx->tsgl.cur = 0;
+	ctx->inflight = 0;
 	ctx->aead_assoclen = 0;
 	af_alg_init_completion(&ctx->completion);
-	sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
-	INIT_LIST_HEAD(&ctx->list);
 
+	ctx->aead_tfm = private;
 	ask->private = ctx;
 
-	aead_request_set_tfm(&ctx->aead_req, private);
-	aead_request_set_callback(&ctx->aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  af_alg_complete, &ctx->completion);
-
 	sk->sk_destruct = aead_sock_destruct;
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-17 22:31 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <2523592.mde6d2a8Lg@positron.chronox.de>

The updated memory management is described in the top part of the code.
As one benefit of the changed memory management, the AIO and synchronous
operation is now implemented in one common function. The AF_ALG
operation uses the async kernel crypto API interface for each cipher
operation. Thus, the only difference between the AIO and sync operation
types visible from user space is:

1. the callback function to be invoked when the asynchronous operation
   is completed

2. whether to wait for the completion of the kernel crypto API operation
   or not

In addition, the code structure is adjusted to match the structure of
algif_aead for easier code assessment.

The user space interface changed slightly as follows: the old AIO
operation returned zero upon success and < 0 in case of an error to user
space. As all other AF_ALG interfaces (including the sync skcipher
interface) returned the number of processed bytes upon success and < 0
in case of an error, the new skcipher interface (regardless of AIO or
sync) returns the number of processed bytes in case of success.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 540 ++++++++++++++++++++++--------------------------
 1 file changed, 251 insertions(+), 289 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a9e79d8..d3e0226 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -10,6 +10,21 @@
  * Software Foundation; either version 2 of the License, or (at your option)
  * any later version.
  *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
  */
 
 #include <crypto/scatterwalk.h>
@@ -23,86 +38,58 @@
 #include <linux/net.h>
 #include <net/sock.h>
 
-struct skcipher_sg_list {
+struct skcipher_tsgl {
 	struct list_head list;
-
 	int cur;
-
 	struct scatterlist sg[0];
 };
 
+struct skcipher_rsgl {
+	struct af_alg_sgl sgl;
+	struct list_head list;
+};
+
+struct skcipher_async_req {
+	struct kiocb *iocb;
+	struct sock *sk;
+
+	struct skcipher_rsgl first_sgl;
+	struct list_head rsgl_list;
+
+	struct scatterlist *tsgl;
+	unsigned int tsgl_entries;
+
+	unsigned int areqlen;
+	struct skcipher_request req;
+};
+
 struct skcipher_tfm {
 	struct crypto_skcipher *skcipher;
 	bool has_key;
 };
 
 struct skcipher_ctx {
-	struct list_head tsgl;
-	struct af_alg_sgl rsgl;
+	struct list_head tsgl_list;
 
 	void *iv;
 
 	struct af_alg_completion completion;
 
-	atomic_t inflight;
+	unsigned int inflight;
 	size_t used;
 
-	unsigned int len;
 	bool more;
 	bool merge;
 	bool enc;
 
-	struct skcipher_request req;
-};
-
-struct skcipher_async_rsgl {
-	struct af_alg_sgl sgl;
-	struct list_head list;
+	unsigned int len;
 };
 
-struct skcipher_async_req {
-	struct kiocb *iocb;
-	struct skcipher_async_rsgl first_sgl;
-	struct list_head list;
-	struct scatterlist *tsg;
-	atomic_t *inflight;
-	struct skcipher_request req;
-};
+static DECLARE_WAIT_QUEUE_HEAD(skcipher_aio_finish_wait);
 
-#define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_sg_list)) / \
+#define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_tsgl)) / \
 		      sizeof(struct scatterlist) - 1)
 
-static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
-{
-	struct skcipher_async_rsgl *rsgl, *tmp;
-	struct scatterlist *sgl;
-	struct scatterlist *sg;
-	int i, n;
-
-	list_for_each_entry_safe(rsgl, tmp, &sreq->list, list) {
-		af_alg_free_sg(&rsgl->sgl);
-		if (rsgl != &sreq->first_sgl)
-			kfree(rsgl);
-	}
-	sgl = sreq->tsg;
-	n = sg_nents(sgl);
-	for_each_sg(sgl, sg, n, i)
-		put_page(sg_page(sg));
-
-	kfree(sreq->tsg);
-}
-
-static void skcipher_async_cb(struct crypto_async_request *req, int err)
-{
-	struct skcipher_async_req *sreq = req->data;
-	struct kiocb *iocb = sreq->iocb;
-
-	atomic_dec(sreq->inflight);
-	skcipher_free_async_sgls(sreq);
-	kzfree(sreq);
-	iocb->ki_complete(iocb, err, err);
-}
-
 static inline int skcipher_sndbuf(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -117,15 +104,15 @@ static inline bool skcipher_writable(struct sock *sk)
 	return PAGE_SIZE <= skcipher_sndbuf(sk);
 }
 
-static int skcipher_alloc_sgl(struct sock *sk)
+static int skcipher_alloc_tsgl(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	struct scatterlist *sg = NULL;
 
-	sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
-	if (!list_empty(&ctx->tsgl))
+	sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_tsgl, list);
+	if (!list_empty(&ctx->tsgl_list))
 		sg = sgl->sg;
 
 	if (!sg || sgl->cur >= MAX_SGL_ENTS) {
@@ -141,31 +128,66 @@ static int skcipher_alloc_sgl(struct sock *sk)
 		if (sg)
 			sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
 
-		list_add_tail(&sgl->list, &ctx->tsgl);
+		list_add_tail(&sgl->list, &ctx->tsgl_list);
 	}
 
 	return 0;
 }
 
-static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
+static unsigned int skcipher_count_tsgl(struct sock *sk, size_t bytes)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct skcipher_ctx *ctx = ask->private;
+	struct skcipher_tsgl *sgl, *tmp;
+	unsigned int i;
+	unsigned int sgl_count = 0;
+
+	if (!bytes)
+		return 0;
+
+	list_for_each_entry_safe(sgl, tmp, &ctx->tsgl_list, list) {
+		struct scatterlist *sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			sgl_count++;
+			if (sg[i].length >= bytes)
+				return sgl_count;
+
+			bytes -= sg[i].length;
+		}
+	}
+
+	return sgl_count;
+}
+
+static void skcipher_pull_tsgl(struct sock *sk, size_t used,
+			       struct scatterlist *dst)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	struct scatterlist *sg;
-	int i;
+	unsigned int i;
 
-	while (!list_empty(&ctx->tsgl)) {
-		sgl = list_first_entry(&ctx->tsgl, struct skcipher_sg_list,
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct skcipher_tsgl,
 				       list);
 		sg = sgl->sg;
 
 		for (i = 0; i < sgl->cur; i++) {
 			size_t plen = min_t(size_t, used, sg[i].length);
+			struct page *page = sg_page(sg + i);
 
-			if (!sg_page(sg + i))
+			if (!page)
 				continue;
 
+			/*
+			 * Assumption: caller created skcipher_count_tsgl(len)
+			 * SG entries in dst.
+			 */
+			if (dst)
+				sg_set_page(dst + i, page, plen, sg[i].offset);
+
 			sg[i].length -= plen;
 			sg[i].offset += plen;
 
@@ -174,27 +196,45 @@ static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
 
 			if (sg[i].length)
 				return;
-			if (put)
-				put_page(sg_page(sg + i));
+
+			if (!dst)
+				put_page(page);
 			sg_assign_page(sg + i, NULL);
 		}
 
 		list_del(&sgl->list);
-		sock_kfree_s(sk, sgl,
-			     sizeof(*sgl) + sizeof(sgl->sg[0]) *
-					    (MAX_SGL_ENTS + 1));
+		sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+						     (MAX_SGL_ENTS + 1));
 	}
 
 	if (!ctx->used)
 		ctx->merge = 0;
 }
 
-static void skcipher_free_sgl(struct sock *sk)
+static void skcipher_free_areq_sgls(struct skcipher_async_req *areq)
 {
-	struct alg_sock *ask = alg_sk(sk);
-	struct skcipher_ctx *ctx = ask->private;
+	struct sock *sk = areq->sk;
+	struct skcipher_rsgl *rsgl, *tmp;
+	struct scatterlist *tsgl;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		list_del(&rsgl->list);
+		if (rsgl != &areq->first_sgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+
+	tsgl = areq->tsgl;
+	for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
+		if (!sg_page(sg))
+			continue;
+		put_page(sg_page(sg));
+	}
 
-	skcipher_pull_sgl(sk, ctx->used, 1);
+	if (areq->tsgl && areq->tsgl_entries)
+		sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
 }
 
 static int skcipher_wait_for_wmem(struct sock *sk, unsigned flags)
@@ -301,7 +341,7 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 	struct skcipher_tfm *skc = pask->private;
 	struct crypto_skcipher *tfm = skc->skcipher;
 	unsigned ivsize = crypto_skcipher_ivsize(tfm);
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	struct af_alg_control con = {};
 	long copied = 0;
 	bool enc = 0;
@@ -348,8 +388,8 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 		size_t plen;
 
 		if (ctx->merge) {
-			sgl = list_entry(ctx->tsgl.prev,
-					 struct skcipher_sg_list, list);
+			sgl = list_entry(ctx->tsgl_list.prev,
+					 struct skcipher_tsgl, list);
 			sg = sgl->sg + sgl->cur - 1;
 			len = min_t(unsigned long, len,
 				    PAGE_SIZE - sg->offset - sg->length);
@@ -378,11 +418,12 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		len = min_t(unsigned long, len, skcipher_sndbuf(sk));
 
-		err = skcipher_alloc_sgl(sk);
+		err = skcipher_alloc_tsgl(sk);
 		if (err)
 			goto unlock;
 
-		sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
+		sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_tsgl,
+				 list);
 		sg = sgl->sg;
 		if (sgl->cur)
 			sg_unmark_end(sg + sgl->cur - 1);
@@ -434,7 +475,7 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	int err = -EINVAL;
 
 	if (flags & MSG_SENDPAGE_NOTLAST)
@@ -453,12 +494,12 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
 			goto unlock;
 	}
 
-	err = skcipher_alloc_sgl(sk);
+	err = skcipher_alloc_tsgl(sk);
 	if (err)
 		goto unlock;
 
 	ctx->merge = 0;
-	sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
+	sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_tsgl, list);
 
 	if (sgl->cur)
 		sg_unmark_end(sgl->sg + sgl->cur - 1);
@@ -479,25 +520,36 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
 	return err ?: size;
 }
 
-static int skcipher_all_sg_nents(struct skcipher_ctx *ctx)
+static void skcipher_async_cb(struct crypto_async_request *req, int err)
 {
-	struct skcipher_sg_list *sgl;
-	struct scatterlist *sg;
-	int nents = 0;
+	struct skcipher_async_req *areq = req->data;
+	struct sock *sk = areq->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct skcipher_ctx *ctx = ask->private;
+	struct kiocb *iocb = areq->iocb;
+	unsigned int resultlen;
 
-	list_for_each_entry(sgl, &ctx->tsgl, list) {
-		sg = sgl->sg;
+	lock_sock(sk);
 
-		while (!sg->length)
-			sg++;
+	BUG_ON(!ctx->inflight);
 
-		nents += sg_nents(sg);
-	}
-	return nents;
+	/* Buffer size written by crypto operation. */
+	resultlen = areq->req.cryptlen;
+
+	skcipher_free_areq_sgls(areq);
+	sock_kfree_s(sk, areq, areq->areqlen);
+	__sock_put(sk);
+	ctx->inflight--;
+
+	iocb->ki_complete(iocb, err ? err : resultlen, 0);
+
+	release_sock(sk);
+
+	wake_up_interruptible(&skcipher_aio_finish_wait);
 }
 
-static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
-				  int flags)
+static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+			    size_t ignored, int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
@@ -506,215 +558,137 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 	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;
-	unsigned int reqsize = crypto_skcipher_reqsize(tfm);
-	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+	unsigned int bs = crypto_skcipher_blocksize(tfm);
+	unsigned int areqlen = sizeof(struct skcipher_async_req) +
+			       crypto_skcipher_reqsize(tfm);
+	struct skcipher_async_req *areq;
+	struct skcipher_rsgl *last_rsgl = NULL;
 	int err = -ENOMEM;
-	bool mark = false;
-	char *iv;
-
-	sreq = kzalloc(sizeof(*sreq) + reqsize + ivsize, GFP_KERNEL);
-	if (unlikely(!sreq))
-		goto out;
-
-	req = &sreq->req;
-	iv = (char *)(req + 1) + reqsize;
-	sreq->iocb = msg->msg_iocb;
-	INIT_LIST_HEAD(&sreq->list);
-	sreq->inflight = &ctx->inflight;
+	size_t len = 0;
 
 	lock_sock(sk);
-	tx_nents = skcipher_all_sg_nents(ctx);
-	sreq->tsg = kcalloc(tx_nents, sizeof(*sg), GFP_KERNEL);
-	if (unlikely(!sreq->tsg))
+
+	/* Allocate cipher request for current operation. */
+	areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+	if (unlikely(!areq))
 		goto unlock;
-	sg_init_table(sreq->tsg, tx_nents);
-	memcpy(iv, ctx->iv, ivsize);
-	skcipher_request_set_tfm(req, tfm);
-	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
-				      skcipher_async_cb, sreq);
+	areq->areqlen = areqlen;
+	areq->sk = sk;
+	INIT_LIST_HEAD(&areq->rsgl_list);
+	areq->tsgl = NULL;
+	areq->tsgl_entries = 0;
 
-	while (iov_iter_count(&msg->msg_iter)) {
-		struct skcipher_async_rsgl *rsgl;
-		int used;
+	/* convert iovecs of output buffers into RX SGL */
+	while (len < ctx->used && iov_iter_count(&msg->msg_iter)) {
+		struct skcipher_rsgl *rsgl;
+		size_t seglen;
 
 		if (!ctx->used) {
 			err = skcipher_wait_for_data(sk, flags);
 			if (err)
 				goto free;
 		}
-		sgl = list_first_entry(&ctx->tsgl,
-				       struct skcipher_sg_list, list);
-		sg = sgl->sg;
 
-		while (!sg->length)
-			sg++;
-
-		used = min_t(unsigned long, ctx->used,
-			     iov_iter_count(&msg->msg_iter));
-		used = min_t(unsigned long, used, sg->length);
-
-		if (txbufs == tx_nents) {
-			struct scatterlist *tmp;
-			int x;
-			/* Ran out of tx slots in async request
-			 * need to expand */
-			tmp = kcalloc(tx_nents * 2, sizeof(*tmp),
-				      GFP_KERNEL);
-			if (!tmp) {
-				err = -ENOMEM;
-				goto free;
-			}
+		seglen = min_t(size_t, ctx->used,
+			       iov_iter_count(&msg->msg_iter));
 
-			sg_init_table(tmp, tx_nents * 2);
-			for (x = 0; x < tx_nents; x++)
-				sg_set_page(&tmp[x], sg_page(&sreq->tsg[x]),
-					    sreq->tsg[x].length,
-					    sreq->tsg[x].offset);
-			kfree(sreq->tsg);
-			sreq->tsg = tmp;
-			tx_nents *= 2;
-			mark = true;
-		}
-		/* Need to take over the tx sgl from ctx
-		 * to the asynch req - these sgls will be freed later */
-		sg_set_page(sreq->tsg + txbufs++, sg_page(sg), sg->length,
-			    sg->offset);
-
-		if (list_empty(&sreq->list)) {
-			rsgl = &sreq->first_sgl;
-			list_add_tail(&rsgl->list, &sreq->list);
+		if (list_empty(&areq->rsgl_list)) {
+			rsgl = &areq->first_sgl;
 		} else {
-			rsgl = kmalloc(sizeof(*rsgl), GFP_KERNEL);
+			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
 			if (!rsgl) {
 				err = -ENOMEM;
 				goto free;
 			}
-			list_add_tail(&rsgl->list, &sreq->list);
 		}
 
-		used = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, used);
-		err = used;
-		if (used < 0)
+		rsgl->sgl.npages = 0;
+		list_add_tail(&rsgl->list, &areq->rsgl_list);
+
+		/* make one iovec available as scatterlist */
+		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
+		if (err < 0)
 			goto free;
+
+		/* chain the new scatterlist with previous one */
 		if (last_rsgl)
 			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
 
 		last_rsgl = rsgl;
-		len += used;
-		skcipher_pull_sgl(sk, used, 0);
-		iov_iter_advance(&msg->msg_iter, used);
+		len += err;
+		iov_iter_advance(&msg->msg_iter, err);
 	}
 
-	if (mark)
-		sg_mark_end(sreq->tsg + txbufs - 1);
+	/* Process only as much RX buffers for which we have TX data */
+	if (len > ctx->used)
+		len = ctx->used;
+
+	/*
+	 * If more buffers are to be expected to be processed, process only
+	 * full block size buffers.
+	 */
+	if (ctx->more || len < ctx->used)
+		len -= len % bs;
+
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
+	areq->tsgl_entries = skcipher_count_tsgl(sk, len);
+	if (!areq->tsgl_entries)
+		areq->tsgl_entries = 1;
+	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
+				  GFP_KERNEL);
+	if (!areq->tsgl) {
+		err = -ENOMEM;
+		goto free;
+	}
+	sg_init_table(areq->tsgl, areq->tsgl_entries);
+	skcipher_pull_tsgl(sk, len, areq->tsgl);
+
+	/* Initialize the crypto operation */
+	skcipher_request_set_tfm(&areq->req, tfm);
+	skcipher_request_set_crypt(&areq->req, areq->tsgl,
+				   areq->first_sgl.sgl.sg, len, ctx->iv);
+
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+		/* AIO operation */
+		areq->iocb = msg->msg_iocb;
+		skcipher_request_set_callback(&areq->req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP,
+					      skcipher_async_cb, areq);
+		err = ctx->enc ? crypto_skcipher_encrypt(&areq->req) :
+				 crypto_skcipher_decrypt(&areq->req);
+	} else {
+		/* Synchronous operation */
+		skcipher_request_set_callback(&areq->req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP |
+					      CRYPTO_TFM_REQ_MAY_BACKLOG,
+					      af_alg_complete,
+					      &ctx->completion);
+		err = af_alg_wait_for_completion(ctx->enc ?
+					crypto_skcipher_encrypt(&areq->req) :
+					crypto_skcipher_decrypt(&areq->req),
+						 &ctx->completion);
+	}
 
-	skcipher_request_set_crypt(req, sreq->tsg, sreq->first_sgl.sgl.sg,
-				   len, iv);
-	err = ctx->enc ? crypto_skcipher_encrypt(req) :
-			 crypto_skcipher_decrypt(req);
+	/* AIO operation in progress */
 	if (err == -EINPROGRESS) {
-		atomic_inc(&ctx->inflight);
+		sock_hold(sk);
 		err = -EIOCBQUEUED;
-		sreq = NULL;
+		ctx->inflight++;
 		goto unlock;
 	}
-free:
-	skcipher_free_async_sgls(sreq);
-unlock:
-	skcipher_wmem_wakeup(sk);
-	release_sock(sk);
-	kzfree(sreq);
-out:
-	return err;
-}
-
-static int skcipher_recvmsg_sync(struct socket *sock, struct msghdr *msg,
-				 int flags)
-{
-	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;
-	unsigned bs = crypto_skcipher_blocksize(tfm);
-	struct skcipher_sg_list *sgl;
-	struct scatterlist *sg;
-	int err = -EAGAIN;
-	int used;
-	long copied = 0;
-
-	lock_sock(sk);
-	while (msg_data_left(msg)) {
-		if (!ctx->used) {
-			err = skcipher_wait_for_data(sk, flags);
-			if (err)
-				goto unlock;
-		}
-
-		used = min_t(unsigned long, ctx->used, msg_data_left(msg));
-
-		used = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, used);
-		err = used;
-		if (err < 0)
-			goto unlock;
-
-		if (ctx->more || used < ctx->used)
-			used -= used % bs;
-
-		err = -EINVAL;
-		if (!used)
-			goto free;
-
-		sgl = list_first_entry(&ctx->tsgl,
-				       struct skcipher_sg_list, list);
-		sg = sgl->sg;
-
-		while (!sg->length)
-			sg++;
-
-		skcipher_request_set_crypt(&ctx->req, sg, ctx->rsgl.sg, used,
-					   ctx->iv);
-
-		err = af_alg_wait_for_completion(
-				ctx->enc ?
-					crypto_skcipher_encrypt(&ctx->req) :
-					crypto_skcipher_decrypt(&ctx->req),
-				&ctx->completion);
 
 free:
-		af_alg_free_sg(&ctx->rsgl);
-
-		if (err)
-			goto unlock;
-
-		copied += used;
-		skcipher_pull_sgl(sk, used, 1);
-		iov_iter_advance(&msg->msg_iter, used);
-	}
-
-	err = 0;
+	skcipher_free_areq_sgls(areq);
+	if (areq)
+		sock_kfree_s(sk, areq, areqlen);
 
 unlock:
 	skcipher_wmem_wakeup(sk);
 	release_sock(sk);
-
-	return copied ?: err;
-}
-
-static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
-			    size_t ignored, int flags)
-{
-	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
-		skcipher_recvmsg_async(sock, msg, flags) :
-		skcipher_recvmsg_sync(sock, msg, flags);
+	return err ? err : len;
 }
 
 static unsigned int skcipher_poll(struct file *file, struct socket *sock,
@@ -723,10 +697,9 @@ static unsigned int skcipher_poll(struct file *file, struct socket *sock,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	unsigned int mask;
+	unsigned int mask = 0;
 
 	sock_poll_wait(file, sk_sleep(sk), wait);
-	mask = 0;
 
 	if (ctx->used)
 		mask |= POLLIN | POLLRDNORM;
@@ -894,26 +867,20 @@ static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 	return err;
 }
 
-static void skcipher_wait(struct sock *sk)
-{
-	struct alg_sock *ask = alg_sk(sk);
-	struct skcipher_ctx *ctx = ask->private;
-	int ctr = 0;
-
-	while (atomic_read(&ctx->inflight) && ctr++ < 100)
-		msleep(100);
-}
-
 static void skcipher_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(&ctx->req);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
+	struct skcipher_tfm *skc = pask->private;
+	struct crypto_skcipher *tfm = skc->skcipher;
 
-	if (atomic_read(&ctx->inflight))
-		skcipher_wait(sk);
+	/* Suspend caller if AIO operations are in flight. */
+	wait_event_interruptible(skcipher_aio_finish_wait,
+				 (ctx->inflight == 0));
 
-	skcipher_free_sgl(sk);
+	skcipher_pull_tsgl(sk, ctx->used, NULL);
 	sock_kzfree_s(sk, ctx->iv, crypto_skcipher_ivsize(tfm));
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
@@ -925,7 +892,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_tfm *tfm = private;
 	struct crypto_skcipher *skcipher = tfm->skcipher;
-	unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(skcipher);
+	unsigned int len = sizeof(*ctx);
 
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
 	if (!ctx)
@@ -940,22 +907,17 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 
 	memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
 
-	INIT_LIST_HEAD(&ctx->tsgl);
+	INIT_LIST_HEAD(&ctx->tsgl_list);
 	ctx->len = len;
 	ctx->used = 0;
+	ctx->inflight = 0;
 	ctx->more = 0;
 	ctx->merge = 0;
 	ctx->enc = 0;
-	atomic_set(&ctx->inflight, 0);
 	af_alg_init_completion(&ctx->completion);
 
 	ask->private = ctx;
 
-	skcipher_request_set_tfm(&ctx->req, skcipher);
-	skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_SLEEP |
-						 CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      af_alg_complete, &ctx->completion);
-
 	sk->sk_destruct = skcipher_sock_destruct;
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 0/2] crypto: AF_ALG memory management fix
From: Stephan Müller @ 2017-02-17 22:31 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto

Hi Herbert,

Changes v5:
* use list_for_each_entry_safe in aead_count_tsgl

Changes v4:
* replace ctx->processed with a maintenance of a private recvmsg
  TX SGL
* algif_skcipher: rename skcipher_sg_list into skcipher_tsgl to make
  it consistent with algif_aead to prepare for a code consolidation

Changes v3:
* in *_pull_tsgl: make sure ctx->processed cannot be less than zero
* perform fuzzing of all input parameters with bogus values

Changes v2:
* import fix from Harsh Jain <harsh@chelsio.com> to remove SG
  from list before freeing
* fix return code used for ki_complete to match AIO behavior
  with sync behavior
* rename variable list -> tsgl_list
* update the algif_aead patch to include a dynamic TX SGL
  allocation similar to what algif_skcipher does. This allows
  concurrent continuous read/write operations to the extent
  you requested. Although I have not implemented "pairs of
  TX/RX SGLs" as I think that is even more overhead, the
  implementation conceptually defines such pairs. The recvmsg
  call defines how much from the input data is processed.
  The caller can have arbitrary number of sendmsg calls
  where the data is added to the TX SGL before an recvmsg
  asks the kernel to process a given amount (or all) of the
  TX SGL.

With the changes, you will see a lot of code duplication now
as I deliberately tried to use the same struct and variable names,
the same function names and even the same oder of functions.
If you agree to this patch, I volunteer to provide a followup
patch that will extract the code duplication into common
functions.

Please find attached memory management updates to

- simplify the code: the old AIO memory management is very
  complex and seemingly very fragile -- the update now
  eliminates all reported bugs in the skcipher and AEAD
  interfaces which allowed the kernel to be crashed by
  an unprivileged user

- streamline the code: there is one code path for AIO and sync
  operation; the code between algif_skcipher and algif_aead
  is very similar (if that patch set is accepted, I volunteer
  to reduce code duplication by moving service operations
  into af_alg.c and to further unify the TX SGL handling)

- unify the AIO and sync operation which only differ in the
  kernel crypto API callback and whether to wait for the
  crypto operation or not

- fix all reported bugs regarding the handling of multiple
  IOCBs.

The following testing was performed:

- stress testing to verify that no memleaks exist

- testing using Tadeusz Struck AIO test tool (see
  https://github.com/tstruk/afalg_async_test) -- the AEAD test
  is not applicable any more due to the changed user space
  interface; the skcipher test works once the user space
  interface change is honored in the test code

- using the libkcapi test suite, all tests including the
  originally failing ones (AIO with multiple IOCBs) work now --
  the current libkcapi code artificially limits the AEAD
  operation to one IOCB. After altering the libkcapi code
  to allow multiple IOCBs, the testing works flawless.

Stephan Mueller (2):
  crypto: skcipher AF_ALG - overhaul memory management
  crypto: aead AF_ALG - overhaul memory management

 crypto/algif_aead.c     | 702 ++++++++++++++++++++++++++----------------------
 crypto/algif_skcipher.c | 540 +++++++++++++++++--------------------
 2 files changed, 637 insertions(+), 605 deletions(-)

-- 
2.9.3

^ permalink raw reply

* Re: BUG: af_alg bind fails for 50 % request from userspace for hash algo
From: Stephan Müller @ 2017-02-17 22:25 UTC (permalink / raw)
  To: Harsh Jain; +Cc: Herbert Xu, linux-crypto
In-Reply-To: <4fae9b51-ce64-f380-def3-f91f7bffc244@chelsio.com>

Am Mittwoch, 15. Februar 2017, 22:12:16 CET schrieb Harsh Jain:

Hi Harsh,

> Hi Herbert/Stephen,
> 
> When I try to run 100 application which calculates sha384 digest from
> userspace, nearly 50 applications fail in bind system call with error
> ENOENT.
> 
> "crypto_alg_mod_lookup" in api.c call fails in kernel space. Issue comes in
> 1st try only(Seems some relation with crypto test executions).  If I
> execute same test again issue didn't reproduce.

I can concur that once in a while I see an ENOENT too for hashes. Let me try 
to investigate that.
> 
> Regards
> 
> Harsh Jain


Ciao
Stephan

^ permalink raw reply

* crypto: hang in crypto_larval_lookup
From: Harald Freudenberger @ 2017-02-17 17:50 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, Martin Schwidefsky


Hello all

I am currently following a hang at modprobe aes_s390 where
crypto_register_alg() does not come back for the xts(aes) algorithm.

The registration is waiting forever in algapi.c crypto_wait_for_test() but
the completion never occurs. The cryptomgr is triggering a test via
kthread_run to invoce cryptomgr_probe and this thread is calling the
create() function of the xts template (file xts.c). Following this thread
it comes down to api.c crypto_larval_lookup(name="aes") which is first
requesting the module "crypto-aes" via request_module() successful and then
blocking forever in requesting the module "crypto-aes-all".

The xts(aes) has at registration CRYPTO_ALG_NEED_FALLBACK flag on.

This problem is seen since about 6 weeks now, first only on the linux next
kernel. Now it appers on the 4.10-rc kernels as well. And I still have no
idea on how this could be fixed or what's wrong with just the xts
registration (ecb, cbc, ctr work fine).

Any ideas or hints?
Thank's in advance.

regards
Harald Freudenberger

^ permalink raw reply

* Re: [PATCH v4 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-17 19:30 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <3126581.xcUJNzAaop@positron.chronox.de>

Am Donnerstag, 16. Februar 2017, 22:21:44 CET schrieb Stephan Müller:

Hi,

> +static unsigned int skcipher_count_tsgl(struct sock *sk, size_t bytes)
>  {
>  	struct alg_sock *ask = alg_sk(sk);
>  	struct skcipher_ctx *ctx = ask->private;
> -	struct skcipher_sg_list *sgl;
> +	struct skcipher_tsgl *sgl;
>  	struct scatterlist *sg;
> -	int i;
> +	unsigned int i;
> +	unsigned int sgl_count = 0;
> +
> +	if (!bytes)
> +		return 0;
> +
> +	while (!list_empty(&ctx->tsgl_list)) {

This is wrong, it should be list_for_each_entry_safe(sgl, tmp, &ctx-
>tsgl_list, list) {

I will send a new patch shortly.

Ciao
Stephan

^ permalink raw reply

* [PATCH] crypto: cavium: fix leak on curr if curr->head fails to be allocated
From: Colin King @ 2017-02-17 15:57 UTC (permalink / raw)
  To: George Cherian, Herbert Xu, David S . Miller, linux-crypto
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The exit path when curr->head cannot be allocated fails to kfree the
earlier allocated curr.  Fix this by kfree'ing it.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/crypto/cavium/cpt/cptvf_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/cavium/cpt/cptvf_main.c b/drivers/crypto/cavium/cpt/cptvf_main.c
index 527bdc3..3247c7f 100644
--- a/drivers/crypto/cavium/cpt/cptvf_main.c
+++ b/drivers/crypto/cavium/cpt/cptvf_main.c
@@ -242,6 +242,7 @@ static int alloc_command_queues(struct cpt_vf *cptvf,
 			if (!curr->head) {
 				dev_err(&pdev->dev, "Command Q (%d) chunk (%d) allocation failed\n",
 					i, queue->nchunks);
+				kfree(curr);
 				goto cmd_qfail;
 			}
 
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH v2] dm: switch dm-verity to async hash crypto API
From: Gilad Ben-Yossef @ 2017-02-17 14:52 UTC (permalink / raw)
  To: Milan Broz
  Cc: dm-devel, Alasdair Kergon, Mike Snitzer, linux-crypto,
	gilad.benyossef, Ofir Drang, Eric Biggers,
	Ondrej Mosnáček
In-Reply-To: <74bc1a00-6fb4-4b2f-b57b-a7fea0969eeb@gmail.com>

Hi Milan,

Thank you for the review and testing.

On Fri, Feb 17, 2017 at 3:00 PM, Milan Broz <gmazyland@gmail.com> wrote:
> On 02/06/2017 02:58 PM, Gilad Ben-Yossef wrote:
>> Use of the synchronous digest API limits dm-verity to using pure
>> CPU based algorithm providers and rules out the use of off CPU
>> algorithm providers which are normally asynchronous by nature,
>> potentially freeing CPU cycles.
>>
>> This can reduce performance per Watt in situations such as during
>> boot time when a lot of concurrent file accesses are made to the
>> protected volume.
>>
>> Move DM_VERITY to the asynchronous hash API.
>
> Did you test that async hash completion path?

Yes, I did - with the Arm TrustZone CryptoCell hardware accelerator.
I did not try with cryptd though.
>
> I just tried to force async crypto by replacing "sha256"
> in mapping table with "cryptd(sha256-generic)" and it kills
> kernel immediately.
> https://mbroz.fedorapeople.org/tmp/verity-fail.png
>
> (I hope this trick should cause to use cryptd and use async processing.
> In previous version the parameter is properly rejected, because unsupported
> by shash api.)
>

Thanks for this trick. I was not aware you can invoke cryptd it like that.

I will recreate the issue and fix it.

>
> Some more comments below.
> ...
>
>> -static int verity_hash_update(struct dm_verity *v, struct shash_desc *desc,
>> -                           const u8 *data, size_t len)
>> +static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
>> +                             const u8 *data, size_t len,
>> +                             struct verity_result *res)
>>  {
>> -     int r = crypto_shash_update(desc, data, len);
>> +     struct scatterlist sg;
>>
>> -     if (unlikely(r < 0))
>> -             DMERR("crypto_shash_update failed: %d", r);
>> +     sg_init_table(&sg, 1);
>> +     sg_set_buf(&sg, data, len);
>
> why not use sg_init_one?

No good reason. I will amend it in the next revision.

>
>> +     ahash_request_set_crypt(req, &sg, NULL, len);
>> +
>> +     return verity_complete_op(res, crypto_ahash_update(req));
>> +}
>
> ...
>
>> -int verity_hash(struct dm_verity *v, struct shash_desc *desc,
>> +int verity_hash(struct dm_verity *v, struct ahash_request *req,
>>               const u8 *data, size_t len, u8 *digest)
>>  {
>>       int r;
>> +     struct verity_result res;
>>
>> -     r = verity_hash_init(v, desc);
>> +     r = verity_hash_init(v, req, &res);
>>       if (unlikely(r < 0))
>> -             return r;
>> +             goto out;
>
> why it is changed to goto? it doesn't simplify anything in this function
>

I generally prefer for a function to have a single return point, if it does
not over complicates things. I find it makes code more readable and easier
to reason about - put debugging log statement for return for example.

>>
>> -     r = verity_hash_update(v, desc, data, len);
>> +     r = verity_hash_update(v, req, data, len, &res);
>>       if (unlikely(r < 0))
>> -             return r;
>> +             goto out;
>> +
>> +     r = verity_hash_final(v, req, digest, &res);
>>
>> -     return verity_hash_final(v, desc, digest);
>> +out:
>> +     return r;
>>  }

I will post a new revision of the patch early next week  .

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

^ permalink raw reply

* Re: [PATCH v2] dm: switch dm-verity to async hash crypto API
From: Gilad Ben-Yossef @ 2017-02-17 14:47 UTC (permalink / raw)
  To: Milan Broz
  Cc: Mike Snitzer, gilad.benyossef, Eric Biggers, dm-devel,
	linux-crypto, Ondrej Mosnáček, Alasdair Kergon,
	Ofir Drang
In-Reply-To: <74bc1a00-6fb4-4b2f-b57b-a7fea0969eeb@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 3492 bytes --]

Hi Milan,

Thank you for the review and testing.

On Fri, Feb 17, 2017 at 3:00 PM, Milan Broz <gmazyland@gmail.com> wrote:

> On 02/06/2017 02:58 PM, Gilad Ben-Yossef wrote:
> > Use of the synchronous digest API limits dm-verity to using pure
> > CPU based algorithm providers and rules out the use of off CPU
> > algorithm providers which are normally asynchronous by nature,
> > potentially freeing CPU cycles.
> >
> > This can reduce performance per Watt in situations such as during
> > boot time when a lot of concurrent file accesses are made to the
> > protected volume.
> >
> > Move DM_VERITY to the asynchronous hash API.
>
> Did you test that async hash completion path?
>
> Yes, I did - with the Arm TrustZone CryptoCell hardware accelerator.
I did not try with cryptd though.



> I just tried to force async crypto by replacing "sha256"
> in mapping table with "cryptd(sha256-generic)" and it kills
> kernel immediately.
> https://mbroz.fedorapeople.org/tmp/verity-fail.png
>
> (I hope this trick should cause to use cryptd and use async processing.
> In previous version the parameter is properly rejected, because unsupported
> by shash api.)
>
>
>
Thanks for this trick. I was not aware you can invoke cryptd it like that.

I will recreate the issue and fix it.

Some more comments below.
> ...
>
> > -static int verity_hash_update(struct dm_verity *v, struct shash_desc
> *desc,
> > -                           const u8 *data, size_t len)
> > +static int verity_hash_update(struct dm_verity *v, struct ahash_request
> *req,
> > +                             const u8 *data, size_t len,
> > +                             struct verity_result *res)
> >  {
> > -     int r = crypto_shash_update(desc, data, len);
> > +     struct scatterlist sg;
> >
> > -     if (unlikely(r < 0))
> > -             DMERR("crypto_shash_update failed: %d", r);
> > +     sg_init_table(&sg, 1);
> > +     sg_set_buf(&sg, data, len);
>
> No good reason. I will amend it in the next revision.
>



> +     ahash_request_set_crypt(req, &sg, NULL, len);
> > +
> > +     return verity_complete_op(res, crypto_ahash_update(req));
> > +}
>
> ...
>
> > -int verity_hash(struct dm_verity *v, struct shash_desc *desc,
> > +int verity_hash(struct dm_verity *v, struct ahash_request *req,
> >               const u8 *data, size_t len, u8 *digest)
> >  {
> >       int r;
> > +     struct verity_result res;
> >
> > -     r = verity_hash_init(v, desc);
> > +     r = verity_hash_init(v, req, &res);
> >       if (unlikely(r < 0))
> > -             return r;
> > +             goto out;
>
> why it is changed to goto? it doesn't simplify anything in this function
>
>
I generally prefer for a function to have a single return point, if it does
not over complicates things. I find it makes code more readable and easier
to reason about - put debugging log statement for return for example.



> >
> > -     r = verity_hash_update(v, desc, data, len);
> > +     r = verity_hash_update(v, req, data, len, &res);
> >       if (unlikely(r < 0))
> > -             return r;
> > +             goto out;
> > +
> > +     r = verity_hash_final(v, req, digest, &res);
> >
> > -     return verity_hash_final(v, desc, digest);
> > +out:
> > +     return r;
> >  }
>
>

I will post a new revision of the patch early next week  .

Thanks,
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a situation
where the homework eats your dog?"
 -- Jean-Baptiste Queru

[-- Attachment #1.2: Type: text/html, Size: 5641 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [PATCH v2] dm: switch dm-verity to async hash crypto API
From: Milan Broz @ 2017-02-17 13:00 UTC (permalink / raw)
  To: Gilad Ben-Yossef, dm-devel, Alasdair Kergon, Mike Snitzer
  Cc: linux-crypto, gilad.benyossef, ofir.drang, Eric Biggers,
	Ondrej Mosnáček
In-Reply-To: <1486389482-26992-1-git-send-email-gilad@benyossef.com>

On 02/06/2017 02:58 PM, Gilad Ben-Yossef wrote:
> Use of the synchronous digest API limits dm-verity to using pure
> CPU based algorithm providers and rules out the use of off CPU
> algorithm providers which are normally asynchronous by nature,
> potentially freeing CPU cycles.
> 
> This can reduce performance per Watt in situations such as during
> boot time when a lot of concurrent file accesses are made to the
> protected volume.
> 
> Move DM_VERITY to the asynchronous hash API.

Did you test that async hash completion path?

I just tried to force async crypto by replacing "sha256"
in mapping table with "cryptd(sha256-generic)" and it kills
kernel immediately.
https://mbroz.fedorapeople.org/tmp/verity-fail.png

(I hope this trick should cause to use cryptd and use async processing.
In previous version the parameter is properly rejected, because unsupported
by shash api.)


Some more comments below.
...

> -static int verity_hash_update(struct dm_verity *v, struct shash_desc *desc,
> -			      const u8 *data, size_t len)
> +static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
> +				const u8 *data, size_t len,
> +				struct verity_result *res)
>  {
> -	int r = crypto_shash_update(desc, data, len);
> +	struct scatterlist sg;
>  
> -	if (unlikely(r < 0))
> -		DMERR("crypto_shash_update failed: %d", r);
> +	sg_init_table(&sg, 1);
> +	sg_set_buf(&sg, data, len);

why not use sg_init_one?

> +	ahash_request_set_crypt(req, &sg, NULL, len);
> +
> +	return verity_complete_op(res, crypto_ahash_update(req));
> +}

...

> -int verity_hash(struct dm_verity *v, struct shash_desc *desc,
> +int verity_hash(struct dm_verity *v, struct ahash_request *req,
>  		const u8 *data, size_t len, u8 *digest)
>  {
>  	int r;
> +	struct verity_result res;
>  
> -	r = verity_hash_init(v, desc);
> +	r = verity_hash_init(v, req, &res);
>  	if (unlikely(r < 0))
> -		return r;
> +		goto out;

why it is changed to goto? it doesn't simplify anything in this function

>  
> -	r = verity_hash_update(v, desc, data, len);
> +	r = verity_hash_update(v, req, data, len, &res);
>  	if (unlikely(r < 0))
> -		return r;
> +		goto out;
> +
> +	r = verity_hash_final(v, req, digest, &res);
>  
> -	return verity_hash_final(v, desc, digest);
> +out:
> +	return r;
>  }


Milan

^ permalink raw reply

* Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn
From: Geert Uytterhoeven @ 2017-02-17 12:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: open list:FRAMEBUFFER LAYER, linux-ia64@vger.kernel.org,
	Linux-sh list, Alexander Shishkin, nouveau,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	dri-devel, virtualization, linux-ide@vger.kernel.org,
	MTD Maling List, sparclinux, Lars Ellenberg,
	open list:TARGET SUBSYSTEM, Richard Weinberger, sfi-devel,
	amd-gfx, ACPI Devel Maling List, tboot-devel, oprofil
In-Reply-To: <CAJZ5v0gNcGtnXpE1n6LSq7ey+UDKx+oQfmTg0_PgFwzB0Dd_5A@mail.gmail.com>

Hi Rafael,

On Fri, Feb 17, 2017 at 1:27 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Feb 17, 2017 at 8:11 AM, Joe Perches <joe@perches.com> wrote:
>> There are ~4300 uses of pr_warn and ~250 uses of the older
>> pr_warning in the kernel source tree.
>>
>> Make the use of pr_warn consistent across all kernel files.
>>
>> This excludes all files in tools/ as there is a separate
>> define pr_warning for that directory tree and pr_warn is
>> not used in tools/.
>>
>> Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.
>
> Sorry about asking if that has been asked already.
>
> Wouldn't it be slightly less intrusive to simply redefined
> pr_warning() as a synonym for pr_warn()?

That's already the case.

This series cleans up the cruft, so we can catch all users with
"git grep -w pr_warn".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn
From: Rafael J. Wysocki @ 2017-02-17 12:27 UTC (permalink / raw)
  To: Joe Perches
  Cc: Alexander Shishkin, Karol Herbst, Pekka Paalanen,
	Richard Weinberger, Fabio Estevam, Linux Kernel Mailing List,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linuxppc-dev, tboot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	oprofile-list-TtF/mJH4Jtrk1uMJSBkQmQ,
	sfi-devel-yLnuTTp1/kvcsJTPyzm5gB2eb7JE58TQ,
	xen-devel-GuqFBffKawtpuQazS67q72D2FQJk+8+b,
	ACPI Devel Maling List, drbd-dev-cunTk1MwBs8qoQakbn7OcQ,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	gigaset307x-common-5NWGOfrQmneRv+LV9MX5uv+2+P5yyue3
In-Reply-To: <cover.1487314666.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

On Fri, Feb 17, 2017 at 8:11 AM, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
> There are ~4300 uses of pr_warn and ~250 uses of the older
> pr_warning in the kernel source tree.
>
> Make the use of pr_warn consistent across all kernel files.
>
> This excludes all files in tools/ as there is a separate
> define pr_warning for that directory tree and pr_warn is
> not used in tools/.
>
> Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Sorry about asking if that has been asked already.

Wouldn't it be slightly less intrusive to simply redefined
pr_warning() as a synonym for pr_warn()?

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 08/10] x86: assembly, annotate aliases
From: Juergen Gross @ 2017-02-17 11:52 UTC (permalink / raw)
  To: Jiri Slaby, mingo
  Cc: tglx, hpa, x86, jpoimboe, linux-kernel, Herbert Xu,
	David S. Miller, Boris Ostrovsky, linux-crypto, xen-devel
In-Reply-To: <20170217104757.28588-8-jslaby@suse.cz>

On 17/02/17 11:47, Jiri Slaby wrote:
> _key_expansion_128 is an alias to _key_expansion_256a, __memcpy to
> memcpy, xen_syscall32_target to xen_sysenter_target, and so on. Annotate
> them all using the new ENTRY_ALIAS and ENTRY_LOCAL_ALIAS. This will make
> the tools generating the debuginfo happy.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: <linux-crypto@vger.kernel.org>
> Cc: <xen-devel@lists.xenproject.org>
> ---
>  arch/x86/crypto/aesni-intel_asm.S | 5 ++---
>  arch/x86/lib/memcpy_64.S          | 4 ++--
>  arch/x86/lib/memmove_64.S         | 4 ++--
>  arch/x86/lib/memset_64.S          | 4 ++--
>  arch/x86/xen/xen-asm_64.S         | 4 ++--
>  5 files changed, 10 insertions(+), 11 deletions(-)

Xen parts:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

^ permalink raw reply

* [PATCH 08/10] x86: assembly, annotate aliases
From: Jiri Slaby @ 2017-02-17 10:47 UTC (permalink / raw)
  To: mingo
  Cc: Juergen Gross, hpa, Herbert Xu, Boris Ostrovsky, x86,
	linux-kernel, linux-crypto, jpoimboe, xen-devel, tglx, Jiri Slaby,
	David S. Miller
In-Reply-To: <20170217104757.28588-1-jslaby@suse.cz>

_key_expansion_128 is an alias to _key_expansion_256a, __memcpy to
memcpy, xen_syscall32_target to xen_sysenter_target, and so on. Annotate
them all using the new ENTRY_ALIAS and ENTRY_LOCAL_ALIAS. This will make
the tools generating the debuginfo happy.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: <linux-crypto@vger.kernel.org>
Cc: <xen-devel@lists.xenproject.org>
---
 arch/x86/crypto/aesni-intel_asm.S | 5 ++---
 arch/x86/lib/memcpy_64.S          | 4 ++--
 arch/x86/lib/memmove_64.S         | 4 ++--
 arch/x86/lib/memset_64.S          | 4 ++--
 arch/x86/xen/xen-asm_64.S         | 4 ++--
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
index 624e4303d0fb..6a0f25be1a56 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -1744,8 +1744,7 @@ ENDPROC(aesni_gcm_enc)
 #endif
 
 
-.align 4
-_key_expansion_128:
+ENTRY_LOCAL_ALIAS(_key_expansion_128)
 ENTRY_LOCAL(_key_expansion_256a)
 	pshufd $0b11111111, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
@@ -1756,8 +1755,8 @@ ENTRY_LOCAL(_key_expansion_256a)
 	movaps %xmm0, (TKEYP)
 	add $0x10, TKEYP
 	ret
-ENDPROC(_key_expansion_128)
 ENDPROC(_key_expansion_256a)
+END_ALIAS(_key_expansion_128)
 
 ENTRY_LOCAL(_key_expansion_192a)
 	pshufd $0b01010101, %xmm1, %xmm1
diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index 779782f58324..c9ac54822e87 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -26,7 +26,7 @@
  * Output:
  * rax original destination
  */
-ENTRY(__memcpy)
+ENTRY_ALIAS(__memcpy)
 ENTRY(memcpy)
 	ALTERNATIVE_2 "jmp memcpy_orig", "", X86_FEATURE_REP_GOOD, \
 		      "jmp memcpy_erms", X86_FEATURE_ERMS
@@ -40,7 +40,7 @@ ENTRY(memcpy)
 	rep movsb
 	ret
 ENDPROC(memcpy)
-ENDPROC(__memcpy)
+END_ALIAS(__memcpy)
 EXPORT_SYMBOL(memcpy)
 EXPORT_SYMBOL(__memcpy)
 
diff --git a/arch/x86/lib/memmove_64.S b/arch/x86/lib/memmove_64.S
index 15de86cd15b0..76f54ba3dd26 100644
--- a/arch/x86/lib/memmove_64.S
+++ b/arch/x86/lib/memmove_64.S
@@ -25,7 +25,7 @@
  */
 .weak memmove
 
-ENTRY(memmove)
+ENTRY_ALIAS(memmove)
 ENTRY(__memmove)
 
 	/* Handle more 32 bytes in loop */
@@ -207,6 +207,6 @@ ENTRY(__memmove)
 13:
 	retq
 ENDPROC(__memmove)
-ENDPROC(memmove)
+END_ALIAS(memmove)
 EXPORT_SYMBOL(__memmove)
 EXPORT_SYMBOL(memmove)
diff --git a/arch/x86/lib/memset_64.S b/arch/x86/lib/memset_64.S
index 55b95db30a61..be6c4705ec51 100644
--- a/arch/x86/lib/memset_64.S
+++ b/arch/x86/lib/memset_64.S
@@ -18,7 +18,7 @@
  *
  * rax   original destination
  */
-ENTRY(memset)
+ENTRY_ALIAS(memset)
 ENTRY(__memset)
 	/*
 	 * Some CPUs support enhanced REP MOVSB/STOSB feature. It is recommended
@@ -42,8 +42,8 @@ ENTRY(__memset)
 	rep stosb
 	movq %r9,%rax
 	ret
-ENDPROC(memset)
 ENDPROC(__memset)
+END_ALIAS(memset)
 EXPORT_SYMBOL(memset)
 EXPORT_SYMBOL(__memset)
 
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index d617bea76039..4b0fe749f10c 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -117,13 +117,13 @@ ENDPROC(xen_sysenter_target)
 
 #else /* !CONFIG_IA32_EMULATION */
 
-ENTRY(xen_syscall32_target)
+ENTRY_ALIAS(xen_syscall32_target)
 ENTRY(xen_sysenter_target)
 	lea 16(%rsp), %rsp	/* strip %rcx, %r11 */
 	mov $-ENOSYS, %rax
 	pushq $0
 	jmp hypercall_iret
-ENDPROC(xen_syscall32_target)
 ENDPROC(xen_sysenter_target)
+END_ALIAS(xen_syscall32_target)
 
 #endif	/* CONFIG_IA32_EMULATION */
-- 
2.11.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related

* [PATCH 06/10] x86: crypto, annotate local functions
From: Jiri Slaby @ 2017-02-17 10:47 UTC (permalink / raw)
  To: mingo
  Cc: tglx, hpa, x86, jpoimboe, linux-kernel, Jiri Slaby, Herbert Xu,
	David S. Miller, linux-crypto
In-Reply-To: <20170217104757.28588-1-jslaby@suse.cz>

Use the newly added ENTRY_LOCAL to annotate starts of all functions
which do not have ".globl" annotation, but their ends are annotated by
ENDPROC. This is needed to balance ENDPROC for tools that are about to
generate debuginfo.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: <linux-crypto@vger.kernel.org>
---
 arch/x86/crypto/aesni-intel_asm.S            | 29 ++++++++++------------------
 arch/x86/crypto/camellia-aesni-avx-asm_64.S  | 10 +++++-----
 arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 10 +++++-----
 arch/x86/crypto/cast5-avx-x86_64-asm_64.S    |  4 ++--
 arch/x86/crypto/cast6-avx-x86_64-asm_64.S    |  4 ++--
 arch/x86/crypto/ghash-clmulni-intel_asm.S    |  2 +-
 arch/x86/crypto/serpent-avx-x86_64-asm_64.S  |  4 ++--
 arch/x86/crypto/serpent-avx2-asm_64.S        |  4 ++--
 arch/x86/crypto/twofish-avx-x86_64-asm_64.S  |  4 ++--
 9 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
index 3c465184ff8a..624e4303d0fb 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -1746,7 +1746,7 @@ ENDPROC(aesni_gcm_enc)
 
 .align 4
 _key_expansion_128:
-_key_expansion_256a:
+ENTRY_LOCAL(_key_expansion_256a)
 	pshufd $0b11111111, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
 	pxor %xmm4, %xmm0
@@ -1759,8 +1759,7 @@ _key_expansion_256a:
 ENDPROC(_key_expansion_128)
 ENDPROC(_key_expansion_256a)
 
-.align 4
-_key_expansion_192a:
+ENTRY_LOCAL(_key_expansion_192a)
 	pshufd $0b01010101, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
 	pxor %xmm4, %xmm0
@@ -1784,8 +1783,7 @@ _key_expansion_192a:
 	ret
 ENDPROC(_key_expansion_192a)
 
-.align 4
-_key_expansion_192b:
+ENTRY_LOCAL(_key_expansion_192b)
 	pshufd $0b01010101, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
 	pxor %xmm4, %xmm0
@@ -1804,8 +1802,7 @@ _key_expansion_192b:
 	ret
 ENDPROC(_key_expansion_192b)
 
-.align 4
-_key_expansion_256b:
+ENTRY_LOCAL(_key_expansion_256b)
 	pshufd $0b10101010, %xmm1, %xmm1
 	shufps $0b00010000, %xmm2, %xmm4
 	pxor %xmm4, %xmm2
@@ -1968,8 +1965,7 @@ ENDPROC(aesni_enc)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_enc1:
+ENTRY_LOCAL(_aesni_enc1)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE		# round 0
@@ -2032,8 +2028,7 @@ ENDPROC(_aesni_enc1)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_enc4:
+ENTRY_LOCAL(_aesni_enc4)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE1		# round 0
@@ -2160,8 +2155,7 @@ ENDPROC(aesni_dec)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_dec1:
+ENTRY_LOCAL(_aesni_dec1)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE		# round 0
@@ -2224,8 +2218,7 @@ ENDPROC(_aesni_dec1)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_dec4:
+ENTRY_LOCAL(_aesni_dec4)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE1		# round 0
@@ -2591,8 +2584,7 @@ ENDPROC(aesni_cbc_dec)
  *	INC:	== 1, in little endian
  *	BSWAP_MASK == endian swapping mask
  */
-.align 4
-_aesni_inc_init:
+ENTRY_LOCAL(_aesni_inc_init)
 	movaps .Lbswap_mask, BSWAP_MASK
 	movaps IV, CTR
 	PSHUFB_XMM BSWAP_MASK CTR
@@ -2617,8 +2609,7 @@ ENDPROC(_aesni_inc_init)
  *	CTR:	== output IV, in little endian
  *	TCTR_LOW: == lower qword of CTR
  */
-.align 4
-_aesni_inc:
+ENTRY_LOCAL(_aesni_inc)
 	paddq INC, CTR
 	add $1, TCTR_LOW
 	jnc .Linc_low
diff --git a/arch/x86/crypto/camellia-aesni-avx-asm_64.S b/arch/x86/crypto/camellia-aesni-avx-asm_64.S
index f7c495e2863c..f203607da5d0 100644
--- a/arch/x86/crypto/camellia-aesni-avx-asm_64.S
+++ b/arch/x86/crypto/camellia-aesni-avx-asm_64.S
@@ -188,7 +188,7 @@
  * larger and would only be 0.5% faster (on sandy-bridge).
  */
 .align 8
-roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
+ENTRY_LOCAL(roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 	roundsm16(%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		  %xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm15,
 		  %rcx, (%r9));
@@ -196,7 +196,7 @@ roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
 ENDPROC(roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 
 .align 8
-roundsm16_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab:
+ENTRY_LOCAL(roundsm16_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 	roundsm16(%xmm4, %xmm5, %xmm6, %xmm7, %xmm0, %xmm1, %xmm2, %xmm3,
 		  %xmm12, %xmm13, %xmm14, %xmm15, %xmm8, %xmm9, %xmm10, %xmm11,
 		  %rax, (%r9));
@@ -721,7 +721,7 @@ ENDPROC(roundsm16_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 .text
 
 .align 8
-__camellia_enc_blk16:
+ENTRY_LOCAL(__camellia_enc_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 256 bytes
@@ -808,7 +808,7 @@ __camellia_enc_blk16:
 ENDPROC(__camellia_enc_blk16)
 
 .align 8
-__camellia_dec_blk16:
+ENTRY_LOCAL(__camellia_dec_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 256 bytes
@@ -1119,7 +1119,7 @@ ENDPROC(camellia_ctr_16way)
 	vpxor tmp, iv, iv;
 
 .align 8
-camellia_xts_crypt_16way:
+ENTRY_LOCAL(camellia_xts_crypt_16way)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rsi: dst (16 blocks)
diff --git a/arch/x86/crypto/camellia-aesni-avx2-asm_64.S b/arch/x86/crypto/camellia-aesni-avx2-asm_64.S
index eee5b3982cfd..e7aaf121db74 100644
--- a/arch/x86/crypto/camellia-aesni-avx2-asm_64.S
+++ b/arch/x86/crypto/camellia-aesni-avx2-asm_64.S
@@ -227,7 +227,7 @@
  * larger and would only marginally faster.
  */
 .align 8
-roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
+ENTRY_LOCAL(roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 	roundsm32(%ymm0, %ymm1, %ymm2, %ymm3, %ymm4, %ymm5, %ymm6, %ymm7,
 		  %ymm8, %ymm9, %ymm10, %ymm11, %ymm12, %ymm13, %ymm14, %ymm15,
 		  %rcx, (%r9));
@@ -235,7 +235,7 @@ roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
 ENDPROC(roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 
 .align 8
-roundsm32_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab:
+ENTRY_LOCAL(roundsm32_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 	roundsm32(%ymm4, %ymm5, %ymm6, %ymm7, %ymm0, %ymm1, %ymm2, %ymm3,
 		  %ymm12, %ymm13, %ymm14, %ymm15, %ymm8, %ymm9, %ymm10, %ymm11,
 		  %rax, (%r9));
@@ -764,7 +764,7 @@ ENDPROC(roundsm32_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 .text
 
 .align 8
-__camellia_enc_blk32:
+ENTRY_LOCAL(__camellia_enc_blk32)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 512 bytes
@@ -851,7 +851,7 @@ __camellia_enc_blk32:
 ENDPROC(__camellia_enc_blk32)
 
 .align 8
-__camellia_dec_blk32:
+ENTRY_LOCAL(__camellia_dec_blk32)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 512 bytes
@@ -1226,7 +1226,7 @@ ENDPROC(camellia_ctr_32way)
 	vpxor tmp1, iv, iv;
 
 .align 8
-camellia_xts_crypt_32way:
+ENTRY_LOCAL(camellia_xts_crypt_32way)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rsi: dst (32 blocks)
diff --git a/arch/x86/crypto/cast5-avx-x86_64-asm_64.S b/arch/x86/crypto/cast5-avx-x86_64-asm_64.S
index b4a8806234ea..76b5ca62c84d 100644
--- a/arch/x86/crypto/cast5-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/cast5-avx-x86_64-asm_64.S
@@ -224,7 +224,7 @@
 .text
 
 .align 16
-__cast5_enc_blk16:
+ENTRY_LOCAL(__cast5_enc_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RL1: blocks 1 and 2
@@ -296,7 +296,7 @@ __cast5_enc_blk16:
 ENDPROC(__cast5_enc_blk16)
 
 .align 16
-__cast5_dec_blk16:
+ENTRY_LOCAL(__cast5_dec_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RL1: encrypted blocks 1 and 2
diff --git a/arch/x86/crypto/cast6-avx-x86_64-asm_64.S b/arch/x86/crypto/cast6-avx-x86_64-asm_64.S
index 952d3156a933..d5eca2c0c4f6 100644
--- a/arch/x86/crypto/cast6-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/cast6-avx-x86_64-asm_64.S
@@ -262,7 +262,7 @@
 .text
 
 .align 8
-__cast6_enc_blk8:
+ENTRY_LOCAL(__cast6_enc_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: blocks
@@ -308,7 +308,7 @@ __cast6_enc_blk8:
 ENDPROC(__cast6_enc_blk8)
 
 .align 8
-__cast6_dec_blk8:
+ENTRY_LOCAL(__cast6_dec_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: encrypted blocks
diff --git a/arch/x86/crypto/ghash-clmulni-intel_asm.S b/arch/x86/crypto/ghash-clmulni-intel_asm.S
index f94375a8dcd1..5a63c73463b3 100644
--- a/arch/x86/crypto/ghash-clmulni-intel_asm.S
+++ b/arch/x86/crypto/ghash-clmulni-intel_asm.S
@@ -47,7 +47,7 @@
  *	T2
  *	T3
  */
-__clmul_gf128mul_ble:
+ENTRY_LOCAL(__clmul_gf128mul_ble)
 	movaps DATA, T1
 	pshufd $0b01001110, DATA, T2
 	pshufd $0b01001110, SHASH, T3
diff --git a/arch/x86/crypto/serpent-avx-x86_64-asm_64.S b/arch/x86/crypto/serpent-avx-x86_64-asm_64.S
index 2925077f8c6a..856fe0ccb262 100644
--- a/arch/x86/crypto/serpent-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/serpent-avx-x86_64-asm_64.S
@@ -570,7 +570,7 @@
 	transpose_4x4(x0, x1, x2, x3, t0, t1, t2)
 
 .align 8
-__serpent_enc_blk8_avx:
+ENTRY_LOCAL(__serpent_enc_blk8_avx)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: blocks
@@ -624,7 +624,7 @@ __serpent_enc_blk8_avx:
 ENDPROC(__serpent_enc_blk8_avx)
 
 .align 8
-__serpent_dec_blk8_avx:
+ENTRY_LOCAL(__serpent_dec_blk8_avx)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: encrypted blocks
diff --git a/arch/x86/crypto/serpent-avx2-asm_64.S b/arch/x86/crypto/serpent-avx2-asm_64.S
index d67888f2a52a..b73e11d94489 100644
--- a/arch/x86/crypto/serpent-avx2-asm_64.S
+++ b/arch/x86/crypto/serpent-avx2-asm_64.S
@@ -566,7 +566,7 @@
 	transpose_4x4(x0, x1, x2, x3, t0, t1, t2)
 
 .align 8
-__serpent_enc_blk16:
+ENTRY_LOCAL(__serpent_enc_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: plaintext
@@ -620,7 +620,7 @@ __serpent_enc_blk16:
 ENDPROC(__serpent_enc_blk16)
 
 .align 8
-__serpent_dec_blk16:
+ENTRY_LOCAL(__serpent_dec_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: ciphertext
diff --git a/arch/x86/crypto/twofish-avx-x86_64-asm_64.S b/arch/x86/crypto/twofish-avx-x86_64-asm_64.S
index b3f49d286348..59d635935c8e 100644
--- a/arch/x86/crypto/twofish-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/twofish-avx-x86_64-asm_64.S
@@ -249,7 +249,7 @@
 	vpxor		x3, wkey, x3;
 
 .align 8
-__twofish_enc_blk8:
+ENTRY_LOCAL(__twofish_enc_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: blocks
@@ -291,7 +291,7 @@ __twofish_enc_blk8:
 ENDPROC(__twofish_enc_blk8)
 
 .align 8
-__twofish_dec_blk8:
+ENTRY_LOCAL(__twofish_dec_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RC1, RD1, RA1, RB1, RC2, RD2, RA2, RB2: encrypted blocks
-- 
2.11.1

^ permalink raw reply related

* Re: [PATCH] crypto: Fix next IV issue for CTS template
From: Ard Biesheuvel @ 2017-02-17 10:42 UTC (permalink / raw)
  To: Dennis Chen
  Cc: Libo.Wang, Herbert Xu, David S. Miller,
	linux-crypto@vger.kernel.org, Ofir.Drang, nd
In-Reply-To: <20170217100643.GB28198@arm.com>


> On 17 Feb 2017, at 10:06, Dennis Chen <dennis.chen@arm.com> wrote:
> 
>> On Fri, Feb 17, 2017 at 09:23:00AM +0000, Ard Biesheuvel wrote:
>> 
>>> On 17 Feb 2017, at 09:17, Dennis Chen <dennis.chen@arm.com> wrote:
>>> 
>>> Hello Ard,
>>> Morning!
>>>> On Fri, Feb 17, 2017 at 07:12:46AM +0000, Ard Biesheuvel wrote:
>>>> Hello Libo,
>>>> 
>>>>> On 17 February 2017 at 03:47,  <Libo.Wang@arm.com> wrote:
>>>>> From: Libo Wang <Libo Wang@arm.com>
>>>>> 
>>>>> CTS template assumes underlying CBC algorithm will carry out next IV for
>>>>> further process.But some implementations of CBC algorithm in kernel break
>>>>> this assumption, for example, some hardware crypto drivers ignore next IV
>>>>> for performance consider, inthis case, tcry cts(cbc(aes)) test case will
>>>>> fail. This patch is trying to fix it by getting next IV information ready
>>>>> before last two blocks processed.
>>>>> 
>>>>> Signed-off-by: Libo Wang <libo.wang@arm.com>
>>>>> Signed-off-by: Dennis Chen <dennis.chen@arm.com>
>>>> 
>>>> Which algorithms in particular break this assumption? I recently fixed
>>>> some ARM accelerated software drivers for this reason. If there are
>>>> others, we should fix those rather than try to fix it in the CTS
>>>> driver.
>>>> 
>>> There're some ARM HW accelerated drivers which are not upstream yet(IP license)
>>> breaks this assumption. The current CTS template requires the underlying CBC 
>>> algorithm always fill the next IV, but in some cases like CBC standalone mode, 
>>> it doesn't need to fill the next IV, so apparently it will induce performance
>>> degrade from the point of drivers, that's why we fix it in the CTS template,except
>>> that it will also mitigate the potential defect of other HW-based CBC drivers. 
>> 
>> You should fix this in the driver, not in the cts code.
>> 
> Ah, we're open for the change, but is it better to give the reason to fix it in the driver
> instead of in cts code? 

Because it is the h/w driver that violates the api.

Please search the list: I discussed this with Herbert very recently

>>>> 
>>>>> ---
>>>>> crypto/cts.c | 29 +++++++++++++++++++++++++----
>>>>> 1 file changed, 25 insertions(+), 4 deletions(-)
>>>>> 
>>>>> diff --git a/crypto/cts.c b/crypto/cts.c
>>>>> index a1335d6..712164b 100644
>>>>> --- a/crypto/cts.c
>>>>> +++ b/crypto/cts.c
>>>>> @@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
>>>>>       unsigned int nbytes = req->cryptlen;
>>>>>       int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
>>>>>       unsigned int offset;
>>>>> +       int ret = 0;
>>>>> 
>>>>>       skcipher_request_set_tfm(subreq, ctx->child);
>>>>> 
>>>>> @@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
>>>>>       skcipher_request_set_crypt(subreq, req->src, req->dst,
>>>>>                                  offset, req->iv);
>>>>> 
>>>>> -       return crypto_skcipher_encrypt(subreq) ?:
>>>>> -              cts_cbc_encrypt(req);
>>>>> +       /* process CBC blocks */
>>>>> +       ret = crypto_skcipher_encrypt(subreq);
>>>>> +       /* process last two blocks */
>>>>> +       if (!ret) {
>>>> 
>>>> What happens if an async driver returns -EINPROGRESS here?
>>> For this case, the CTS will return -EINPROGRESS directly.
>>> 
>> 
>> I see. But how do you handle the missing out IV when the async call completes?
>> 
> Not quite understand here, is there some logic inconsistent of the change here comparing the
> original one to handle this case? We think it will have the same result if the fix is in driver layer.
> 
> Thanks,
> Dennis 
>> 
>>>> 
>>>>> +               /* Get IVn-1 back */
>>>>> +               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
>>>>> +               /* Continue last two blocks */
>>>>> +               return cts_cbc_encrypt(req);
>>>>> +       }
>>>>> +
>>>>> +       return ret;
>>>>> }
>>>>> 
>>>>> static int cts_cbc_decrypt(struct skcipher_request *req)
>>>>> @@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
>>>>>       int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
>>>>>       unsigned int offset;
>>>>>       u8 *space;
>>>>> +       int ret = 0;
>>>>> +       u8 iv_next[bsize];
>>>>> 
>>>>>       skcipher_request_set_tfm(subreq, ctx->child);
>>>>> 
>>>>> @@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
>>>>>       skcipher_request_set_crypt(subreq, req->src, req->dst,
>>>>>                                  offset, req->iv);
>>>>> 
>>>>> -       return crypto_skcipher_decrypt(subreq) ?:
>>>>> -              cts_cbc_decrypt(req);
>>>>> +       /* process last two blocks */
>>>>> +       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
>>>>> +       ret = crypto_skcipher_decrypt(subreq);
>>>>> +       if (!ret) {
>>>>> +               /* Set Next IV */
>>>>> +               subreq->iv = iv_next;
>>>>> +               /* process last two blocks */
>>>>> +               return cts_cbc_decrypt(req);
>>>>> +       }
>>>>> +
>>>>> +       return ret;
>>>>> }
>>>>> 
>>>>> static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)
>>>>> --
>>>>> 1.8.3.1
>>>>> 

^ permalink raw reply

* Re: [PATCH] crypto: Fix next IV issue for CTS template
From: Dennis Chen @ 2017-02-17 10:06 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Libo.Wang, Herbert Xu, David S. Miller,
	linux-crypto@vger.kernel.org, Ofir.Drang, nd
In-Reply-To: <486E1F1E-8905-45C5-9AD7-9DB3E33CA338@linaro.org>

On Fri, Feb 17, 2017 at 09:23:00AM +0000, Ard Biesheuvel wrote:
> 
> > On 17 Feb 2017, at 09:17, Dennis Chen <dennis.chen@arm.com> wrote:
> > 
> > Hello Ard,
> > Morning!
> >> On Fri, Feb 17, 2017 at 07:12:46AM +0000, Ard Biesheuvel wrote:
> >> Hello Libo,
> >> 
> >>> On 17 February 2017 at 03:47,  <Libo.Wang@arm.com> wrote:
> >>> From: Libo Wang <Libo Wang@arm.com>
> >>> 
> >>> CTS template assumes underlying CBC algorithm will carry out next IV for
> >>> further process.But some implementations of CBC algorithm in kernel break
> >>> this assumption, for example, some hardware crypto drivers ignore next IV
> >>> for performance consider, inthis case, tcry cts(cbc(aes)) test case will
> >>> fail. This patch is trying to fix it by getting next IV information ready
> >>> before last two blocks processed.
> >>> 
> >>> Signed-off-by: Libo Wang <libo.wang@arm.com>
> >>> Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> >> 
> >> Which algorithms in particular break this assumption? I recently fixed
> >> some ARM accelerated software drivers for this reason. If there are
> >> others, we should fix those rather than try to fix it in the CTS
> >> driver.
> >> 
> > There're some ARM HW accelerated drivers which are not upstream yet(IP license)
> > breaks this assumption. The current CTS template requires the underlying CBC 
> > algorithm always fill the next IV, but in some cases like CBC standalone mode, 
> > it doesn't need to fill the next IV, so apparently it will induce performance
> > degrade from the point of drivers, that's why we fix it in the CTS template,except
> > that it will also mitigate the potential defect of other HW-based CBC drivers. 
> 
> You should fix this in the driver, not in the cts code.
>
Ah, we're open for the change, but is it better to give the reason to fix it in the driver
instead of in cts code? 
> >> 
> >>> ---
> >>> crypto/cts.c | 29 +++++++++++++++++++++++++----
> >>> 1 file changed, 25 insertions(+), 4 deletions(-)
> >>> 
> >>> diff --git a/crypto/cts.c b/crypto/cts.c
> >>> index a1335d6..712164b 100644
> >>> --- a/crypto/cts.c
> >>> +++ b/crypto/cts.c
> >>> @@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
> >>>        unsigned int nbytes = req->cryptlen;
> >>>        int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
> >>>        unsigned int offset;
> >>> +       int ret = 0;
> >>> 
> >>>        skcipher_request_set_tfm(subreq, ctx->child);
> >>> 
> >>> @@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
> >>>        skcipher_request_set_crypt(subreq, req->src, req->dst,
> >>>                                   offset, req->iv);
> >>> 
> >>> -       return crypto_skcipher_encrypt(subreq) ?:
> >>> -              cts_cbc_encrypt(req);
> >>> +       /* process CBC blocks */
> >>> +       ret = crypto_skcipher_encrypt(subreq);
> >>> +       /* process last two blocks */
> >>> +       if (!ret) {
> >> 
> >> What happens if an async driver returns -EINPROGRESS here?
> > For this case, the CTS will return -EINPROGRESS directly.
> > 
> 
> I see. But how do you handle the missing out IV when the async call completes?
>
Not quite understand here, is there some logic inconsistent of the change here comparing the
original one to handle this case? We think it will have the same result if the fix is in driver layer.

Thanks,
Dennis 
>
> >> 
> >>> +               /* Get IVn-1 back */
> >>> +               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
> >>> +               /* Continue last two blocks */
> >>> +               return cts_cbc_encrypt(req);
> >>> +       }
> >>> +
> >>> +       return ret;
> >>> }
> >>> 
> >>> static int cts_cbc_decrypt(struct skcipher_request *req)
> >>> @@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
> >>>        int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
> >>>        unsigned int offset;
> >>>        u8 *space;
> >>> +       int ret = 0;
> >>> +       u8 iv_next[bsize];
> >>> 
> >>>        skcipher_request_set_tfm(subreq, ctx->child);
> >>> 
> >>> @@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
> >>>        skcipher_request_set_crypt(subreq, req->src, req->dst,
> >>>                                   offset, req->iv);
> >>> 
> >>> -       return crypto_skcipher_decrypt(subreq) ?:
> >>> -              cts_cbc_decrypt(req);
> >>> +       /* process last two blocks */
> >>> +       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
> >>> +       ret = crypto_skcipher_decrypt(subreq);
> >>> +       if (!ret) {
> >>> +               /* Set Next IV */
> >>> +               subreq->iv = iv_next;
> >>> +               /* process last two blocks */
> >>> +               return cts_cbc_decrypt(req);
> >>> +       }
> >>> +
> >>> +       return ret;
> >>> }
> >>> 
> >>> static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)
> >>> --
> >>> 1.8.3.1
> >>> 

^ permalink raw reply

* Re: [PATCH] crypto: Fix next IV issue for CTS template
From: Ard Biesheuvel @ 2017-02-17  9:23 UTC (permalink / raw)
  To: Dennis Chen
  Cc: Libo.Wang, Herbert Xu, David S. Miller,
	linux-crypto@vger.kernel.org, Ofir.Drang, nd
In-Reply-To: <20170217091724.GA28198@arm.com>


> On 17 Feb 2017, at 09:17, Dennis Chen <dennis.chen@arm.com> wrote:
> 
> Hello Ard,
> Morning!
>> On Fri, Feb 17, 2017 at 07:12:46AM +0000, Ard Biesheuvel wrote:
>> Hello Libo,
>> 
>>> On 17 February 2017 at 03:47,  <Libo.Wang@arm.com> wrote:
>>> From: Libo Wang <Libo Wang@arm.com>
>>> 
>>> CTS template assumes underlying CBC algorithm will carry out next IV for
>>> further process.But some implementations of CBC algorithm in kernel break
>>> this assumption, for example, some hardware crypto drivers ignore next IV
>>> for performance consider, inthis case, tcry cts(cbc(aes)) test case will
>>> fail. This patch is trying to fix it by getting next IV information ready
>>> before last two blocks processed.
>>> 
>>> Signed-off-by: Libo Wang <libo.wang@arm.com>
>>> Signed-off-by: Dennis Chen <dennis.chen@arm.com>
>> 
>> Which algorithms in particular break this assumption? I recently fixed
>> some ARM accelerated software drivers for this reason. If there are
>> others, we should fix those rather than try to fix it in the CTS
>> driver.
>> 
> There're some ARM HW accelerated drivers which are not upstream yet(IP license)
> breaks this assumption. The current CTS template requires the underlying CBC 
> algorithm always fill the next IV, but in some cases like CBC standalone mode, 
> it doesn't need to fill the next IV, so apparently it will induce performance
> degrade from the point of drivers, that's why we fix it in the CTS template,except
> that it will also mitigate the potential defect of other HW-based CBC drivers. 

You should fix this in the driver, not in the cts code.

>> 
>>> ---
>>> crypto/cts.c | 29 +++++++++++++++++++++++++----
>>> 1 file changed, 25 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/crypto/cts.c b/crypto/cts.c
>>> index a1335d6..712164b 100644
>>> --- a/crypto/cts.c
>>> +++ b/crypto/cts.c
>>> @@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
>>>        unsigned int nbytes = req->cryptlen;
>>>        int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
>>>        unsigned int offset;
>>> +       int ret = 0;
>>> 
>>>        skcipher_request_set_tfm(subreq, ctx->child);
>>> 
>>> @@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
>>>        skcipher_request_set_crypt(subreq, req->src, req->dst,
>>>                                   offset, req->iv);
>>> 
>>> -       return crypto_skcipher_encrypt(subreq) ?:
>>> -              cts_cbc_encrypt(req);
>>> +       /* process CBC blocks */
>>> +       ret = crypto_skcipher_encrypt(subreq);
>>> +       /* process last two blocks */
>>> +       if (!ret) {
>> 
>> What happens if an async driver returns -EINPROGRESS here?
> For this case, the CTS will return -EINPROGRESS directly.
> 

I see. But how do you handle the missing out IV when the async call completes?

>> 
>>> +               /* Get IVn-1 back */
>>> +               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
>>> +               /* Continue last two blocks */
>>> +               return cts_cbc_encrypt(req);
>>> +       }
>>> +
>>> +       return ret;
>>> }
>>> 
>>> static int cts_cbc_decrypt(struct skcipher_request *req)
>>> @@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
>>>        int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
>>>        unsigned int offset;
>>>        u8 *space;
>>> +       int ret = 0;
>>> +       u8 iv_next[bsize];
>>> 
>>>        skcipher_request_set_tfm(subreq, ctx->child);
>>> 
>>> @@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
>>>        skcipher_request_set_crypt(subreq, req->src, req->dst,
>>>                                   offset, req->iv);
>>> 
>>> -       return crypto_skcipher_decrypt(subreq) ?:
>>> -              cts_cbc_decrypt(req);
>>> +       /* process last two blocks */
>>> +       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
>>> +       ret = crypto_skcipher_decrypt(subreq);
>>> +       if (!ret) {
>>> +               /* Set Next IV */
>>> +               subreq->iv = iv_next;
>>> +               /* process last two blocks */
>>> +               return cts_cbc_decrypt(req);
>>> +       }
>>> +
>>> +       return ret;
>>> }
>>> 
>>> static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)
>>> --
>>> 1.8.3.1
>>> 

^ permalink raw reply

* Re: [PATCH] crypto: Fix next IV issue for CTS template
From: Dennis Chen @ 2017-02-17  9:17 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Libo.Wang, Herbert Xu, David S. Miller,
	linux-crypto@vger.kernel.org, Ofir.Drang, nd
In-Reply-To: <CAKv+Gu8s6PrAv9ZMGa_TsRn2UYKoo8KhqERU0NZPjfBhMDK88Q@mail.gmail.com>

Hello Ard,
Morning!
On Fri, Feb 17, 2017 at 07:12:46AM +0000, Ard Biesheuvel wrote:
> Hello Libo,
> 
> On 17 February 2017 at 03:47,  <Libo.Wang@arm.com> wrote:
> > From: Libo Wang <Libo Wang@arm.com>
> >
> > CTS template assumes underlying CBC algorithm will carry out next IV for
> > further process.But some implementations of CBC algorithm in kernel break
> > this assumption, for example, some hardware crypto drivers ignore next IV
> > for performance consider, inthis case, tcry cts(cbc(aes)) test case will
> > fail. This patch is trying to fix it by getting next IV information ready
> > before last two blocks processed.
> >
> > Signed-off-by: Libo Wang <libo.wang@arm.com>
> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> 
> Which algorithms in particular break this assumption? I recently fixed
> some ARM accelerated software drivers for this reason. If there are
> others, we should fix those rather than try to fix it in the CTS
> driver.
>
There're some ARM HW accelerated drivers which are not upstream yet(IP license)
breaks this assumption. The current CTS template requires the underlying CBC 
algorithm always fill the next IV, but in some cases like CBC standalone mode, 
it doesn't need to fill the next IV, so apparently it will induce performance
degrade from the point of drivers, that's why we fix it in the CTS template,except
that it will also mitigate the potential defect of other HW-based CBC drivers. 
> 
> > ---
> >  crypto/cts.c | 29 +++++++++++++++++++++++++----
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/crypto/cts.c b/crypto/cts.c
> > index a1335d6..712164b 100644
> > --- a/crypto/cts.c
> > +++ b/crypto/cts.c
> > @@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
> >         unsigned int nbytes = req->cryptlen;
> >         int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
> >         unsigned int offset;
> > +       int ret = 0;
> >
> >         skcipher_request_set_tfm(subreq, ctx->child);
> >
> > @@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
> >         skcipher_request_set_crypt(subreq, req->src, req->dst,
> >                                    offset, req->iv);
> >
> > -       return crypto_skcipher_encrypt(subreq) ?:
> > -              cts_cbc_encrypt(req);
> > +       /* process CBC blocks */
> > +       ret = crypto_skcipher_encrypt(subreq);
> > +       /* process last two blocks */
> > +       if (!ret) {
> 
> What happens if an async driver returns -EINPROGRESS here?
For this case, the CTS will return -EINPROGRESS directly.

Thanks,
Dennis
> 
> > +               /* Get IVn-1 back */
> > +               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
> > +               /* Continue last two blocks */
> > +               return cts_cbc_encrypt(req);
> > +       }
> > +
> > +       return ret;
> >  }
> >
> >  static int cts_cbc_decrypt(struct skcipher_request *req)
> > @@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
> >         int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
> >         unsigned int offset;
> >         u8 *space;
> > +       int ret = 0;
> > +       u8 iv_next[bsize];
> >
> >         skcipher_request_set_tfm(subreq, ctx->child);
> >
> > @@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
> >         skcipher_request_set_crypt(subreq, req->src, req->dst,
> >                                    offset, req->iv);
> >
> > -       return crypto_skcipher_decrypt(subreq) ?:
> > -              cts_cbc_decrypt(req);
> > +       /* process last two blocks */
> > +       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
> > +       ret = crypto_skcipher_decrypt(subreq);
> > +       if (!ret) {
> > +               /* Set Next IV */
> > +               subreq->iv = iv_next;
> > +               /* process last two blocks */
> > +               return cts_cbc_decrypt(req);
> > +       }
> > +
> > +       return ret;
> >  }
> >
> >  static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)
> > --
> > 1.8.3.1
> >

^ permalink raw reply

* Re: [PATCH] crypto: Fix next IV issue for CTS template
From: Ard Biesheuvel @ 2017-02-17  7:12 UTC (permalink / raw)
  To: Libo.Wang
  Cc: Herbert Xu, David S. Miller, linux-crypto@vger.kernel.org,
	Ofir.Drang, Dennis Chen
In-Reply-To: <1487303262-5602-1-git-send-email-Libo.Wang@arm.com>

Hello Libo,

On 17 February 2017 at 03:47,  <Libo.Wang@arm.com> wrote:
> From: Libo Wang <Libo Wang@arm.com>
>
> CTS template assumes underlying CBC algorithm will carry out next IV for
> further process.But some implementations of CBC algorithm in kernel break
> this assumption, for example, some hardware crypto drivers ignore next IV
> for performance consider, inthis case, tcry cts(cbc(aes)) test case will
> fail. This patch is trying to fix it by getting next IV information ready
> before last two blocks processed.
>
> Signed-off-by: Libo Wang <libo.wang@arm.com>
> Signed-off-by: Dennis Chen <dennis.chen@arm.com>

Which algorithms in particular break this assumption? I recently fixed
some ARM accelerated software drivers for this reason. If there are
others, we should fix those rather than try to fix it in the CTS
driver.

> ---
>  crypto/cts.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/crypto/cts.c b/crypto/cts.c
> index a1335d6..712164b 100644
> --- a/crypto/cts.c
> +++ b/crypto/cts.c
> @@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
>         unsigned int nbytes = req->cryptlen;
>         int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
>         unsigned int offset;
> +       int ret = 0;
>
>         skcipher_request_set_tfm(subreq, ctx->child);
>
> @@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
>         skcipher_request_set_crypt(subreq, req->src, req->dst,
>                                    offset, req->iv);
>
> -       return crypto_skcipher_encrypt(subreq) ?:
> -              cts_cbc_encrypt(req);
> +       /* process CBC blocks */
> +       ret = crypto_skcipher_encrypt(subreq);
> +       /* process last two blocks */
> +       if (!ret) {

What happens if an async driver returns -EINPROGRESS here?

> +               /* Get IVn-1 back */
> +               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
> +               /* Continue last two blocks */
> +               return cts_cbc_encrypt(req);
> +       }
> +
> +       return ret;
>  }
>
>  static int cts_cbc_decrypt(struct skcipher_request *req)
> @@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
>         int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
>         unsigned int offset;
>         u8 *space;
> +       int ret = 0;
> +       u8 iv_next[bsize];
>
>         skcipher_request_set_tfm(subreq, ctx->child);
>
> @@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
>         skcipher_request_set_crypt(subreq, req->src, req->dst,
>                                    offset, req->iv);
>
> -       return crypto_skcipher_decrypt(subreq) ?:
> -              cts_cbc_decrypt(req);
> +       /* process last two blocks */
> +       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
> +       ret = crypto_skcipher_decrypt(subreq);
> +       if (!ret) {
> +               /* Set Next IV */
> +               subreq->iv = iv_next;
> +               /* process last two blocks */
> +               return cts_cbc_decrypt(req);
> +       }
> +
> +       return ret;
>  }
>
>  static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)
> --
> 1.8.3.1
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Please configure your email client so it doesn't spit out these.

^ permalink raw reply

* [PATCH 15/35] drivers/crypto: Convert remaining uses of pr_warning to pr_warn
From: Joe Perches @ 2017-02-17  7:11 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller; +Cc: linux-crypto, linux-kernel
In-Reply-To: <cover.1487314666.git.joe@perches.com>

To enable eventual removal of pr_warning

This makes pr_warn use consistent for drivers/crypto

Prior to this patch, there were 3 uses of pr_warning and
12 uses of pr_warn in drivers/crypto

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/crypto/n2_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index c5aac25a5738..82ab91adfee7 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -365,8 +365,8 @@ static int n2_hash_cra_init(struct crypto_tfm *tfm)
 	fallback_tfm = crypto_alloc_ahash(fallback_driver_name, 0,
 					  CRYPTO_ALG_NEED_FALLBACK);
 	if (IS_ERR(fallback_tfm)) {
-		pr_warning("Fallback driver '%s' could not be loaded!\n",
-			   fallback_driver_name);
+		pr_warn("Fallback driver '%s' could not be loaded!\n",
+			fallback_driver_name);
 		err = PTR_ERR(fallback_tfm);
 		goto out;
 	}
@@ -402,16 +402,16 @@ static int n2_hmac_cra_init(struct crypto_tfm *tfm)
 	fallback_tfm = crypto_alloc_ahash(fallback_driver_name, 0,
 					  CRYPTO_ALG_NEED_FALLBACK);
 	if (IS_ERR(fallback_tfm)) {
-		pr_warning("Fallback driver '%s' could not be loaded!\n",
-			   fallback_driver_name);
+		pr_warn("Fallback driver '%s' could not be loaded!\n",
+			fallback_driver_name);
 		err = PTR_ERR(fallback_tfm);
 		goto out;
 	}
 
 	child_shash = crypto_alloc_shash(n2alg->child_alg, 0, 0);
 	if (IS_ERR(child_shash)) {
-		pr_warning("Child shash '%s' could not be loaded!\n",
-			   n2alg->child_alg);
+		pr_warn("Child shash '%s' could not be loaded!\n",
+			n2alg->child_alg);
 		err = PTR_ERR(child_shash);
 		goto out_free_fallback;
 	}
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related

* [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn
From: Joe Perches @ 2017-02-17  7:11 UTC (permalink / raw)
  To: Alexander Shishkin, Karol Herbst, Pekka Paalanen,
	Richard Weinberger, Fabio Estevam, linux-kernel, linux-arm-kernel,
	linuxppc-dev, tboot-devel, nouveau, oprofile-list, sfi-devel,
	xen-devel, linux-acpi, drbd-dev, virtualization, linux-crypto,
	linux-ide, gigaset307x-common, linux-media, linux-omap, linux-mtd,
	devicetree, acpi4asus-user, platform-driver-x86, linux-scsi, lin
  Cc: linux-ia64, linux-sh, netdev, linux-input, adi-buildroot-devel,
	amd-gfx, dri-devel, linux-alpha, sparclinux

There are ~4300 uses of pr_warn and ~250 uses of the older
pr_warning in the kernel source tree.

Make the use of pr_warn consistent across all kernel files.

This excludes all files in tools/ as there is a separate
define pr_warning for that directory tree and pr_warn is
not used in tools/.

Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Miscellanea:

o Coalesce formats and realign arguments

Some files not compiled - no cross-compilers

Joe Perches (35):
  alpha: Convert remaining uses of pr_warning to pr_warn
  ARM: ep93xx: Convert remaining uses of pr_warning to pr_warn
  arm64: Convert remaining uses of pr_warning to pr_warn
  arch/blackfin: Convert remaining uses of pr_warning to pr_warn
  ia64: Convert remaining use of pr_warning to pr_warn
  powerpc: Convert remaining uses of pr_warning to pr_warn
  sh: Convert remaining uses of pr_warning to pr_warn
  sparc: Convert remaining use of pr_warning to pr_warn
  x86: Convert remaining uses of pr_warning to pr_warn
  drivers/acpi: Convert remaining uses of pr_warning to pr_warn
  block/drbd: Convert remaining uses of pr_warning to pr_warn
  gdrom: Convert remaining uses of pr_warning to pr_warn
  drivers/char: Convert remaining use of pr_warning to pr_warn
  clocksource: Convert remaining use of pr_warning to pr_warn
  drivers/crypto: Convert remaining uses of pr_warning to pr_warn
  fmc: Convert remaining use of pr_warning to pr_warn
  drivers/gpu: Convert remaining uses of pr_warning to pr_warn
  drivers/ide: Convert remaining uses of pr_warning to pr_warn
  drivers/input: Convert remaining uses of pr_warning to pr_warn
  drivers/isdn: Convert remaining uses of pr_warning to pr_warn
  drivers/macintosh: Convert remaining uses of pr_warning to pr_warn
  drivers/media: Convert remaining use of pr_warning to pr_warn
  drivers/mfd: Convert remaining uses of pr_warning to pr_warn
  drivers/mtd: Convert remaining uses of pr_warning to pr_warn
  drivers/of: Convert remaining uses of pr_warning to pr_warn
  drivers/oprofile: Convert remaining uses of pr_warning to pr_warn
  drivers/platform: Convert remaining uses of pr_warning to pr_warn
  drivers/rapidio: Convert remaining use of pr_warning to pr_warn
  drivers/scsi: Convert remaining use of pr_warning to pr_warn
  drivers/sh: Convert remaining use of pr_warning to pr_warn
  drivers/tty: Convert remaining uses of pr_warning to pr_warn
  drivers/video: Convert remaining uses of pr_warning to pr_warn
  kernel/trace: Convert remaining uses of pr_warning to pr_warn
  lib: Convert remaining uses of pr_warning to pr_warn
  sound/soc: Convert remaining uses of pr_warning to pr_warn

 arch/alpha/kernel/perf_event.c                     |  4 +-
 arch/arm/mach-ep93xx/core.c                        |  4 +-
 arch/arm64/include/asm/syscall.h                   |  8 ++--
 arch/arm64/kernel/hw_breakpoint.c                  |  8 ++--
 arch/arm64/kernel/smp.c                            |  4 +-
 arch/blackfin/kernel/nmi.c                         |  2 +-
 arch/blackfin/kernel/ptrace.c                      |  2 +-
 arch/blackfin/mach-bf533/boards/stamp.c            |  2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537e.c        |  2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537u.c        |  2 +-
 arch/blackfin/mach-bf537/boards/stamp.c            |  2 +-
 arch/blackfin/mach-bf537/boards/tcm_bf537.c        |  2 +-
 arch/blackfin/mach-bf561/boards/cm_bf561.c         |  2 +-
 arch/blackfin/mach-bf561/boards/ezkit.c            |  2 +-
 arch/blackfin/mm/isram-driver.c                    |  4 +-
 arch/ia64/kernel/setup.c                           |  6 +--
 arch/powerpc/kernel/pci-common.c                   |  4 +-
 arch/powerpc/mm/init_64.c                          |  5 +--
 arch/powerpc/mm/mem.c                              |  3 +-
 arch/powerpc/platforms/512x/mpc512x_shared.c       |  4 +-
 arch/powerpc/platforms/85xx/socrates_fpga_pic.c    |  7 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c         |  2 +-
 arch/powerpc/platforms/pasemi/dma_lib.c            |  4 +-
 arch/powerpc/platforms/powernv/opal.c              |  8 ++--
 arch/powerpc/platforms/powernv/pci-ioda.c          | 10 ++---
 arch/powerpc/platforms/ps3/device-init.c           | 14 +++----
 arch/powerpc/platforms/ps3/mm.c                    |  4 +-
 arch/powerpc/platforms/ps3/os-area.c               |  2 +-
 arch/powerpc/platforms/pseries/iommu.c             |  8 ++--
 arch/powerpc/platforms/pseries/setup.c             |  4 +-
 arch/powerpc/sysdev/fsl_pci.c                      |  9 ++---
 arch/powerpc/sysdev/mpic.c                         | 10 ++---
 arch/powerpc/sysdev/xics/icp-native.c              | 10 ++---
 arch/powerpc/sysdev/xics/ics-opal.c                |  4 +-
 arch/powerpc/sysdev/xics/ics-rtas.c                |  4 +-
 arch/powerpc/sysdev/xics/xics-common.c             |  8 ++--
 arch/sh/boards/mach-sdk7786/nmi.c                  |  2 +-
 arch/sh/drivers/pci/fixups-sdk7786.c               |  2 +-
 arch/sh/kernel/io_trapped.c                        |  2 +-
 arch/sh/kernel/setup.c                             |  2 +-
 arch/sh/mm/consistent.c                            |  5 +--
 arch/sparc/kernel/smp_64.c                         |  5 +--
 arch/x86/kernel/amd_gart_64.c                      | 12 ++----
 arch/x86/kernel/apic/apic.c                        | 46 ++++++++++------------
 arch/x86/kernel/apic/apic_noop.c                   |  2 +-
 arch/x86/kernel/setup_percpu.c                     |  4 +-
 arch/x86/kernel/tboot.c                            | 15 ++++---
 arch/x86/kernel/tsc_sync.c                         |  8 ++--
 arch/x86/mm/kmmio.c                                |  8 ++--
 arch/x86/mm/mmio-mod.c                             |  5 +--
 arch/x86/mm/numa.c                                 | 12 +++---
 arch/x86/mm/numa_emulation.c                       |  6 +--
 arch/x86/mm/testmmiotrace.c                        |  5 +--
 arch/x86/oprofile/op_x86_model.h                   |  6 +--
 arch/x86/platform/olpc/olpc-xo15-sci.c             |  2 +-
 arch/x86/platform/sfi/sfi.c                        |  3 +-
 arch/x86/xen/debugfs.c                             |  2 +-
 arch/x86/xen/setup.c                               |  2 +-
 drivers/acpi/apei/apei-base.c                      | 32 +++++++--------
 drivers/acpi/apei/einj.c                           |  4 +-
 drivers/acpi/apei/erst-dbg.c                       |  4 +-
 drivers/acpi/apei/ghes.c                           | 30 +++++++-------
 drivers/acpi/apei/hest.c                           | 10 ++---
 drivers/acpi/resource.c                            |  4 +-
 drivers/block/drbd/drbd_nl.c                       | 13 +++---
 drivers/cdrom/gdrom.c                              |  4 +-
 drivers/char/virtio_console.c                      |  2 +-
 drivers/clocksource/samsung_pwm_timer.c            |  4 +-
 drivers/crypto/n2_core.c                           | 12 +++---
 drivers/fmc/fmc-fakedev.c                          |  2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h       |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c    |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 +++----
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/ide/tx4938ide.c                            |  2 +-
 drivers/ide/tx4939ide.c                            |  5 +--
 drivers/input/gameport/gameport.c                  |  4 +-
 drivers/input/joystick/gamecon.c                   |  3 +-
 drivers/input/misc/apanel.c                        |  3 +-
 drivers/input/misc/xen-kbdfront.c                  |  8 ++--
 drivers/input/serio/serio.c                        |  8 ++--
 drivers/isdn/gigaset/interface.c                   |  2 +-
 drivers/isdn/hardware/mISDN/avmfritz.c             | 17 ++++----
 drivers/isdn/hardware/mISDN/hfcmulti.c             |  8 ++--
 drivers/isdn/hardware/mISDN/hfcpci.c               |  4 +-
 drivers/isdn/hardware/mISDN/hfcsusb.c              |  4 +-
 drivers/isdn/hardware/mISDN/mISDNipac.c            |  4 +-
 drivers/isdn/hardware/mISDN/mISDNisar.c            | 10 ++---
 drivers/isdn/hardware/mISDN/netjet.c               |  8 ++--
 drivers/isdn/hardware/mISDN/w6692.c                | 12 +++---
 drivers/isdn/mISDN/hwchannel.c                     |  8 ++--
 drivers/macintosh/windfarm_fcu_controls.c          |  5 +--
 drivers/macintosh/windfarm_lm87_sensor.c           |  4 +-
 drivers/macintosh/windfarm_pm72.c                  | 22 +++++------
 drivers/macintosh/windfarm_rm31.c                  |  6 +--
 drivers/media/platform/sh_vou.c                    |  4 +-
 drivers/mfd/db8500-prcmu.c                         |  2 +-
 drivers/mfd/sta2x11-mfd.c                          |  4 +-
 drivers/mfd/twl4030-power.c                        |  7 +---
 drivers/mtd/chips/cfi_cmdset_0002.c                | 12 ++++--
 drivers/mtd/nand/cmx270_nand.c                     |  4 +-
 drivers/mtd/ofpart.c                               |  4 +-
 drivers/of/fdt.c                                   | 20 +++++-----
 drivers/oprofile/oprofile_perf.c                   |  8 ++--
 drivers/platform/x86/asus-laptop.c                 |  2 +-
 drivers/platform/x86/eeepc-laptop.c                |  2 +-
 drivers/platform/x86/intel_oaktrail.c              | 10 ++---
 drivers/rapidio/rio-sysfs.c                        |  4 +-
 drivers/scsi/a3000.c                               |  2 +-
 drivers/sh/intc/core.c                             |  4 +-
 drivers/tty/hvc/hvcs.c                             |  2 +-
 drivers/tty/tty_io.c                               |  4 +-
 drivers/video/fbdev/aty/radeon_base.c              |  4 +-
 drivers/video/fbdev/core/fbmon.c                   |  4 +-
 drivers/video/fbdev/pxafb.c                        |  7 ++--
 kernel/trace/trace_benchmark.c                     |  4 +-
 lib/cpu_rmap.c                                     |  2 +-
 lib/dma-debug.c                                    |  2 +-
 sound/soc/fsl/imx-audmux.c                         |  6 +--
 sound/soc/samsung/s3c-i2s-v2.c                     |  6 +--
 122 files changed, 367 insertions(+), 397 deletions(-)

-- 
2.10.0.rc2.1.g053435c


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* [PATCH] crypto: Fix next IV issue for CTS template
From: Libo.Wang @ 2017-02-17  3:47 UTC (permalink / raw)
  To: herbert, davem, linux-crypto
  Cc: ard.biesheuvel, Ofir.Drang, Libo Wang, Dennis Chen

From: Libo Wang <Libo Wang@arm.com>

CTS template assumes underlying CBC algorithm will carry out next IV for
further process.But some implementations of CBC algorithm in kernel break
this assumption, for example, some hardware crypto drivers ignore next IV
for performance consider, inthis case, tcry cts(cbc(aes)) test case will
fail. This patch is trying to fix it by getting next IV information ready
before last two blocks processed.

Signed-off-by: Libo Wang <libo.wang@arm.com>
Signed-off-by: Dennis Chen <dennis.chen@arm.com>
---
 crypto/cts.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/crypto/cts.c b/crypto/cts.c
index a1335d6..712164b 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c
@@ -154,6 +154,7 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
        unsigned int nbytes = req->cryptlen;
        int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
        unsigned int offset;
+       int ret = 0;

        skcipher_request_set_tfm(subreq, ctx->child);

@@ -174,8 +175,17 @@ static int crypto_cts_encrypt(struct skcipher_request *req)
        skcipher_request_set_crypt(subreq, req->src, req->dst,
                                   offset, req->iv);

-       return crypto_skcipher_encrypt(subreq) ?:
-              cts_cbc_encrypt(req);
+       /* process CBC blocks */
+       ret = crypto_skcipher_encrypt(subreq);
+       /* process last two blocks */
+       if (!ret) {
+               /* Get IVn-1 back */
+               scatterwalk_map_and_copy(req->iv, req->dst, (offset - bsize), bsize, 0);
+               /* Continue last two blocks */
+               return cts_cbc_encrypt(req);
+       }
+
+       return ret;
 }

 static int cts_cbc_decrypt(struct skcipher_request *req)
@@ -248,6 +258,8 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
        int cbc_blocks = (nbytes + bsize - 1) / bsize - 1;
        unsigned int offset;
        u8 *space;
+       int ret = 0;
+       u8 iv_next[bsize];

        skcipher_request_set_tfm(subreq, ctx->child);

@@ -277,8 +289,17 @@ static int crypto_cts_decrypt(struct skcipher_request *req)
        skcipher_request_set_crypt(subreq, req->src, req->dst,
                                   offset, req->iv);

-       return crypto_skcipher_decrypt(subreq) ?:
-              cts_cbc_decrypt(req);
+       /* process last two blocks */
+       scatterwalk_map_and_copy(iv_next, req->src, (offset - bsize), bsize, 0);
+       ret = crypto_skcipher_decrypt(subreq);
+       if (!ret) {
+               /* Set Next IV */
+               subreq->iv = iv_next;
+               /* process last two blocks */
+               return cts_cbc_decrypt(req);
+       }
+
+       return ret;
 }

 static int crypto_cts_init_tfm(struct crypto_skcipher *tfm)

^ permalink raw reply related

* [PATCH v4 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-16 21:21 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <1781086.PMKVUsvYzT@positron.chronox.de>

The updated memory management is described in the top part of the code.
As one benefit of the changed memory management, the AIO and synchronous
operation is now implemented in one common function. The AF_ALG
operation uses the async kernel crypto API interface for each cipher
operation. Thus, the only difference between the AIO and sync operation
types visible from user space is:

1. the callback function to be invoked when the asynchronous operation
   is completed

2. whether to wait for the completion of the kernel crypto API operation
   or not

In addition, the code structure is adjusted to match the structure of
algif_aead for easier code assessment.

The user space interface changed slightly as follows: the old AIO
operation returned zero upon success and < 0 in case of an error to user
space. As all other AF_ALG interfaces (including the sync skcipher
interface) returned the number of processed bytes upon success and < 0
in case of an error, the new skcipher interface (regardless of AIO or
sync) returns the number of processed bytes in case of success.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_skcipher.c | 543 ++++++++++++++++++++++--------------------------
 1 file changed, 254 insertions(+), 289 deletions(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a9e79d8..9b15c15 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -10,6 +10,21 @@
  * Software Foundation; either version 2 of the License, or (at your option)
  * any later version.
  *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
  */
 
 #include <crypto/scatterwalk.h>
@@ -23,86 +38,58 @@
 #include <linux/net.h>
 #include <net/sock.h>
 
-struct skcipher_sg_list {
+struct skcipher_tsgl {
 	struct list_head list;
-
 	int cur;
-
 	struct scatterlist sg[0];
 };
 
+struct skcipher_rsgl {
+	struct af_alg_sgl sgl;
+	struct list_head list;
+};
+
+struct skcipher_async_req {
+	struct kiocb *iocb;
+	struct sock *sk;
+
+	struct skcipher_rsgl first_sgl;
+	struct list_head rsgl_list;
+
+	struct scatterlist *tsgl;
+	unsigned int tsgl_entries;
+
+	unsigned int areqlen;
+	struct skcipher_request req;
+};
+
 struct skcipher_tfm {
 	struct crypto_skcipher *skcipher;
 	bool has_key;
 };
 
 struct skcipher_ctx {
-	struct list_head tsgl;
-	struct af_alg_sgl rsgl;
+	struct list_head tsgl_list;
 
 	void *iv;
 
 	struct af_alg_completion completion;
 
-	atomic_t inflight;
+	unsigned int inflight;
 	size_t used;
 
-	unsigned int len;
 	bool more;
 	bool merge;
 	bool enc;
 
-	struct skcipher_request req;
-};
-
-struct skcipher_async_rsgl {
-	struct af_alg_sgl sgl;
-	struct list_head list;
+	unsigned int len;
 };
 
-struct skcipher_async_req {
-	struct kiocb *iocb;
-	struct skcipher_async_rsgl first_sgl;
-	struct list_head list;
-	struct scatterlist *tsg;
-	atomic_t *inflight;
-	struct skcipher_request req;
-};
+static DECLARE_WAIT_QUEUE_HEAD(skcipher_aio_finish_wait);
 
-#define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_sg_list)) / \
+#define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_tsgl)) / \
 		      sizeof(struct scatterlist) - 1)
 
-static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
-{
-	struct skcipher_async_rsgl *rsgl, *tmp;
-	struct scatterlist *sgl;
-	struct scatterlist *sg;
-	int i, n;
-
-	list_for_each_entry_safe(rsgl, tmp, &sreq->list, list) {
-		af_alg_free_sg(&rsgl->sgl);
-		if (rsgl != &sreq->first_sgl)
-			kfree(rsgl);
-	}
-	sgl = sreq->tsg;
-	n = sg_nents(sgl);
-	for_each_sg(sgl, sg, n, i)
-		put_page(sg_page(sg));
-
-	kfree(sreq->tsg);
-}
-
-static void skcipher_async_cb(struct crypto_async_request *req, int err)
-{
-	struct skcipher_async_req *sreq = req->data;
-	struct kiocb *iocb = sreq->iocb;
-
-	atomic_dec(sreq->inflight);
-	skcipher_free_async_sgls(sreq);
-	kzfree(sreq);
-	iocb->ki_complete(iocb, err, err);
-}
-
 static inline int skcipher_sndbuf(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -117,15 +104,15 @@ static inline bool skcipher_writable(struct sock *sk)
 	return PAGE_SIZE <= skcipher_sndbuf(sk);
 }
 
-static int skcipher_alloc_sgl(struct sock *sk)
+static int skcipher_alloc_tsgl(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	struct scatterlist *sg = NULL;
 
-	sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
-	if (!list_empty(&ctx->tsgl))
+	sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_tsgl, list);
+	if (!list_empty(&ctx->tsgl_list))
 		sg = sgl->sg;
 
 	if (!sg || sgl->cur >= MAX_SGL_ENTS) {
@@ -141,31 +128,69 @@ static int skcipher_alloc_sgl(struct sock *sk)
 		if (sg)
 			sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
 
-		list_add_tail(&sgl->list, &ctx->tsgl);
+		list_add_tail(&sgl->list, &ctx->tsgl_list);
 	}
 
 	return 0;
 }
 
-static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
+static unsigned int skcipher_count_tsgl(struct sock *sk, size_t bytes)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	struct scatterlist *sg;
-	int i;
+	unsigned int i;
+	unsigned int sgl_count = 0;
+
+	if (!bytes)
+		return 0;
+
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct skcipher_tsgl,
+				       list);
+		sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			sgl_count++;
+			if (sg[i].length >= bytes)
+				return sgl_count;
+
+			bytes -= sg[i].length;
+		}
+	}
+
+	return sgl_count;
+}
+
+static void skcipher_pull_tsgl(struct sock *sk, size_t used,
+			       struct scatterlist *dst)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct skcipher_ctx *ctx = ask->private;
+	struct skcipher_tsgl *sgl;
+	struct scatterlist *sg;
+	unsigned int i;
 
-	while (!list_empty(&ctx->tsgl)) {
-		sgl = list_first_entry(&ctx->tsgl, struct skcipher_sg_list,
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct skcipher_tsgl,
 				       list);
 		sg = sgl->sg;
 
 		for (i = 0; i < sgl->cur; i++) {
 			size_t plen = min_t(size_t, used, sg[i].length);
+			struct page *page = sg_page(sg + i);
 
-			if (!sg_page(sg + i))
+			if (!page)
 				continue;
 
+			/*
+			 * Assumption: caller created skcipher_count_tsgl(len)
+			 * SG entries in dst.
+			 */
+			if (dst)
+				sg_set_page(dst + i, page, plen, sg[i].offset);
+
 			sg[i].length -= plen;
 			sg[i].offset += plen;
 
@@ -174,27 +199,45 @@ static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
 
 			if (sg[i].length)
 				return;
-			if (put)
-				put_page(sg_page(sg + i));
+
+			if (!dst)
+				put_page(page);
 			sg_assign_page(sg + i, NULL);
 		}
 
 		list_del(&sgl->list);
-		sock_kfree_s(sk, sgl,
-			     sizeof(*sgl) + sizeof(sgl->sg[0]) *
-					    (MAX_SGL_ENTS + 1));
+		sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+						     (MAX_SGL_ENTS + 1));
 	}
 
 	if (!ctx->used)
 		ctx->merge = 0;
 }
 
-static void skcipher_free_sgl(struct sock *sk)
+static void skcipher_free_areq_sgls(struct skcipher_async_req *areq)
 {
-	struct alg_sock *ask = alg_sk(sk);
-	struct skcipher_ctx *ctx = ask->private;
+	struct sock *sk = areq->sk;
+	struct skcipher_rsgl *rsgl, *tmp;
+	struct scatterlist *tsgl;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		list_del(&rsgl->list);
+		if (rsgl != &areq->first_sgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+
+	tsgl = areq->tsgl;
+	for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
+		if (!sg_page(sg))
+			continue;
+		put_page(sg_page(sg));
+	}
 
-	skcipher_pull_sgl(sk, ctx->used, 1);
+	if (areq->tsgl && areq->tsgl_entries)
+		sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
 }
 
 static int skcipher_wait_for_wmem(struct sock *sk, unsigned flags)
@@ -301,7 +344,7 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 	struct skcipher_tfm *skc = pask->private;
 	struct crypto_skcipher *tfm = skc->skcipher;
 	unsigned ivsize = crypto_skcipher_ivsize(tfm);
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	struct af_alg_control con = {};
 	long copied = 0;
 	bool enc = 0;
@@ -348,8 +391,8 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 		size_t plen;
 
 		if (ctx->merge) {
-			sgl = list_entry(ctx->tsgl.prev,
-					 struct skcipher_sg_list, list);
+			sgl = list_entry(ctx->tsgl_list.prev,
+					 struct skcipher_tsgl, list);
 			sg = sgl->sg + sgl->cur - 1;
 			len = min_t(unsigned long, len,
 				    PAGE_SIZE - sg->offset - sg->length);
@@ -378,11 +421,12 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		len = min_t(unsigned long, len, skcipher_sndbuf(sk));
 
-		err = skcipher_alloc_sgl(sk);
+		err = skcipher_alloc_tsgl(sk);
 		if (err)
 			goto unlock;
 
-		sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
+		sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_tsgl,
+				 list);
 		sg = sgl->sg;
 		if (sgl->cur)
 			sg_unmark_end(sg + sgl->cur - 1);
@@ -434,7 +478,7 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct skcipher_sg_list *sgl;
+	struct skcipher_tsgl *sgl;
 	int err = -EINVAL;
 
 	if (flags & MSG_SENDPAGE_NOTLAST)
@@ -453,12 +497,12 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
 			goto unlock;
 	}
 
-	err = skcipher_alloc_sgl(sk);
+	err = skcipher_alloc_tsgl(sk);
 	if (err)
 		goto unlock;
 
 	ctx->merge = 0;
-	sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
+	sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_tsgl, list);
 
 	if (sgl->cur)
 		sg_unmark_end(sgl->sg + sgl->cur - 1);
@@ -479,25 +523,36 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
 	return err ?: size;
 }
 
-static int skcipher_all_sg_nents(struct skcipher_ctx *ctx)
+static void skcipher_async_cb(struct crypto_async_request *req, int err)
 {
-	struct skcipher_sg_list *sgl;
-	struct scatterlist *sg;
-	int nents = 0;
+	struct skcipher_async_req *areq = req->data;
+	struct sock *sk = areq->sk;
+	struct alg_sock *ask = alg_sk(sk);
+	struct skcipher_ctx *ctx = ask->private;
+	struct kiocb *iocb = areq->iocb;
+	unsigned int resultlen;
 
-	list_for_each_entry(sgl, &ctx->tsgl, list) {
-		sg = sgl->sg;
+	lock_sock(sk);
 
-		while (!sg->length)
-			sg++;
+	BUG_ON(!ctx->inflight);
 
-		nents += sg_nents(sg);
-	}
-	return nents;
+	/* Buffer size written by crypto operation. */
+	resultlen = areq->req.cryptlen;
+
+	skcipher_free_areq_sgls(areq);
+	sock_kfree_s(sk, areq, areq->areqlen);
+	__sock_put(sk);
+	ctx->inflight--;
+
+	iocb->ki_complete(iocb, err ? err : resultlen, 0);
+
+	release_sock(sk);
+
+	wake_up_interruptible(&skcipher_aio_finish_wait);
 }
 
-static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
-				  int flags)
+static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+			    size_t ignored, int flags)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
@@ -506,215 +561,137 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
 	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;
-	unsigned int reqsize = crypto_skcipher_reqsize(tfm);
-	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+	unsigned int bs = crypto_skcipher_blocksize(tfm);
+	unsigned int areqlen = sizeof(struct skcipher_async_req) +
+			       crypto_skcipher_reqsize(tfm);
+	struct skcipher_async_req *areq;
+	struct skcipher_rsgl *last_rsgl = NULL;
 	int err = -ENOMEM;
-	bool mark = false;
-	char *iv;
-
-	sreq = kzalloc(sizeof(*sreq) + reqsize + ivsize, GFP_KERNEL);
-	if (unlikely(!sreq))
-		goto out;
-
-	req = &sreq->req;
-	iv = (char *)(req + 1) + reqsize;
-	sreq->iocb = msg->msg_iocb;
-	INIT_LIST_HEAD(&sreq->list);
-	sreq->inflight = &ctx->inflight;
+	size_t len = 0;
 
 	lock_sock(sk);
-	tx_nents = skcipher_all_sg_nents(ctx);
-	sreq->tsg = kcalloc(tx_nents, sizeof(*sg), GFP_KERNEL);
-	if (unlikely(!sreq->tsg))
+
+	/* Allocate cipher request for current operation. */
+	areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+	if (unlikely(!areq))
 		goto unlock;
-	sg_init_table(sreq->tsg, tx_nents);
-	memcpy(iv, ctx->iv, ivsize);
-	skcipher_request_set_tfm(req, tfm);
-	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
-				      skcipher_async_cb, sreq);
+	areq->areqlen = areqlen;
+	areq->sk = sk;
+	INIT_LIST_HEAD(&areq->rsgl_list);
+	areq->tsgl = NULL;
+	areq->tsgl_entries = 0;
 
-	while (iov_iter_count(&msg->msg_iter)) {
-		struct skcipher_async_rsgl *rsgl;
-		int used;
+	/* convert iovecs of output buffers into RX SGL */
+	while (len < ctx->used && iov_iter_count(&msg->msg_iter)) {
+		struct skcipher_rsgl *rsgl;
+		size_t seglen;
 
 		if (!ctx->used) {
 			err = skcipher_wait_for_data(sk, flags);
 			if (err)
 				goto free;
 		}
-		sgl = list_first_entry(&ctx->tsgl,
-				       struct skcipher_sg_list, list);
-		sg = sgl->sg;
 
-		while (!sg->length)
-			sg++;
-
-		used = min_t(unsigned long, ctx->used,
-			     iov_iter_count(&msg->msg_iter));
-		used = min_t(unsigned long, used, sg->length);
-
-		if (txbufs == tx_nents) {
-			struct scatterlist *tmp;
-			int x;
-			/* Ran out of tx slots in async request
-			 * need to expand */
-			tmp = kcalloc(tx_nents * 2, sizeof(*tmp),
-				      GFP_KERNEL);
-			if (!tmp) {
-				err = -ENOMEM;
-				goto free;
-			}
+		seglen = min_t(size_t, ctx->used,
+			       iov_iter_count(&msg->msg_iter));
 
-			sg_init_table(tmp, tx_nents * 2);
-			for (x = 0; x < tx_nents; x++)
-				sg_set_page(&tmp[x], sg_page(&sreq->tsg[x]),
-					    sreq->tsg[x].length,
-					    sreq->tsg[x].offset);
-			kfree(sreq->tsg);
-			sreq->tsg = tmp;
-			tx_nents *= 2;
-			mark = true;
-		}
-		/* Need to take over the tx sgl from ctx
-		 * to the asynch req - these sgls will be freed later */
-		sg_set_page(sreq->tsg + txbufs++, sg_page(sg), sg->length,
-			    sg->offset);
-
-		if (list_empty(&sreq->list)) {
-			rsgl = &sreq->first_sgl;
-			list_add_tail(&rsgl->list, &sreq->list);
+		if (list_empty(&areq->rsgl_list)) {
+			rsgl = &areq->first_sgl;
 		} else {
-			rsgl = kmalloc(sizeof(*rsgl), GFP_KERNEL);
+			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
 			if (!rsgl) {
 				err = -ENOMEM;
 				goto free;
 			}
-			list_add_tail(&rsgl->list, &sreq->list);
 		}
 
-		used = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, used);
-		err = used;
-		if (used < 0)
+		rsgl->sgl.npages = 0;
+		list_add_tail(&rsgl->list, &areq->rsgl_list);
+
+		/* make one iovec available as scatterlist */
+		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
+		if (err < 0)
 			goto free;
+
+		/* chain the new scatterlist with previous one */
 		if (last_rsgl)
 			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
 
 		last_rsgl = rsgl;
-		len += used;
-		skcipher_pull_sgl(sk, used, 0);
-		iov_iter_advance(&msg->msg_iter, used);
+		len += err;
+		iov_iter_advance(&msg->msg_iter, err);
 	}
 
-	if (mark)
-		sg_mark_end(sreq->tsg + txbufs - 1);
+	/* Process only as much RX buffers for which we have TX data */
+	if (len > ctx->used)
+		len = ctx->used;
+
+	/*
+	 * If more buffers are to be expected to be processed, process only
+	 * full block size buffers.
+	 */
+	if (ctx->more || len < ctx->used)
+		len -= len % bs;
+
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
+	areq->tsgl_entries = skcipher_count_tsgl(sk, len);
+	if (!areq->tsgl_entries)
+		areq->tsgl_entries = 1;
+	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
+				  GFP_KERNEL);
+	if (!areq->tsgl) {
+		err = -ENOMEM;
+		goto free;
+	}
+	sg_init_table(areq->tsgl, areq->tsgl_entries);
+	skcipher_pull_tsgl(sk, len, areq->tsgl);
+
+	/* Initialize the crypto operation */
+	skcipher_request_set_tfm(&areq->req, tfm);
+	skcipher_request_set_crypt(&areq->req, areq->tsgl,
+				   areq->first_sgl.sgl.sg, len, ctx->iv);
+
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+		/* AIO operation */
+		areq->iocb = msg->msg_iocb;
+		skcipher_request_set_callback(&areq->req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP,
+					      skcipher_async_cb, areq);
+		err = ctx->enc ? crypto_skcipher_encrypt(&areq->req) :
+				 crypto_skcipher_decrypt(&areq->req);
+	} else {
+		/* Synchronous operation */
+		skcipher_request_set_callback(&areq->req,
+					      CRYPTO_TFM_REQ_MAY_SLEEP |
+					      CRYPTO_TFM_REQ_MAY_BACKLOG,
+					      af_alg_complete,
+					      &ctx->completion);
+		err = af_alg_wait_for_completion(ctx->enc ?
+					crypto_skcipher_encrypt(&areq->req) :
+					crypto_skcipher_decrypt(&areq->req),
+						 &ctx->completion);
+	}
 
-	skcipher_request_set_crypt(req, sreq->tsg, sreq->first_sgl.sgl.sg,
-				   len, iv);
-	err = ctx->enc ? crypto_skcipher_encrypt(req) :
-			 crypto_skcipher_decrypt(req);
+	/* AIO operation in progress */
 	if (err == -EINPROGRESS) {
-		atomic_inc(&ctx->inflight);
+		sock_hold(sk);
 		err = -EIOCBQUEUED;
-		sreq = NULL;
+		ctx->inflight++;
 		goto unlock;
 	}
-free:
-	skcipher_free_async_sgls(sreq);
-unlock:
-	skcipher_wmem_wakeup(sk);
-	release_sock(sk);
-	kzfree(sreq);
-out:
-	return err;
-}
-
-static int skcipher_recvmsg_sync(struct socket *sock, struct msghdr *msg,
-				 int flags)
-{
-	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;
-	unsigned bs = crypto_skcipher_blocksize(tfm);
-	struct skcipher_sg_list *sgl;
-	struct scatterlist *sg;
-	int err = -EAGAIN;
-	int used;
-	long copied = 0;
-
-	lock_sock(sk);
-	while (msg_data_left(msg)) {
-		if (!ctx->used) {
-			err = skcipher_wait_for_data(sk, flags);
-			if (err)
-				goto unlock;
-		}
-
-		used = min_t(unsigned long, ctx->used, msg_data_left(msg));
-
-		used = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, used);
-		err = used;
-		if (err < 0)
-			goto unlock;
-
-		if (ctx->more || used < ctx->used)
-			used -= used % bs;
-
-		err = -EINVAL;
-		if (!used)
-			goto free;
-
-		sgl = list_first_entry(&ctx->tsgl,
-				       struct skcipher_sg_list, list);
-		sg = sgl->sg;
-
-		while (!sg->length)
-			sg++;
-
-		skcipher_request_set_crypt(&ctx->req, sg, ctx->rsgl.sg, used,
-					   ctx->iv);
-
-		err = af_alg_wait_for_completion(
-				ctx->enc ?
-					crypto_skcipher_encrypt(&ctx->req) :
-					crypto_skcipher_decrypt(&ctx->req),
-				&ctx->completion);
 
 free:
-		af_alg_free_sg(&ctx->rsgl);
-
-		if (err)
-			goto unlock;
-
-		copied += used;
-		skcipher_pull_sgl(sk, used, 1);
-		iov_iter_advance(&msg->msg_iter, used);
-	}
-
-	err = 0;
+	skcipher_free_areq_sgls(areq);
+	if (areq)
+		sock_kfree_s(sk, areq, areqlen);
 
 unlock:
 	skcipher_wmem_wakeup(sk);
 	release_sock(sk);
-
-	return copied ?: err;
-}
-
-static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
-			    size_t ignored, int flags)
-{
-	return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
-		skcipher_recvmsg_async(sock, msg, flags) :
-		skcipher_recvmsg_sync(sock, msg, flags);
+	return err ? err : len;
 }
 
 static unsigned int skcipher_poll(struct file *file, struct socket *sock,
@@ -723,10 +700,9 @@ static unsigned int skcipher_poll(struct file *file, struct socket *sock,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	unsigned int mask;
+	unsigned int mask = 0;
 
 	sock_poll_wait(file, sk_sleep(sk), wait);
-	mask = 0;
 
 	if (ctx->used)
 		mask |= POLLIN | POLLRDNORM;
@@ -894,26 +870,20 @@ static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
 	return err;
 }
 
-static void skcipher_wait(struct sock *sk)
-{
-	struct alg_sock *ask = alg_sk(sk);
-	struct skcipher_ctx *ctx = ask->private;
-	int ctr = 0;
-
-	while (atomic_read(&ctx->inflight) && ctr++ < 100)
-		msleep(100);
-}
-
 static void skcipher_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_ctx *ctx = ask->private;
-	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(&ctx->req);
+	struct sock *psk = ask->parent;
+	struct alg_sock *pask = alg_sk(psk);
+	struct skcipher_tfm *skc = pask->private;
+	struct crypto_skcipher *tfm = skc->skcipher;
 
-	if (atomic_read(&ctx->inflight))
-		skcipher_wait(sk);
+	/* Suspend caller if AIO operations are in flight. */
+	wait_event_interruptible(skcipher_aio_finish_wait,
+				 (ctx->inflight == 0));
 
-	skcipher_free_sgl(sk);
+	skcipher_pull_tsgl(sk, ctx->used, NULL);
 	sock_kzfree_s(sk, ctx->iv, crypto_skcipher_ivsize(tfm));
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
@@ -925,7 +895,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 	struct alg_sock *ask = alg_sk(sk);
 	struct skcipher_tfm *tfm = private;
 	struct crypto_skcipher *skcipher = tfm->skcipher;
-	unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(skcipher);
+	unsigned int len = sizeof(*ctx);
 
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
 	if (!ctx)
@@ -940,22 +910,17 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 
 	memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
 
-	INIT_LIST_HEAD(&ctx->tsgl);
+	INIT_LIST_HEAD(&ctx->tsgl_list);
 	ctx->len = len;
 	ctx->used = 0;
+	ctx->inflight = 0;
 	ctx->more = 0;
 	ctx->merge = 0;
 	ctx->enc = 0;
-	atomic_set(&ctx->inflight, 0);
 	af_alg_init_completion(&ctx->completion);
 
 	ask->private = ctx;
 
-	skcipher_request_set_tfm(&ctx->req, skcipher);
-	skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_SLEEP |
-						 CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      af_alg_complete, &ctx->completion);
-
 	sk->sk_destruct = skcipher_sock_destruct;
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v4 2/2] crypto: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-16 21:22 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <1781086.PMKVUsvYzT@positron.chronox.de>

The updated memory management is described in the top part of the code.
As one benefit of the changed memory management, the AIO and synchronous
operation is now implemented in one common function. The AF_ALG
operation uses the async kernel crypto API interface for each cipher
operation. Thus, the only difference between the AIO and sync operation
types visible from user space is:

1. the callback function to be invoked when the asynchronous operation
   is completed

2. whether to wait for the completion of the kernel crypto API operation
   or not

The change includes the overhaul of the TX and RX SGL handling. The TX
SGL holding the data sent from user space to the kernel is now dynamic
similar to algif_skcipher. This dynamic nature allows a continuous
operation of a thread sending data and a second thread receiving the
data. These threads do not need to synchronize as the kernel processes
as much data from the TX SGL to fill the RX SGL.

The caller reading the data from the kernel defines the amount of data
to be processed. Considering that the interface covers AEAD
authenticating ciphers, the reader must provide the buffer in the
correct size. Thus the reader defines the encryption size.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_aead.c | 705 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 389 insertions(+), 316 deletions(-)

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 533265f..0a00639 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -5,12 +5,26 @@
  *
  * This file provides the user-space API for AEAD ciphers.
  *
- * This file is derived from algif_skcipher.c.
- *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free
  * Software Foundation; either version 2 of the License, or (at your option)
  * any later version.
+ *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. As part of the recvmsg operation, the processed
+ * TX buffers are extracted from the TX SGL into a separate SGL.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The extracted TX SGL parts are released together with
+ * the RX SGL release.
  */
 
 #include <crypto/internal/aead.h>
@@ -24,45 +38,57 @@
 #include <linux/net.h>
 #include <net/sock.h>
 
-struct aead_sg_list {
-	unsigned int cur;
-	struct scatterlist sg[ALG_MAX_PAGES];
+struct aead_tsgl {
+	struct list_head list;
+	unsigned int cur;		/* Last processed SG entry */
+	struct scatterlist sg[0];	/* Array of SGs forming the SGL */
 };
 
-struct aead_async_rsgl {
+struct aead_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 sock *sk;
+
+	struct aead_rsgl first_rsgl;	/* First RX SG */
+	struct list_head rsgl_list;	/* Track RX SGs */
+
+	struct scatterlist *tsgl;	/* priv. TX SGL of buffers to process */
+	unsigned int tsgl_entries;	/* number of entries in priv. TX SGL */
+
+	unsigned int outlen;		/* Filled output buf length */
+
+	unsigned int areqlen;		/* Length of this data struct */
+	struct aead_request aead_req;	/* req ctx trails this struct */
 };
 
 struct aead_ctx {
-	struct aead_sg_list tsgl;
-	struct aead_async_rsgl first_rsgl;
-	struct list_head list;
+	struct list_head tsgl_list;	/* Link to TX SGL */
 
 	void *iv;
+	size_t aead_assoclen;
 
-	struct af_alg_completion completion;
+	struct af_alg_completion completion;	/* sync work queue */
 
-	unsigned long used;
+	unsigned int inflight;	/* Outstanding AIO ops */
+	size_t used;		/* TX bytes sent to kernel */
 
-	unsigned int len;
-	bool more;
-	bool merge;
-	bool enc;
+	bool more;		/* More data to be expected? */
+	bool merge;		/* Merge new data into existing SG */
+	bool enc;		/* Crypto operation: enc, dec */
 
-	size_t aead_assoclen;
-	struct aead_request aead_req;
+	unsigned int len;	/* Length of allocated memory for this struct */
+	struct crypto_aead *aead_tfm;
 };
 
+static DECLARE_WAIT_QUEUE_HEAD(aead_aio_finish_wait);
+
+#define MAX_SGL_ENTS ((4096 - sizeof(struct aead_tsgl)) / \
+		      sizeof(struct scatterlist) - 1)
+
 static inline int aead_sndbuf(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
@@ -79,7 +105,7 @@ static inline bool aead_writable(struct sock *sk)
 
 static inline bool aead_sufficient_data(struct aead_ctx *ctx)
 {
-	unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
+	unsigned int as = crypto_aead_authsize(ctx->aead_tfm);
 
 	/*
 	 * The minimum amount of memory needed for an AEAD cipher is
@@ -88,33 +114,166 @@ static inline bool aead_sufficient_data(struct aead_ctx *ctx)
 	return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as);
 }
 
-static void aead_reset_ctx(struct aead_ctx *ctx)
+static int aead_alloc_tsgl(struct sock *sk)
 {
-	struct aead_sg_list *sgl = &ctx->tsgl;
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct aead_tsgl *sgl;
+	struct scatterlist *sg = NULL;
 
-	sg_init_table(sgl->sg, ALG_MAX_PAGES);
-	sgl->cur = 0;
-	ctx->used = 0;
-	ctx->more = 0;
-	ctx->merge = 0;
+	sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl, list);
+	if (!list_empty(&ctx->tsgl_list))
+		sg = sgl->sg;
+
+	if (!sg || sgl->cur >= MAX_SGL_ENTS) {
+		sgl = sock_kmalloc(sk, sizeof(*sgl) +
+				       sizeof(sgl->sg[0]) * (MAX_SGL_ENTS + 1),
+				   GFP_KERNEL);
+		if (!sgl)
+			return -ENOMEM;
+
+		sg_init_table(sgl->sg, MAX_SGL_ENTS + 1);
+		sgl->cur = 0;
+
+		if (sg)
+			sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
+
+		list_add_tail(&sgl->list, &ctx->tsgl_list);
+	}
+
+	return 0;
+}
+
+static unsigned int aead_count_tsgl(struct sock *sk, size_t bytes)
+{
+	struct alg_sock *ask = alg_sk(sk);
+	struct aead_ctx *ctx = ask->private;
+	struct aead_tsgl *sgl;
+	struct scatterlist *sg;
+	unsigned int i;
+	unsigned int sgl_count = 0;
+
+	if (!bytes)
+		return 0;
+
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct aead_tsgl,
+				       list);
+		sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			sgl_count++;
+			if (sg[i].length >= bytes)
+				return sgl_count;
+
+			bytes -= sg[i].length;
+		}
+	}
+
+	return sgl_count;
 }
 
-static void aead_put_sgl(struct sock *sk)
+static void aead_pull_tsgl(struct sock *sk, size_t used,
+			   struct scatterlist *dst)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	struct aead_sg_list *sgl = &ctx->tsgl;
-	struct scatterlist *sg = sgl->sg;
+	struct aead_tsgl *sgl;
+	struct scatterlist *sg;
 	unsigned int i;
 
-	for (i = 0; i < sgl->cur; i++) {
-		if (!sg_page(sg + i))
+	while (!list_empty(&ctx->tsgl_list)) {
+		sgl = list_first_entry(&ctx->tsgl_list, struct aead_tsgl,
+				       list);
+		sg = sgl->sg;
+
+		for (i = 0; i < sgl->cur; i++) {
+			size_t plen = min_t(size_t, used, sg[i].length);
+			struct page *page = sg_page(sg + i);
+
+			if (!page)
+				continue;
+
+			/*
+			 * Assumption: caller created aead_count_tsgl(len)
+			 * SG entries in dst.
+			 */
+			if (dst)
+				sg_set_page(dst + i, page, plen, sg[i].offset);
+
+			sg[i].length -= plen;
+			sg[i].offset += plen;
+
+			used -= plen;
+			ctx->used -= plen;
+
+			if (sg[i].length)
+				return;
+
+			if (!dst)
+				put_page(page);
+			sg_assign_page(sg + i, NULL);
+		}
+
+		list_del(&sgl->list);
+		sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+						     (MAX_SGL_ENTS + 1));
+	}
+
+	if (!ctx->used)
+		ctx->merge = 0;
+}
+
+static void aead_free_areq_sgls(struct aead_async_req *areq)
+{
+	struct sock *sk = areq->sk;
+	struct aead_rsgl *rsgl, *tmp;
+	struct scatterlist *tsgl;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+		af_alg_free_sg(&rsgl->sgl);
+		list_del(&rsgl->list);
+		if (rsgl != &areq->first_rsgl)
+			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+	}
+
+	tsgl = areq->tsgl;
+	for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
+		if (!sg_page(sg))
 			continue;
+		put_page(sg_page(sg));
+	}
+
+	if (areq->tsgl && areq->tsgl_entries)
+		sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
+}
 
-		put_page(sg_page(sg + i));
-		sg_assign_page(sg + i, NULL);
+static int aead_wait_for_wmem(struct sock *sk, unsigned flags)
+{
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	int err = -ERESTARTSYS;
+	long timeout;
+
+	if (flags & MSG_DONTWAIT)
+		return -EAGAIN;
+
+	sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+
+	add_wait_queue(sk_sleep(sk), &wait);
+	for (;;) {
+		if (signal_pending(current))
+			break;
+		timeout = MAX_SCHEDULE_TIMEOUT;
+		if (sk_wait_event(sk, &timeout, aead_writable(sk), &wait)) {
+			err = 0;
+			break;
+		}
 	}
-	aead_reset_ctx(ctx);
+	remove_wait_queue(sk_sleep(sk), &wait);
+
+	return err;
 }
 
 static void aead_wmem_wakeup(struct sock *sk)
@@ -146,6 +305,7 @@ static int aead_wait_for_data(struct sock *sk, unsigned flags)
 		return -EAGAIN;
 
 	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+
 	add_wait_queue(sk_sleep(sk), &wait);
 	for (;;) {
 		if (signal_pending(current))
@@ -169,8 +329,6 @@ static void aead_data_wakeup(struct sock *sk)
 	struct aead_ctx *ctx = ask->private;
 	struct socket_wq *wq;
 
-	if (ctx->more)
-		return;
 	if (!ctx->used)
 		return;
 
@@ -189,14 +347,13 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	unsigned ivsize =
-		crypto_aead_ivsize(crypto_aead_reqtfm(&ctx->aead_req));
-	struct aead_sg_list *sgl = &ctx->tsgl;
+	unsigned int ivsize = crypto_aead_ivsize(ctx->aead_tfm);
+	struct aead_tsgl *sgl;
 	struct af_alg_control con = {};
 	long copied = 0;
 	bool enc = 0;
 	bool init = 0;
-	int err = -EINVAL;
+	int err = 0;
 
 	if (msg->msg_controllen) {
 		err = af_alg_cmsg_send(msg, &con);
@@ -220,8 +377,10 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	}
 
 	lock_sock(sk);
-	if (!ctx->more && ctx->used)
+	if (!ctx->more && ctx->used) {
+		err = -EINVAL;
 		goto unlock;
+	}
 
 	if (init) {
 		ctx->enc = enc;
@@ -232,11 +391,14 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	}
 
 	while (size) {
+		struct scatterlist *sg;
 		size_t len = size;
-		struct scatterlist *sg = NULL;
+		size_t plen;
 
 		/* use the existing memory in an allocated page */
 		if (ctx->merge) {
+			sgl = list_entry(ctx->tsgl_list.prev,
+					 struct aead_tsgl, list);
 			sg = sgl->sg + sgl->cur - 1;
 			len = min_t(unsigned long, len,
 				    PAGE_SIZE - sg->offset - sg->length);
@@ -257,57 +419,60 @@ 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);
-			err = -EMSGSIZE;
-			goto unlock;
+			err = aead_wait_for_wmem(sk, msg->msg_flags);
+			if (err)
+				goto unlock;
 		}
 
 		/* allocate a new page */
 		len = min_t(unsigned long, size, aead_sndbuf(sk));
-		while (len) {
-			size_t plen = 0;
 
-			if (sgl->cur >= ALG_MAX_PAGES) {
-				aead_put_sgl(sk);
-				err = -E2BIG;
-				goto unlock;
-			}
+		err = aead_alloc_tsgl(sk);
+		if (err)
+			goto unlock;
+
+		sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl,
+				 list);
+		sg = sgl->sg;
+		if (sgl->cur)
+			sg_unmark_end(sg + sgl->cur - 1);
+
+		do {
+			unsigned int i = sgl->cur;
 
-			sg = sgl->sg + sgl->cur;
 			plen = min_t(size_t, len, PAGE_SIZE);
 
-			sg_assign_page(sg, alloc_page(GFP_KERNEL));
-			err = -ENOMEM;
-			if (!sg_page(sg))
+			sg_assign_page(sg + i, alloc_page(GFP_KERNEL));
+			if (!sg_page(sg + i)) {
+				err = -ENOMEM;
 				goto unlock;
+			}
 
-			err = memcpy_from_msg(page_address(sg_page(sg)),
+			err = memcpy_from_msg(page_address(sg_page(sg + i)),
 					      msg, plen);
 			if (err) {
-				__free_page(sg_page(sg));
-				sg_assign_page(sg, NULL);
+				__free_page(sg_page(sg + i));
+				sg_assign_page(sg + i, NULL);
 				goto unlock;
 			}
 
-			sg->offset = 0;
-			sg->length = plen;
+			sg[i].length = plen;
 			len -= plen;
 			ctx->used += plen;
 			copied += plen;
-			sgl->cur++;
 			size -= plen;
-			ctx->merge = plen & (PAGE_SIZE - 1);
-		}
+			sgl->cur++;
+		} while (len && sgl->cur < MAX_SGL_ENTS);
+
+		if (!size)
+			sg_mark_end(sg + sgl->cur - 1);
+
+		ctx->merge = plen & (PAGE_SIZE - 1);
 	}
 
 	err = 0;
 
 	ctx->more = msg->msg_flags & MSG_MORE;
-	if (!ctx->more && !aead_sufficient_data(ctx)) {
-		aead_put_sgl(sk);
-		err = -EMSGSIZE;
-	}
 
 unlock:
 	aead_data_wakeup(sk);
@@ -322,15 +487,12 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	struct aead_sg_list *sgl = &ctx->tsgl;
+	struct aead_tsgl *sgl;
 	int err = -EINVAL;
 
 	if (flags & MSG_SENDPAGE_NOTLAST)
 		flags |= MSG_MORE;
 
-	if (sgl->cur >= ALG_MAX_PAGES)
-		return -E2BIG;
-
 	lock_sock(sk);
 	if (!ctx->more && ctx->used)
 		goto unlock;
@@ -339,13 +501,22 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 		goto done;
 
 	if (!aead_writable(sk)) {
-		/* user space sent too much data */
-		aead_put_sgl(sk);
-		err = -EMSGSIZE;
-		goto unlock;
+		err = aead_wait_for_wmem(sk, flags);
+		if (err)
+			goto unlock;
 	}
 
+	err = aead_alloc_tsgl(sk);
+	if (err)
+		goto unlock;
+
 	ctx->merge = 0;
+	sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl, list);
+
+	if (sgl->cur)
+		sg_unmark_end(sgl->sg + sgl->cur - 1);
+
+	sg_mark_end(sgl->sg + sgl->cur);
 
 	get_page(page);
 	sg_set_page(sgl->sg + sgl->cur, page, size, offset);
@@ -356,11 +527,6 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 
 done:
 	ctx->more = flags & MSG_MORE;
-	if (!ctx->more && !aead_sufficient_data(ctx)) {
-		aead_put_sgl(sk);
-		err = -EMSGSIZE;
-	}
-
 unlock:
 	aead_data_wakeup(sk);
 	release_sock(sk);
@@ -368,205 +534,58 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
 	return err ?: size;
 }
 
-#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 aead_async_req *areq = _req->data;
+	struct sock *sk = areq->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_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 = 0;
-	size_t usedpages = 0;
+	unsigned int resultlen;
 
 	lock_sock(sk);
-	if (ctx->more) {
-		err = aead_wait_for_data(sk, flags);
-		if (err)
-			goto unlock;
-	}
-
-	if (!aead_sufficient_data(ctx))
-		goto unlock;
-
-	used = ctx->used;
-	if (ctx->enc)
-		outlen = used + as;
-	else
-		outlen = used - as;
-
-	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;
-
-	/* take over all tx sgls from ctx */
-	areq->tsgl = sock_kmalloc(sk,
-				  sizeof(*areq->tsgl) * max_t(u32, sgl->cur, 1),
-				  GFP_KERNEL);
-	if (unlikely(!areq->tsgl))
-		goto free;
-
-	sg_init_table(areq->tsgl, max_t(u32, sgl->cur, 1));
-	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 (outlen > usedpages && 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);
+	BUG_ON(!ctx->inflight);
 
-		/* make one iovec available as scatterlist */
-		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
-		if (err < 0)
-			goto free;
-
-		usedpages += err;
+	/* Buffer size written by crypto operation. */
+	resultlen = areq->outlen;
 
-		/* chain the new scatterlist with previous one */
-		if (last_rsgl)
-			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
-
-		last_rsgl = rsgl;
-
-		iov_iter_advance(&msg->msg_iter, err);
-	}
-
-	/* ensure output buffer is sufficiently large */
-	if (usedpages < outlen) {
-		err = -EINVAL;
-		goto unlock;
-	}
+	aead_free_areq_sgls(areq);
+	sock_kfree_s(sk, areq, areq->areqlen);
+	__sock_put(sk);
+	ctx->inflight--;
 
-	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);
-		}
-		goto free;
-	}
-	aead_put_sgl(sk);
+	iocb->ki_complete(iocb, err ? err : resultlen, 0);
 
-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;
+
+	wake_up_interruptible(&aead_aio_finish_wait);
 }
 
-static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
+static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
+			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;
-	struct aead_async_rsgl *last_rsgl = NULL;
-	struct aead_async_rsgl *rsgl, *tmp;
+	struct crypto_aead *tfm = ctx->aead_tfm;
+	unsigned int as = crypto_aead_authsize(tfm);
+	unsigned int areqlen =
+		sizeof(struct aead_async_req) + crypto_aead_reqsize(tfm);
+	struct aead_async_req *areq;
+	struct aead_rsgl *last_rsgl = NULL;
 	int err = -EINVAL;
-	unsigned long used = 0;
-	size_t outlen = 0;
-	size_t usedpages = 0;
+	size_t used = 0;		/* [in]  TX bufs to be en/decrypted */
+	size_t outlen = 0;		/* [out] RX bufs produced by kernel */
+	size_t usedpages = 0;		/* [in]  RX bufs to be used from user */
+	size_t processed = 0;		/* [in]  TX bufs to be consumed */
 
 	lock_sock(sk);
 
 	/*
-	 * Please see documentation of aead_request_set_crypt for the
-	 * description of the AEAD memory structure expected from the caller.
+	 * Data length provided by caller via sendmsg/sendpage that has not
+	 * yet been processed.
 	 */
-
-	if (ctx->more) {
-		err = aead_wait_for_data(sk, flags);
-		if (err)
-			goto unlock;
-	}
-
-	/* data length provided by caller via sendmsg/sendpage */
 	used = ctx->used;
 
 	/*
@@ -600,96 +619,151 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
 	 */
 	used -= ctx->aead_assoclen;
 
-	/* convert iovecs of output buffers into scatterlists */
+	/* Allocate cipher request for current operation. */
+	areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+	if (unlikely(!areq)) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+	areq->areqlen = areqlen;
+	areq->sk = sk;
+	INIT_LIST_HEAD(&areq->rsgl_list);
+	areq->tsgl = NULL;
+	areq->tsgl_entries = 0;
+
+	/* convert iovecs of output buffers into RX SGL */
 	while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
-		size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
-				      (outlen - usedpages));
+		struct aead_rsgl *rsgl;
+		size_t seglen;
+
+		if (!ctx->used) {
+			err = aead_wait_for_data(sk, flags);
+			if (err)
+				goto free;
+		}
+
+		seglen = min_t(size_t, (outlen - usedpages),
+			       iov_iter_count(&msg->msg_iter));
 
-		if (list_empty(&ctx->list)) {
-			rsgl = &ctx->first_rsgl;
+		if (list_empty(&areq->rsgl_list)) {
+			rsgl = &areq->first_rsgl;
 		} else {
 			rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
 			if (unlikely(!rsgl)) {
 				err = -ENOMEM;
-				goto unlock;
+				goto free;
 			}
 		}
+
 		rsgl->sgl.npages = 0;
-		list_add_tail(&rsgl->list, &ctx->list);
+		list_add_tail(&rsgl->list, &areq->rsgl_list);
 
 		/* make one iovec available as scatterlist */
 		err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
 		if (err < 0)
-			goto unlock;
-		usedpages += err;
+			goto free;
+
 		/* chain the new scatterlist with previous one */
 		if (last_rsgl)
 			af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
 
 		last_rsgl = rsgl;
-
+		usedpages += err;
 		iov_iter_advance(&msg->msg_iter, err);
 	}
 
-	/* ensure output buffer is sufficiently large */
+	/*
+	 * Ensure output buffer is sufficiently large. If the caller provides
+	 * less buffer space, only use the relative required input size. This
+	 * allows AIO operation where the caller sent all data to be processed
+	 * and the AIO operation performs the operation on the different chunks
+	 * of the input data.
+	 */
 	if (usedpages < outlen) {
-		err = -EINVAL;
-		goto unlock;
-	}
+		size_t less = outlen - usedpages;
 
-	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);
-	aead_request_set_ad(&ctx->aead_req, ctx->aead_assoclen);
+		if (used < less) {
+			err = -EINVAL;
+			goto free;
+		}
+		used -= less;
+		outlen -= less;
+	}
 
-	err = af_alg_wait_for_completion(ctx->enc ?
-					 crypto_aead_encrypt(&ctx->aead_req) :
-					 crypto_aead_decrypt(&ctx->aead_req),
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
+	processed = used + ctx->aead_assoclen;
+	areq->tsgl_entries = aead_count_tsgl(sk, processed);
+	if (!areq->tsgl_entries)
+		areq->tsgl_entries = 1;
+	areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * areq->tsgl_entries,
+				  GFP_KERNEL);
+	if (!areq->tsgl) {
+		err = -ENOMEM;
+		goto free;
+	}
+	sg_init_table(areq->tsgl, areq->tsgl_entries);
+	aead_pull_tsgl(sk, processed, areq->tsgl);
+
+	/* Initialize the crypto operation */
+	aead_request_set_crypt(&areq->aead_req, areq->tsgl,
+			       areq->first_rsgl.sgl.sg, used, ctx->iv);
+	aead_request_set_ad(&areq->aead_req, ctx->aead_assoclen);
+	aead_request_set_tfm(&areq->aead_req, tfm);
+
+	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+		/* AIO operation */
+		areq->iocb = msg->msg_iocb;
+		aead_request_set_callback(&areq->aead_req,
+					  CRYPTO_TFM_REQ_MAY_BACKLOG,
+					  aead_async_cb, areq);
+		err = ctx->enc ? crypto_aead_encrypt(&areq->aead_req) :
+				 crypto_aead_decrypt(&areq->aead_req);
+	} else {
+		/* Synchronous operation */
+		aead_request_set_callback(&areq->aead_req,
+					  CRYPTO_TFM_REQ_MAY_BACKLOG,
+					  af_alg_complete, &ctx->completion);
+		err = af_alg_wait_for_completion(ctx->enc ?
+					 crypto_aead_encrypt(&areq->aead_req) :
+					 crypto_aead_decrypt(&areq->aead_req),
 					 &ctx->completion);
+	}
 
-	if (err) {
-		/* EBADMSG implies a valid cipher operation took place */
-		if (err == -EBADMSG)
-			aead_put_sgl(sk);
+	/* AIO operation in progress */
+	if (err == -EINPROGRESS) {
+		sock_hold(sk);
+		err = -EIOCBQUEUED;
+		ctx->inflight++;
+
+		/* Remember output size that will be generated. */
+		areq->outlen = outlen;
 
 		goto unlock;
 	}
 
-	aead_put_sgl(sk);
-	err = 0;
+free:
+	aead_free_areq_sgls(areq);
+	if (areq)
+		sock_kfree_s(sk, areq, areqlen);
 
 unlock:
-	list_for_each_entry_safe(rsgl, tmp, &ctx->list, list) {
-		af_alg_free_sg(&rsgl->sgl);
-		list_del(&rsgl->list);
-		if (rsgl != &ctx->first_rsgl)
-			sock_kfree_s(sk, rsgl, sizeof(*rsgl));
-	}
-	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)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	unsigned int mask;
+	unsigned int mask = 0;
 
 	sock_poll_wait(file, sk_sleep(sk), wait);
-	mask = 0;
 
 	if (!ctx->more)
 		mask |= POLLIN | POLLRDNORM;
@@ -746,11 +820,14 @@ static void aead_sock_destruct(struct sock *sk)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct aead_ctx *ctx = ask->private;
-	unsigned int ivlen = crypto_aead_ivsize(
-				crypto_aead_reqtfm(&ctx->aead_req));
+	unsigned int ivlen = crypto_aead_ivsize(ctx->aead_tfm);
 
 	WARN_ON(atomic_read(&sk->sk_refcnt) != 0);
-	aead_put_sgl(sk);
+
+	/* Suspend caller if AIO operations are in flight. */
+	wait_event_interruptible(aead_aio_finish_wait, (ctx->inflight == 0));
+
+	aead_pull_tsgl(sk, ctx->used, NULL);
 	sock_kzfree_s(sk, ctx->iv, ivlen);
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
@@ -760,7 +837,7 @@ static int aead_accept_parent(void *private, struct sock *sk)
 {
 	struct aead_ctx *ctx;
 	struct alg_sock *ask = alg_sk(sk);
-	unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(private);
+	unsigned int len = sizeof(*ctx);
 	unsigned int ivlen = crypto_aead_ivsize(private);
 
 	ctx = sock_kmalloc(sk, len, GFP_KERNEL);
@@ -775,23 +852,19 @@ static int aead_accept_parent(void *private, struct sock *sk)
 	}
 	memset(ctx->iv, 0, ivlen);
 
+	INIT_LIST_HEAD(&ctx->tsgl_list);
 	ctx->len = len;
 	ctx->used = 0;
 	ctx->more = 0;
 	ctx->merge = 0;
 	ctx->enc = 0;
-	ctx->tsgl.cur = 0;
+	ctx->inflight = 0;
 	ctx->aead_assoclen = 0;
 	af_alg_init_completion(&ctx->completion);
-	sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
-	INIT_LIST_HEAD(&ctx->list);
 
+	ctx->aead_tfm = private;
 	ask->private = ctx;
 
-	aead_request_set_tfm(&ctx->aead_req, private);
-	aead_request_set_callback(&ctx->aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  af_alg_complete, &ctx->completion);
-
 	sk->sk_destruct = aead_sock_destruct;
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox