Skip to contentSkip to conversation

AF_ALG fixes

PATCH In review Eric Biggers 3 participants 13 messages Apr 30

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover AF_ALG fixes

This series backports the recent AF_ALG fixes to 6.12. These include the fix for https://copy.fail/, fixes for that fix, and some other fixes that went in at around the same time that seem related.

To enable the 5 actual fix commits to cherry-pick cleanly, commit 1 copies the latest implementation of memcpy_sglist() from upstream, and commits 2 and 5 cleanly cherry-pick a couple cleanup commits.

I didn't check older kernels yet, but this should be usable as a starting point for them.

Douya Le (1):
crypto: algif_aead - snapshot IV for async AEAD request

Eric Biggers (3):
crypto: scatterwalk - Backport memcpy_sglist()
crypto: algif_aead - use memcpy_sglist() instead of null skcipher
crypto: authenc - use memcpy_sglist() instead of null skcipher

Herbert Xu (4):
crypto: algif_aead - Revert to operating out-of-place
crypto: authencesn - Do not place hiseq at end of dst for out-of-place
decryption
crypto: authencesn - Fix src offset when decrypting in-place
crypto: af_alg - Fix page reassignment overflow in af_alg_pull_tsgl

 crypto/Kconfig               |   2 -
 crypto/af_alg.c              |  51 ++-------
 crypto/algif_aead.c          | 203 +++++++----------------------------
 crypto/algif_skcipher.c      |   6 +-
 crypto/authenc.c             |  32 +-----
 crypto/authencesn.c          |  84 ++++++---------
 crypto/scatterwalk.c         |  94 ++++++++++++++++
 include/crypto/if_alg.h      |   5 +-
 include/crypto/scatterwalk.h |  31 ++++++
 9 files changed, 215 insertions(+), 293 deletions(-)

base-commit: c286ea5e62389897291fa742d2bb909ecc9ef2d0

Signature · 1 line
2.54.0
#1 lore raw

PATCH 1/8 crypto: scatterwalk - Backport memcpy_sglist()

This backports the current implementation of memcpy_sglist() from upstream commit 4dffc9bbffb9ccfcda730d899c97c553599e7ca8.

This function was rewritten twice. The earlier implementations had many prerequisite commits, while the latest implementation is standalone. It's much easier to just backport the latest code directly.

---
 crypto/scatterwalk.c         | 94 ++++++++++++++++++++++++++++++++++++
 include/crypto/scatterwalk.h | 31 ++++++++++++
 2 files changed, 125 insertions(+)
crypto/scatterwalk.c +94 -0
index 16f6ba896fb6..9f0b27005166 100644
@@ -67,10 +67,104 @@ void scatterwalk_map_and_copy(void *buf, struct scatterlist *sg,
 	scatterwalk_copychunks(buf, &walk, nbytes, out);
 	scatterwalk_done(&walk, out, 0);
 }
 EXPORT_SYMBOL_GPL(scatterwalk_map_and_copy);
 
+/**
+ * memcpy_sglist() - Copy data from one scatterlist to another
+ * @dst: The destination scatterlist.  Can be NULL if @nbytes == 0.
+ * @src: The source scatterlist.  Can be NULL if @nbytes == 0.
+ * @nbytes: Number of bytes to copy
+ *
+ * The scatterlists can describe exactly the same memory, in which case this
+ * function is a no-op.  No other overlaps are supported.
+ *
+ * Context: Any context
+ */
+void memcpy_sglist(struct scatterlist *dst, struct scatterlist *src,
+		   unsigned int nbytes)
+{
+	unsigned int src_offset, dst_offset;
+
+	if (unlikely(nbytes == 0)) /* in case src and/or dst is NULL */
+		return;
+
+	src_offset = src->offset;
+	dst_offset = dst->offset;
+	for (;;) {
+		/* Compute the length to copy this step. */
+		unsigned int len = min3(src->offset + src->length - src_offset,
+					dst->offset + dst->length - dst_offset,
+					nbytes);
+		struct page *src_page = sg_page(src);
+		struct page *dst_page = sg_page(dst);
+		const void *src_virt;
+		void *dst_virt;
+
+		if (IS_ENABLED(CONFIG_HIGHMEM)) {
+			/* HIGHMEM: we may have to actually map the pages. */
+			const unsigned int src_oip = offset_in_page(src_offset);
+			const unsigned int dst_oip = offset_in_page(dst_offset);
+			const unsigned int limit = PAGE_SIZE;
+
+			/* Further limit len to not cross a page boundary. */
+			len = min3(len, limit - src_oip, limit - dst_oip);
+
+			/* Compute the source and destination pages. */
+			src_page += src_offset / PAGE_SIZE;
+			dst_page += dst_offset / PAGE_SIZE;
+
+			if (src_page != dst_page) {
+				/* Copy between different pages. */
+				memcpy_page(dst_page, dst_oip,
+					    src_page, src_oip, len);
+				flush_dcache_page(dst_page);
+			} else if (src_oip != dst_oip) {
+				/* Copy between different parts of same page. */
+				dst_virt = kmap_local_page(dst_page);
+				memcpy(dst_virt + dst_oip, dst_virt + src_oip,
+				       len);
+				kunmap_local(dst_virt);
+				flush_dcache_page(dst_page);
+			} /* Else, it's the same memory.  No action needed. */
+		} else {
+			/*
+			 * !HIGHMEM: no mapping needed.  Just work in the linear
+			 * buffer of each sg entry.  Note that we can cross page
+			 * boundaries, as they are not significant in this case.
+			 */
+			src_virt = page_address(src_page) + src_offset;
+			dst_virt = page_address(dst_page) + dst_offset;
+			if (src_virt != dst_virt) {
+				memcpy(dst_virt, src_virt, len);
+				if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE)
+					__scatterwalk_flush_dcache_pages(
+						dst_page, dst_offset, len);
+			} /* Else, it's the same memory.  No action needed. */
+		}
+		nbytes -= len;
+		if (nbytes == 0) /* No more to copy? */
+			break;
+
+		/*
+		 * There's more to copy.  Advance the offsets by the length
+		 * copied this step, and advance the sg entries as needed.
+		 */
+		src_offset += len;
+		if (src_offset >= src->offset + src->length) {
+			src = sg_next(src);
+			src_offset = src->offset;
+		}
+		dst_offset += len;
+		if (dst_offset >= dst->offset + dst->length) {
+			dst = sg_next(dst);
+			dst_offset = dst->offset;
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(memcpy_sglist);
+
 struct scatterlist *scatterwalk_ffwd(struct scatterlist dst[2],
 				     struct scatterlist *src,
 				     unsigned int len)
 {
 	for (;;) {
include/crypto/scatterwalk.h +31 -0
index 32fc4473175b..7e7942950c07 100644
@@ -81,10 +81,38 @@ static inline void scatterwalk_pagedone(struct scatter_walk *walk, int out,
 
 	if (more && walk->offset >= walk->sg->offset + walk->sg->length)
 		scatterwalk_start(walk, sg_next(walk->sg));
 }
 
+/*
+ * Flush the dcache of any pages that overlap the region
+ * [offset, offset + nbytes) relative to base_page.
+ *
+ * This should be called only when ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, to ensure
+ * that all relevant code (including the call to sg_page() in the caller, if
+ * applicable) gets fully optimized out when !ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE.
+ */
+static inline void __scatterwalk_flush_dcache_pages(struct page *base_page,
+						    unsigned int offset,
+						    unsigned int nbytes)
+{
+	unsigned int num_pages;
+
+	base_page += offset / PAGE_SIZE;
+	offset %= PAGE_SIZE;
+
+	/*
+	 * This is an overflow-safe version of
+	 * num_pages = DIV_ROUND_UP(offset + nbytes, PAGE_SIZE).
+	 */
+	num_pages = nbytes / PAGE_SIZE;
+	num_pages += DIV_ROUND_UP(offset + (nbytes % PAGE_SIZE), PAGE_SIZE);
+
+	for (unsigned int i = 0; i < num_pages; i++)
+		flush_dcache_page(base_page + i);
+}
+
 static inline void scatterwalk_done(struct scatter_walk *walk, int out,
 				    int more)
 {
 	if (!more || walk->offset >= walk->sg->offset + walk->sg->length ||
 	    !(walk->offset & (PAGE_SIZE - 1)))
@@ -92,10 +120,13 @@ static inline void scatterwalk_done(struct scatter_walk *walk, int out,
 }
 
 void scatterwalk_copychunks(void *buf, struct scatter_walk *walk,
 			    size_t nbytes, int out);
 
+void memcpy_sglist(struct scatterlist *dst, struct scatterlist *src,
+		   unsigned int nbytes);
+
 void scatterwalk_map_and_copy(void *buf, struct scatterlist *sg,
 			      unsigned int start, unsigned int nbytes, int out);
 
 struct scatterlist *scatterwalk_ffwd(struct scatterlist dst[2],
 				     struct scatterlist *src,
Signature · 1 line
2.54.0
#2 lore raw

PATCH 2/8 crypto: algif_aead - use memcpy_sglist() instead of null skcipher

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

commit f2804d0eee8ddd57aa79d0b82872b74c21e1b69b upstream.

For copying data between two scatterlists, just use memcpy_sglist() instead of the so-called "null skcipher". This is much simpler.

---
 crypto/Kconfig      |   1 -
 crypto/algif_aead.c | 101 ++++++++------------------------------------
 2 files changed, 18 insertions(+), 84 deletions(-)
crypto/Kconfig +0 -1
index e7528986e94f..c2d0622d99b8 100644
@@ -1422,11 +1422,10 @@ config CRYPTO_USER_API_RNG_CAVP
 config CRYPTO_USER_API_AEAD
 	tristate "AEAD cipher algorithms"
 	depends on NET
 	select CRYPTO_AEAD
 	select CRYPTO_SKCIPHER
-	select CRYPTO_NULL
 	select CRYPTO_USER_API
 	help
 	  Enable the userspace interface for AEAD cipher algorithms.
 
 	  See Documentation/crypto/userspace-if.rst and
crypto/algif_aead.c +18 -83

240 lines not on this page · raw message

Signature · 1 line
2.54.0
#3 lore raw

PATCH 3/8 crypto: algif_aead - Revert to operating out-of-place

In reply to Eric Biggers

From: Herbert Xu <[email protected]>

commit a664bf3d603dc3bdcf9ae47cc21e0daec706d7a5 upstream.

This mostly reverts commit 72548b093ee3 except for the copying of the associated data.

There is no benefit in operating in-place in algif_aead since the source and destination come from different mappings. Get rid of all the complexity added for in-place operation and just copy the AD directly.

Fixes 72548b093ee3 ("crypto: algif_aead - copy AAD from src to dst")
---
 crypto/af_alg.c         |  49 ++++----------------
 crypto/algif_aead.c     | 100 ++++++++--------------------------------
 crypto/algif_skcipher.c |   6 +--
 include/crypto/if_alg.h |   5 +-
 4 files changed, 34 insertions(+), 126 deletions(-)
crypto/af_alg.c +10 -39
index 0530dc85e4f8..6867d177f2a2 100644
@@ -635,19 +635,17 @@ static int af_alg_alloc_tsgl(struct sock *sk)
 }
 
 /**
  * af_alg_count_tsgl - Count number of TX SG entries
  *
- * The counting starts from the beginning of the SGL to @bytes. If
- * an @offset is provided, the counting of the SG entries starts at the @offset.
+ * The counting starts from the beginning of the SGL to @bytes.
  *
  * @sk: socket of connection to user space
  * @bytes: Count the number of SG entries holding given number of bytes.
- * @offset: Start the counting of SG entries from the given offset.
  * Return: Number of TX SG entries found given the constraints
  */
-unsigned int af_alg_count_tsgl(struct sock *sk, size_t bytes, size_t offset)
+unsigned int af_alg_count_tsgl(struct sock *sk, size_t bytes)
 {
 	const struct alg_sock *ask = alg_sk(sk);
 	const struct af_alg_ctx *ctx = ask->private;
 	const struct af_alg_tsgl *sgl;
 	unsigned int i;
@@ -658,29 +656,15 @@ unsigned int af_alg_count_tsgl(struct sock *sk, size_t bytes, size_t offset)
 
 	list_for_each_entry(sgl, &ctx->tsgl_list, list) {
 		const struct scatterlist *sg = sgl->sg;
 
 		for (i = 0; i < sgl->cur; i++) {
-			size_t bytes_count;
-
-			/* Skip offset */
-			if (offset >= sg[i].length) {
-				offset -= sg[i].length;
-				bytes -= sg[i].length;
-				continue;
-			}
-
-			bytes_count = sg[i].length - offset;
-
-			offset = 0;
 			sgl_count++;
-
-			/* If we have seen requested number of bytes, stop */
-			if (bytes_count >= bytes)
+			if (sg[i].length >= bytes)
 				return sgl_count;
 
-			bytes -= bytes_count;
+			bytes -= sg[i].length;
 		}
 	}
 
 	return sgl_count;
 }
@@ -688,23 +672,18 @@ EXPORT_SYMBOL_GPL(af_alg_count_tsgl);
 
 /**
  * af_alg_pull_tsgl - Release the specified buffers from TX SGL
  *
  * If @dst is non-null, reassign the pages to @dst. The caller must release
- * the pages. If @dst_offset is given only reassign the pages to @dst starting
- * at the @dst_offset (byte). The caller must ensure that @dst is large
- * enough (e.g. by using af_alg_count_tsgl with the same offset).
+ * the pages.
  *
  * @sk: socket of connection to user space
  * @used: Number of bytes to pull from TX SGL
  * @dst: If non-NULL, buffer is reassigned to dst SGL instead of releasing. The
  *	 caller must release the buffers in dst.
- * @dst_offset: Reassign the TX SGL from given offset. All buffers before
- *	        reaching the offset is released.
  */
-void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst,
-		      size_t dst_offset)
+void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst)
 {
 	struct alg_sock *ask = alg_sk(sk);
 	struct af_alg_ctx *ctx = ask->private;
 	struct af_alg_tsgl *sgl;
 	struct scatterlist *sg;
@@ -725,22 +704,14 @@ void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst,
 			/*
 			 * Assumption: caller created af_alg_count_tsgl(len)
 			 * SG entries in dst.
 			 */
 			if (dst) {
-				if (dst_offset >= plen) {
-					/* discard page before offset */
-					dst_offset -= plen;
-				} else {
-					/* reassign page to dst after offset */
-					get_page(page);
-					sg_set_page(dst + j, page,
-						    plen - dst_offset,
-						    sg[i].offset + dst_offset);
-					dst_offset = 0;
-					j++;
-				}
+				/* reassign page to dst after offset */
+				get_page(page);
+				sg_set_page(dst + j, page, plen, sg[i].offset);
+				j++;
 			}
 
 			sg[i].length -= plen;
 			sg[i].offset += plen;
 
crypto/algif_aead.c +19 -81

159 lines not on this page · raw message

crypto/algif_skcipher.c +3 -3
index 3549ad1cc42e..ba0a17fd95ac 100644
@@ -141,22 +141,22 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
 
 	/*
 	 * Create a per request TX SGL for this request which tracks the
 	 * SG entries from the global TX SGL.
 	 */
-	areq->tsgl_entries = af_alg_count_tsgl(sk, len, 0);
+	areq->tsgl_entries = af_alg_count_tsgl(sk, len);
 	if (!areq->tsgl_entries)
 		areq->tsgl_entries = 1;
 	areq->tsgl = sock_kmalloc(sk, array_size(sizeof(*areq->tsgl),
 						 areq->tsgl_entries),
 				  GFP_KERNEL);
 	if (!areq->tsgl) {
 		err = -ENOMEM;
 		goto free;
 	}
 	sg_init_table(areq->tsgl, areq->tsgl_entries);
-	af_alg_pull_tsgl(sk, len, areq->tsgl, 0);
+	af_alg_pull_tsgl(sk, len, areq->tsgl);
 
 	/* Initialize the crypto operation */
 	skcipher_request_set_tfm(&areq->cra_u.skcipher_req, tfm);
 	skcipher_request_set_crypt(&areq->cra_u.skcipher_req, areq->tsgl,
 				   areq->first_rsgl.sgl.sgt.sgl, len, ctx->iv);
@@ -366,11 +366,11 @@ static void skcipher_sock_destruct(struct sock *sk)
 	struct af_alg_ctx *ctx = ask->private;
 	struct sock *psk = ask->parent;
 	struct alg_sock *pask = alg_sk(psk);
 	struct crypto_skcipher *tfm = pask->private;
 
-	af_alg_pull_tsgl(sk, ctx->used, NULL, 0);
+	af_alg_pull_tsgl(sk, ctx->used, NULL);
 	sock_kzfree_s(sk, ctx->iv, crypto_skcipher_ivsize(tfm));
 	if (ctx->state)
 		sock_kzfree_s(sk, ctx->state, crypto_skcipher_statesize(tfm));
 	sock_kfree_s(sk, ctx, ctx->len);
 	af_alg_release_parent(sk);
include/crypto/if_alg.h +2 -3
index 107b797c33ec..0cc8fa749f68 100644
@@ -228,13 +228,12 @@ static inline int af_alg_rcvbuf(struct sock *sk)
 static inline bool af_alg_readable(struct sock *sk)
 {
 	return PAGE_SIZE <= af_alg_rcvbuf(sk);
 }
 
-unsigned int af_alg_count_tsgl(struct sock *sk, size_t bytes, size_t offset);
-void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst,
-		      size_t dst_offset);
+unsigned int af_alg_count_tsgl(struct sock *sk, size_t bytes);
+void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst);
 void af_alg_wmem_wakeup(struct sock *sk);
 int af_alg_wait_for_data(struct sock *sk, unsigned flags, unsigned min);
 int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
 		   unsigned int ivsize);
 void af_alg_free_resources(struct af_alg_async_req *areq);
Signature · 1 line
2.54.0
#4 lore raw

PATCH 4/8 crypto: algif_aead - snapshot IV for async AEAD requests

In reply to Eric Biggers

From: Douya Le <[email protected]>

commit 5aa58c3a572b3e3b6c786953339f7978b845cc52 upstream.

AF_ALG AEAD AIO requests currently use the socket-wide IV buffer during request processing. For async requests, later socket activity can update that shared state before the original request has fully completed, which can lead to inconsistent IV handling.

Snapshot the IV into per-request storage when preparing the AEAD request, so in-flight operations no longer depend on mutable socket state.

---
 crypto/algif_aead.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
crypto/algif_aead.c +8 -2
index f8bd45f7dc83..cb651ab58d62 100644
@@ -70,12 +70,14 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 	struct sock *psk = ask->parent;
 	struct alg_sock *pask = alg_sk(psk);
 	struct af_alg_ctx *ctx = ask->private;
 	struct crypto_aead *tfm = pask->private;
 	unsigned int as = crypto_aead_authsize(tfm);
+	unsigned int ivsize = crypto_aead_ivsize(tfm);
 	struct af_alg_async_req *areq;
 	struct scatterlist *rsgl_src, *tsgl_src = NULL;
+	void *iv;
 	int err = 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 */
@@ -123,14 +125,18 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 	 */
 	used -= ctx->aead_assoclen;
 
 	/* Allocate cipher request for current operation. */
 	areq = af_alg_alloc_areq(sk, sizeof(struct af_alg_async_req) +
-				     crypto_aead_reqsize(tfm));
+				     crypto_aead_reqsize(tfm) + ivsize);
 	if (IS_ERR(areq))
 		return PTR_ERR(areq);
 
+	iv = (u8 *)aead_request_ctx(&areq->cra_u.aead_req) +
+	     crypto_aead_reqsize(tfm);
+	memcpy(iv, ctx->iv, ivsize);
+
 	/* convert iovecs of output buffers into RX SGL */
 	err = af_alg_get_rsgl(sk, msg, flags, areq, outlen, &usedpages);
 	if (err)
 		goto free;
 
@@ -185,11 +191,11 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 
 	memcpy_sglist(rsgl_src, tsgl_src, ctx->aead_assoclen);
 
 	/* Initialize the crypto operation */
 	aead_request_set_crypt(&areq->cra_u.aead_req, tsgl_src,
-			       areq->first_rsgl.sgl.sgt.sgl, used, ctx->iv);
+			       areq->first_rsgl.sgl.sgt.sgl, used, iv);
 	aead_request_set_ad(&areq->cra_u.aead_req, ctx->aead_assoclen);
 	aead_request_set_tfm(&areq->cra_u.aead_req, tfm);
 
 	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
 		/* AIO operation */
Signature · 1 line
2.54.0
#5 lore raw

PATCH 5/8 crypto: authenc - use memcpy_sglist() instead of null skcipher

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

commit dbc4b1458e931e47198c3165ff5853bc1ad6bd7a upstream.

For copying data between two scatterlists, just use memcpy_sglist() instead of the so-called "null skcipher". This is much simpler.

---
 crypto/Kconfig      |  1 -
 crypto/authenc.c    | 32 +-------------------------------
 crypto/authencesn.c | 38 +++-----------------------------------
 3 files changed, 4 insertions(+), 67 deletions(-)
crypto/Kconfig +0 -1
index c2d0622d99b8..a997e97937e0 100644
@@ -220,11 +220,10 @@ config CRYPTO_AUTHENC
 	tristate "Authenc support"
 	select CRYPTO_AEAD
 	select CRYPTO_SKCIPHER
 	select CRYPTO_MANAGER
 	select CRYPTO_HASH
-	select CRYPTO_NULL
 	help
 	  Authenc: Combined mode wrapper for IPsec.
 
 	  This is required for IPSec ESP (XFRM_ESP).
 
crypto/authenc.c +1 -31
index d04068af9833..f1f7886bb34c 100644
@@ -7,11 +7,10 @@
 
 #include <crypto/internal/aead.h>
 #include <crypto/internal/hash.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/authenc.h>
-#include <crypto/null.h>
 #include <crypto/scatterwalk.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -26,11 +25,10 @@ struct authenc_instance_ctx {
 };
 
 struct crypto_authenc_ctx {
 	struct crypto_ahash *auth;
 	struct crypto_skcipher *enc;
-	struct crypto_sync_skcipher *null;
 };
 
 struct authenc_request_ctx {
 	struct scatterlist src[2];
 	struct scatterlist dst[2];
@@ -184,25 +182,10 @@ static void crypto_authenc_encrypt_done(void *data, int err)
 	}
 	err = crypto_authenc_genicv(areq, CRYPTO_TFM_REQ_MAY_SLEEP);
 	authenc_request_complete(areq, err);
 }
 
-static int crypto_authenc_copy_assoc(struct aead_request *req)
-{
-	struct crypto_aead *authenc = crypto_aead_reqtfm(req);
-	struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
-	SYNC_SKCIPHER_REQUEST_ON_STACK(skreq, ctx->null);
-
-	skcipher_request_set_sync_tfm(skreq, ctx->null);
-	skcipher_request_set_callback(skreq, aead_request_flags(req),
-				      NULL, NULL);
-	skcipher_request_set_crypt(skreq, req->src, req->dst, req->assoclen,
-				   NULL);
-
-	return crypto_skcipher_encrypt(skreq);
-}
-
 static int crypto_authenc_encrypt(struct aead_request *req)
 {
 	struct crypto_aead *authenc = crypto_aead_reqtfm(req);
 	struct aead_instance *inst = aead_alg_instance(authenc);
 	struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
@@ -217,14 +200,11 @@ static int crypto_authenc_encrypt(struct aead_request *req)
 
 	src = scatterwalk_ffwd(areq_ctx->src, req->src, req->assoclen);
 	dst = src;
 
 	if (req->src != req->dst) {
-		err = crypto_authenc_copy_assoc(req);
-		if (err)
-			return err;
-
+		memcpy_sglist(req->dst, req->src, req->assoclen);
 		dst = scatterwalk_ffwd(areq_ctx->dst, req->dst, req->assoclen);
 	}
 
 	skcipher_request_set_tfm(skreq, enc);
 	skcipher_request_set_callback(skreq, aead_request_flags(req),
@@ -326,11 +306,10 @@ static int crypto_authenc_init_tfm(struct crypto_aead *tfm)
 	struct aead_instance *inst = aead_alg_instance(tfm);
 	struct authenc_instance_ctx *ictx = aead_instance_ctx(inst);
 	struct crypto_authenc_ctx *ctx = crypto_aead_ctx(tfm);
 	struct crypto_ahash *auth;
 	struct crypto_skcipher *enc;
-	struct crypto_sync_skcipher *null;
 	int err;
 
 	auth = crypto_spawn_ahash(&ictx->auth);
 	if (IS_ERR(auth))
 		return PTR_ERR(auth);
@@ -338,18 +317,12 @@ static int crypto_authenc_init_tfm(struct crypto_aead *tfm)
 	enc = crypto_spawn_skcipher(&ictx->enc);
 	err = PTR_ERR(enc);
 	if (IS_ERR(enc))
 		goto err_free_ahash;
 
-	null = crypto_get_default_null_skcipher();
-	err = PTR_ERR(null);
-	if (IS_ERR(null))
-		goto err_free_skcipher;
-
 	ctx->auth = auth;
 	ctx->enc = enc;
-	ctx->null = null;
 
 	crypto_aead_set_reqsize(
 		tfm,
 		sizeof(struct authenc_request_ctx) +
 		ictx->reqoff +
@@ -359,12 +332,10 @@ static int crypto_authenc_init_tfm(struct crypto_aead *tfm)
 		      sizeof(struct skcipher_request) +
 		      crypto_skcipher_reqsize(enc)));
 
 	return 0;
 
-err_free_skcipher:
-	crypto_free_skcipher(enc);
 err_free_ahash:
 	crypto_free_ahash(auth);
 	return err;
 }
 
@@ -372,11 +343,10 @@ static void crypto_authenc_exit_tfm(struct crypto_aead *tfm)
 {
 	struct crypto_authenc_ctx *ctx = crypto_aead_ctx(tfm);
 
 	crypto_free_ahash(ctx->auth);
 	crypto_free_skcipher(ctx->enc);
-	crypto_put_default_null_skcipher();
 }
 
 static void crypto_authenc_free(struct aead_instance *inst)
 {
 	struct authenc_instance_ctx *ctx = aead_instance_ctx(inst);
crypto/authencesn.c +3 -35
index e08032e80f18..a5fbb638d9d7 100644
@@ -10,11 +10,10 @@
 
 #include <crypto/internal/aead.h>
 #include <crypto/internal/hash.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/authenc.h>
-#include <crypto/null.h>
 #include <crypto/scatterwalk.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -29,11 +28,10 @@ struct authenc_esn_instance_ctx {
 
 struct crypto_authenc_esn_ctx {
 	unsigned int reqoff;
 	struct crypto_ahash *auth;
 	struct crypto_skcipher *enc;
-	struct crypto_sync_skcipher *null;
 };
 
 struct authenc_esn_request_ctx {
 	struct scatterlist src[2];
 	struct scatterlist dst[2];
@@ -156,24 +154,10 @@ static void crypto_authenc_esn_encrypt_done(void *data, int err)
 		err = crypto_authenc_esn_genicv(areq, 0);
 
 	authenc_esn_request_complete(areq, err);
 }
 
-static int crypto_authenc_esn_copy(struct aead_request *req, unsigned int len)
-{
-	struct crypto_aead *authenc_esn = crypto_aead_reqtfm(req);
-	struct crypto_authenc_esn_ctx *ctx = crypto_aead_ctx(authenc_esn);
-	SYNC_SKCIPHER_REQUEST_ON_STACK(skreq, ctx->null);
-
-	skcipher_request_set_sync_tfm(skreq, ctx->null);
-	skcipher_request_set_callback(skreq, aead_request_flags(req),
-				      NULL, NULL);
-	skcipher_request_set_crypt(skreq, req->src, req->dst, len, NULL);
-
-	return crypto_skcipher_encrypt(skreq);
-}
-
 static int crypto_authenc_esn_encrypt(struct aead_request *req)
 {
 	struct crypto_aead *authenc_esn = crypto_aead_reqtfm(req);
 	struct authenc_esn_request_ctx *areq_ctx = aead_request_ctx(req);
 	struct crypto_authenc_esn_ctx *ctx = crypto_aead_ctx(authenc_esn);
@@ -191,14 +175,11 @@ static int crypto_authenc_esn_encrypt(struct aead_request *req)
 	sg_init_table(areq_ctx->src, 2);
 	src = scatterwalk_ffwd(areq_ctx->src, req->src, assoclen);
 	dst = src;
 
 	if (req->src != req->dst) {
-		err = crypto_authenc_esn_copy(req, assoclen);
-		if (err)
-			return err;
-
+		memcpy_sglist(req->dst, req->src, assoclen);
 		sg_init_table(areq_ctx->dst, 2);
 		dst = scatterwalk_ffwd(areq_ctx->dst, req->dst, assoclen);
 	}
 
 	skcipher_request_set_tfm(skreq, enc);
@@ -281,15 +262,12 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	if (assoclen < 8)
 		return -EINVAL;
 
 	cryptlen -= authsize;
 
-	if (req->src != dst) {
-		err = crypto_authenc_esn_copy(req, assoclen + cryptlen);
-		if (err)
-			return err;
-	}
+	if (req->src != dst)
+		memcpy_sglist(dst, req->src, assoclen + cryptlen);
 
 	scatterwalk_map_and_copy(ihash, req->src, assoclen + cryptlen,
 				 authsize, 0);
 
 	if (!authsize)
@@ -321,11 +299,10 @@ static int crypto_authenc_esn_init_tfm(struct crypto_aead *tfm)
 	struct aead_instance *inst = aead_alg_instance(tfm);
 	struct authenc_esn_instance_ctx *ictx = aead_instance_ctx(inst);
 	struct crypto_authenc_esn_ctx *ctx = crypto_aead_ctx(tfm);
 	struct crypto_ahash *auth;
 	struct crypto_skcipher *enc;
-	struct crypto_sync_skcipher *null;
 	int err;
 
 	auth = crypto_spawn_ahash(&ictx->auth);
 	if (IS_ERR(auth))
 		return PTR_ERR(auth);
@@ -333,18 +310,12 @@ static int crypto_authenc_esn_init_tfm(struct crypto_aead *tfm)
 	enc = crypto_spawn_skcipher(&ictx->enc);
 	err = PTR_ERR(enc);
 	if (IS_ERR(enc))
 		goto err_free_ahash;
 
-	null = crypto_get_default_null_skcipher();
-	err = PTR_ERR(null);
-	if (IS_ERR(null))
-		goto err_free_skcipher;
-
 	ctx->auth = auth;
 	ctx->enc = enc;
-	ctx->null = null;
 
 	ctx->reqoff = 2 * crypto_ahash_digestsize(auth);
 
 	crypto_aead_set_reqsize(
 		tfm,
@@ -356,12 +327,10 @@ static int crypto_authenc_esn_init_tfm(struct crypto_aead *tfm)
 		      sizeof(struct skcipher_request) +
 		      crypto_skcipher_reqsize(enc)));
 
 	return 0;
 
-err_free_skcipher:
-	crypto_free_skcipher(enc);
 err_free_ahash:
 	crypto_free_ahash(auth);
 	return err;
 }
 
@@ -369,11 +338,10 @@ static void crypto_authenc_esn_exit_tfm(struct crypto_aead *tfm)
 {
 	struct crypto_authenc_esn_ctx *ctx = crypto_aead_ctx(tfm);
 
 	crypto_free_ahash(ctx->auth);
 	crypto_free_skcipher(ctx->enc);
-	crypto_put_default_null_skcipher();
 }
 
 static void crypto_authenc_esn_free(struct aead_instance *inst)
 {
 	struct authenc_esn_instance_ctx *ctx = aead_instance_ctx(inst);
Signature · 1 line
2.54.0
#6 lore raw

PATCH 6/8 crypto: authencesn - Do not place hiseq at end of dst for out-of-place decryption

In reply to Eric Biggers

From: Herbert Xu <[email protected]>

commit e02494114ebf7c8b42777c6cd6982f113bfdbec7 upstream.

When decrypting data that is not in-place (src != dst), there is no need to save the high-order sequence bits in dst as it could simply be re-copied from the source.

However, the data to be hashed need to be rearranged accordingly.

Fixes 104880a6b470 ("crypto: authencesn - Convert to new AEAD interface")
---
 crypto/authencesn.c | 48 +++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 19 deletions(-)
crypto/authencesn.c +29 -19
index a5fbb638d9d7..257af38ac4de 100644
@@ -205,34 +205,39 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 						  ctx->reqoff);
 	struct crypto_ahash *auth = ctx->auth;
 	u8 *ohash = areq_ctx->tail;
 	unsigned int cryptlen = req->cryptlen - authsize;
 	unsigned int assoclen = req->assoclen;
+	struct scatterlist *src = req->src;
 	struct scatterlist *dst = req->dst;
 	u8 *ihash = ohash + crypto_ahash_digestsize(auth);
 	u32 tmp[2];
 
 	if (!authsize)
 		goto decrypt;
 
-	/* Move high-order bits of sequence number back. */
-	scatterwalk_map_and_copy(tmp, dst, 4, 4, 0);
-	scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen, 4, 0);
-	scatterwalk_map_and_copy(tmp, dst, 0, 8, 1);
+	if (src == dst) {
+		/* Move high-order bits of sequence number back. */
+		scatterwalk_map_and_copy(tmp, dst, 4, 4, 0);
+		scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen, 4, 0);
+		scatterwalk_map_and_copy(tmp, dst, 0, 8, 1);
+	} else
+		memcpy_sglist(dst, src, assoclen);
 
 	if (crypto_memneq(ihash, ohash, authsize))
 		return -EBADMSG;
 
 decrypt:
 
-	sg_init_table(areq_ctx->dst, 2);
+	if (src != dst)
+		src = scatterwalk_ffwd(areq_ctx->src, src, assoclen);
 	dst = scatterwalk_ffwd(areq_ctx->dst, dst, assoclen);
 
 	skcipher_request_set_tfm(skreq, ctx->enc);
 	skcipher_request_set_callback(skreq, flags,
 				      req->base.complete, req->base.data);
-	skcipher_request_set_crypt(skreq, dst, dst, cryptlen, req->iv);
+	skcipher_request_set_crypt(skreq, src, dst, cryptlen, req->iv);
 
 	return crypto_skcipher_decrypt(skreq);
 }
 
 static void authenc_esn_verify_ahash_done(void *data, int err)
@@ -253,35 +258,40 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	struct crypto_ahash *auth = ctx->auth;
 	u8 *ohash = areq_ctx->tail;
 	unsigned int assoclen = req->assoclen;
 	unsigned int cryptlen = req->cryptlen;
 	u8 *ihash = ohash + crypto_ahash_digestsize(auth);
+	struct scatterlist *src = req->src;
 	struct scatterlist *dst = req->dst;
 	u32 tmp[2];
 	int err;
 
 	if (assoclen < 8)
 		return -EINVAL;
 
-	cryptlen -= authsize;
-
-	if (req->src != dst)
-		memcpy_sglist(dst, req->src, assoclen + cryptlen);
+	if (!authsize)
+		goto tail;
 
+	cryptlen -= authsize;
 	scatterwalk_map_and_copy(ihash, req->src, assoclen + cryptlen,
 				 authsize, 0);
 
-	if (!authsize)
-		goto tail;
-
 	/* Move high-order bits of sequence number to the end. */
-	scatterwalk_map_and_copy(tmp, dst, 0, 8, 0);
-	scatterwalk_map_and_copy(tmp, dst, 4, 4, 1);
-	scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen, 4, 1);
-
-	sg_init_table(areq_ctx->dst, 2);
-	dst = scatterwalk_ffwd(areq_ctx->dst, dst, 4);
+	scatterwalk_map_and_copy(tmp, src, 0, 8, 0);
+	if (src == dst) {
+		scatterwalk_map_and_copy(tmp, dst, 4, 4, 1);
+		scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen, 4, 1);
+		dst = scatterwalk_ffwd(areq_ctx->dst, dst, 4);
+	} else {
+		scatterwalk_map_and_copy(tmp, dst, 0, 4, 1);
+		scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen - 4, 4, 1);
+
+		src = scatterwalk_ffwd(areq_ctx->src, src, 8);
+		dst = scatterwalk_ffwd(areq_ctx->dst, dst, 4);
+		memcpy_sglist(dst, src, assoclen + cryptlen - 8);
+		dst = req->dst;
+	}
 
 	ahash_request_set_tfm(ahreq, auth);
 	ahash_request_set_crypt(ahreq, dst, ohash, assoclen + cryptlen);
 	ahash_request_set_callback(ahreq, aead_request_flags(req),
 				   authenc_esn_verify_ahash_done, req);
Signature · 1 line
2.54.0
#7 lore raw

PATCH 7/8 crypto: authencesn - Fix src offset when decrypting in-place

In reply to Eric Biggers

From: Herbert Xu <[email protected]>

commit 1f48ad3b19a9dfc947868edda0bb8e48e5b5a8fa upstream.

The src SG list offset wasn't set properly when decrypting in-place, fix it.

Fixes e02494114ebf ("crypto: authencesn - Do not place hiseq at end of dst for out-of-place decryption")
---
 crypto/authencesn.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
crypto/authencesn.c +4 -2
index 257af38ac4de..c01cc3087919 100644
@@ -226,13 +226,15 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 	if (crypto_memneq(ihash, ohash, authsize))
 		return -EBADMSG;
 
 decrypt:
 
-	if (src != dst)
-		src = scatterwalk_ffwd(areq_ctx->src, src, assoclen);
 	dst = scatterwalk_ffwd(areq_ctx->dst, dst, assoclen);
+	if (req->src == req->dst)
+		src = dst;
+	else
+		src = scatterwalk_ffwd(areq_ctx->src, src, assoclen);
 
 	skcipher_request_set_tfm(skreq, ctx->enc);
 	skcipher_request_set_callback(skreq, flags,
 				      req->base.complete, req->base.data);
 	skcipher_request_set_crypt(skreq, src, dst, cryptlen, req->iv);
Signature · 1 line
2.54.0
#8 lore raw

PATCH 8/8 crypto: af_alg - Fix page reassignment overflow in af_alg_pull_tsgl

In reply to Eric Biggers

From: Herbert Xu <[email protected]>

commit 31d00156e50ecad37f2cb6cbf04aaa9a260505ef upstream.

When page reassignment was added to af_alg_pull_tsgl the original loop wasn't updated so it may try to reassign one more page than necessary.

Add the check to the reassignment so that this does not happen.

Also update the comment which still refers to the obsolete offset argument.

Reported-by [email protected]
Fixes e870456d8e7c ("crypto: algif_skcipher - overhaul memory management")
---
 crypto/af_alg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
crypto/af_alg.c +2 -2
index 6867d177f2a2..b61c3ba126ed 100644
@@ -703,12 +703,12 @@ void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst)
 
 			/*
 			 * Assumption: caller created af_alg_count_tsgl(len)
 			 * SG entries in dst.
 			 */
-			if (dst) {
-				/* reassign page to dst after offset */
+			if (dst && plen) {
+				/* reassign page to dst */
 				get_page(page);
 				sg_set_page(dst + j, page, plen, sg[i].offset);
 				j++;
 			}
 
Signature · 1 line
2.54.0
#9 lore raw
In reply to Eric Biggers
Quoted 10 lines · Eric Biggers

This series backports the recent AF_ALG fixes to 6.12. These include the fix for https://copy.fail/, fixes for that fix, and some other fixes that went in at around the same time that seem related.

To enable the 5 actual fix commits to cherry-pick cleanly, commit 1 copies the latest implementation of memcpy_sglist() from upstream, and commits 2 and 5 cleanly cherry-pick a couple cleanup commits.

I didn't check older kernels yet, but this should be usable as a starting point for them.

It applies to 6.6 as well. There's a conflict on 6.1.

- Eric

maintainer
#10 lore raw
Quoted 5 lines · Eric Biggers
Quoted 4 lines · Eric Biggers

This series backports the recent AF_ALG fixes to 6.12. These include the fix for https://copy.fail/, fixes for that fix, and some other fixes that went in at around the same time that seem related.

Quoted 8 lines
Quoted 6 lines

To enable the 5 actual fix commits to cherry-pick cleanly, commit 1 copies the latest implementation of memcpy_sglist() from upstream, and commits 2 and 5 cleanly cherry-pick a couple cleanup commits.

I didn't check older kernels yet, but this should be usable as a starting point for them.

It applies to 6.6 as well. There's a conflict on 6.1.

Thanks a lot for these, let me go do some "quick" releases with these fixes in it to tame my inbox a bit :)

greg k-h

#11 lore raw
In reply to Eric Biggers
Quoted 5 lines · Eric Biggers
Quoted 4 lines · Eric Biggers

This series backports the recent AF_ALG fixes to 6.12. These include the fix for https://copy.fail/, fixes for that fix, and some other fixes that went in at around the same time that seem related.

Quoted 8 lines
Quoted 6 lines

To enable the 5 actual fix commits to cherry-pick cleanly, commit 1 copies the latest implementation of memcpy_sglist() from upstream, and commits 2 and 5 cleanly cherry-pick a couple cleanup commits.

I didn't check older kernels yet, but this should be usable as a starting point for them.

It applies to 6.6 as well. There's a conflict on 6.1.

There's a conflict on 6.6, let me see if I can fix it up...

#12 lore raw
Quoted 10 lines · Greg KH
Quoted 9 lines · Eric Biggers
Quoted 8 lines · Eric Biggers

This series backports the recent AF_ALG fixes to 6.12. These include the fix for https://copy.fail/, fixes for that fix, and some other fixes that went in at around the same time that seem related.

To enable the 5 actual fix commits to cherry-pick cleanly, commit 1 copies the latest implementation of memcpy_sglist() from upstream, and commits 2 and 5 cleanly cherry-pick a couple cleanup commits.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

I didn't check older kernels yet, but this should be usable as a starting point for them.

It applies to 6.6 as well. There's a conflict on 6.1.

There's a conflict on 6.6, let me see if I can fix it up...

I think I got it working now...