[PATCH] crypto: authencesn - Copy high sequence number from src after out-of-place decryption

[PATCH] crypto: authencesn - Copy high sequence number from src after out-of-place decryption

[Herbert Xu]

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.

Reported-by: Taeyang Lee <0wn@theori.io>
Fixes: 104880a6b470 ("crypto: authencesn - Convert to new AEAD interface")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index 542a978663b9..fae8c1dbf495 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -215,9 +215,12 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 		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 (req->src == dst) {
+		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, req->src, 8);
 
 	if (crypto_memneq(ihash, ohash, authsize))
 		return -EBADMSG;
@@ -273,10 +276,12 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	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);
+	if (req->src == dst) {
+		/* Move high-order bits of sequence number to the end. */
+		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);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: authencesn - Copy high sequence number from src after out-of-place decryption

[Linus Torvalds]

On Wed, 25 Mar 2026 at 02:21, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
>         /* 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 (req->src == dst) {
> +               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, req->src, 8);

Side note: can we please just get rid of the horrid
scatterwalk_map_and_copy() when making changes to code?

That function is disgusting. It's really hard to see which direction it copies.

At least with memcpy_to/from_sglist() the function name and the order
of the arguments gives a better hint of what the code is trying to do.

This code is all very hard to read even _without_ the code being
intentionally obfuscated with that horrid interface.

                  Linus

Re: [PATCH] crypto: authencesn - Copy high sequence number from src after out-of-place decryption

[Herbert Xu]

On Wed, Mar 25, 2026 at 09:23:31AM -0700, Linus Torvalds wrote:
>
> Side note: can we please just get rid of the horrid
> scatterwalk_map_and_copy() when making changes to code?
> 
> That function is disgusting. It's really hard to see which direction it copies.
> 
> At least with memcpy_to/from_sglist() the function name and the order
> of the arguments gives a better hint of what the code is trying to do.

Sure, I'll post a follow-up patch to convert scatterwalk_map_and_copy
to memcpy_to/from_sglist.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: authencesn - Copy high sequence number from src after out-of-place decryption

[Herbert Xu]

Convert scatterwalk_map_and_copy to memcpy_to/from_sglist as they
are more readable and less error-prone.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index fae8c1dbf495..1cb7133bfb14 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -94,11 +94,11 @@ static int crypto_authenc_esn_genicv_tail(struct aead_request *req,
 	u32 tmp[2];
 
 	/* 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);
+	memcpy_from_sglist(tmp, dst, 4, 4);
+	memcpy_from_sglist(tmp + 1, dst, assoclen + cryptlen, 4);
+	memcpy_to_sglist(dst, 0, tmp, 8);
 
-	scatterwalk_map_and_copy(hash, dst, assoclen + cryptlen, authsize, 1);
+	memcpy_to_sglist(dst, assoclen + cryptlen, hash, authsize);
 	return 0;
 }
 
@@ -129,9 +129,9 @@ static int crypto_authenc_esn_genicv(struct aead_request *req,
 		return 0;
 
 	/* 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);
+	memcpy_from_sglist(tmp, dst, 0, 8);
+	memcpy_to_sglist(dst, 4, tmp, 4);
+	memcpy_to_sglist(dst, assoclen + cryptlen, tmp + 1, 4);
 
 	sg_init_table(areq_ctx->dst, 2);
 	dst = scatterwalk_ffwd(areq_ctx->dst, dst, 4);
@@ -216,9 +216,9 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 
 	/* Move high-order bits of sequence number back. */
 	if (req->src == dst) {
-		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);
+		memcpy_from_sglist(tmp, dst, 4, 4);
+		memcpy_from_sglist(tmp + 1, dst, assoclen + cryptlen, 4);
+		memcpy_to_sglist(dst, 0, tmp, 8);
 	} else
 		memcpy_sglist(dst, req->src, 8);
 
@@ -270,17 +270,16 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	if (req->src != dst)
 		memcpy_sglist(dst, req->src, assoclen + cryptlen);
 
-	scatterwalk_map_and_copy(ihash, req->src, assoclen + cryptlen,
-				 authsize, 0);
+	memcpy_from_sglist(ihash, req->src, assoclen + cryptlen, authsize);
 
 	if (!authsize)
 		goto tail;
 
-	scatterwalk_map_and_copy(tmp, dst, 0, 8, 0);
-	scatterwalk_map_and_copy(tmp, dst, 4, 4, 1);
+	memcpy_from_sglist(tmp, dst, 0, 8);
+	memcpy_to_sglist(dst, 4, tmp, 4);
 	if (req->src == dst) {
 		/* Move high-order bits of sequence number to the end. */
-		scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen, 4, 1);
+		memcpy_to_sglist(dst, assoclen + cryptlen, tmp + 1, 4);
 	}
 
 	sg_init_table(areq_ctx->dst, 2);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] crypto: authencesn - Use memcpy_from/to_sglist

[Herbert Xu]

Convert scatterwalk_map_and_copy to memcpy_to/from_sglist as they
are more readable and less error-prone.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index fae8c1dbf495..1cb7133bfb14 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -94,11 +94,11 @@ static int crypto_authenc_esn_genicv_tail(struct aead_request *req,
 	u32 tmp[2];
 
 	/* 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);
+	memcpy_from_sglist(tmp, dst, 4, 4);
+	memcpy_from_sglist(tmp + 1, dst, assoclen + cryptlen, 4);
+	memcpy_to_sglist(dst, 0, tmp, 8);
 
-	scatterwalk_map_and_copy(hash, dst, assoclen + cryptlen, authsize, 1);
+	memcpy_to_sglist(dst, assoclen + cryptlen, hash, authsize);
 	return 0;
 }
 
@@ -129,9 +129,9 @@ static int crypto_authenc_esn_genicv(struct aead_request *req,
 		return 0;
 
 	/* 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);
+	memcpy_from_sglist(tmp, dst, 0, 8);
+	memcpy_to_sglist(dst, 4, tmp, 4);
+	memcpy_to_sglist(dst, assoclen + cryptlen, tmp + 1, 4);
 
 	sg_init_table(areq_ctx->dst, 2);
 	dst = scatterwalk_ffwd(areq_ctx->dst, dst, 4);
@@ -216,9 +216,9 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 
 	/* Move high-order bits of sequence number back. */
 	if (req->src == dst) {
-		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);
+		memcpy_from_sglist(tmp, dst, 4, 4);
+		memcpy_from_sglist(tmp + 1, dst, assoclen + cryptlen, 4);
+		memcpy_to_sglist(dst, 0, tmp, 8);
 	} else
 		memcpy_sglist(dst, req->src, 8);
 
@@ -270,17 +270,16 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	if (req->src != dst)
 		memcpy_sglist(dst, req->src, assoclen + cryptlen);
 
-	scatterwalk_map_and_copy(ihash, req->src, assoclen + cryptlen,
-				 authsize, 0);
+	memcpy_from_sglist(ihash, req->src, assoclen + cryptlen, authsize);
 
 	if (!authsize)
 		goto tail;
 
-	scatterwalk_map_and_copy(tmp, dst, 0, 8, 0);
-	scatterwalk_map_and_copy(tmp, dst, 4, 4, 1);
+	memcpy_from_sglist(tmp, dst, 0, 8);
+	memcpy_to_sglist(dst, 4, tmp, 4);
 	if (req->src == dst) {
 		/* Move high-order bits of sequence number to the end. */
-		scatterwalk_map_and_copy(tmp + 1, dst, assoclen + cryptlen, 4, 1);
+		memcpy_to_sglist(dst, assoclen + cryptlen, tmp + 1, 4);
 	}
 
 	sg_init_table(areq_ctx->dst, 2);
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: authencesn - Copy high sequence number from src after out-of-place decryption

[Taeyang Lee]

Hi Herbert, Thanks for the patch.

On Wed, Mar 25, 2026 at 6:21 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> 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.

I don't think checking only `src != dst` is sufficient for the issue I
reported.

In the AF_ALG AEAD decrypt path, the child AEAD request is intentionally
set up to look in-place: `req->src == req->dst` on the RX SGL head, and
the TX-backed authentication-tag pages are then chained behind that RX
SGL. So from authencesn's point of view this still takes the `src == dst`
path, while `dst[assoclen + cryptlen]` can still resolve to TX-backed
pages, including splice()/MSG_SPLICE_PAGES-backed page-cache pages.

> Reported-by: Taeyang Lee <0wn@theori.io>
> Fixes: 104880a6b470 ("crypto: authencesn - Convert to new AEAD interface")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/crypto/authencesn.c b/crypto/authencesn.c
> index 542a978663b9..fae8c1dbf495 100644
> --- a/crypto/authencesn.c
> +++ b/crypto/authencesn.c
> @@ -215,9 +215,12 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
>                 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 (req->src == dst) {
> +               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, req->src, 8);
>
>         if (crypto_memneq(ihash, ohash, authsize))
>                 return -EBADMSG;
> @@ -273,10 +276,12 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
>         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);
> +       if (req->src == dst) {
> +               /* Move high-order bits of sequence number to the end. */
> +               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);

More fundamentally, I think the root cause is that authencesn decrypt
uses caller-owned req->dst as scratch storage at all. During decrypt it
writes both into the reserved destination AAD area and into the first bytes
past the documented AEAD decrypt output region.

Per the AEAD API, even in the out-of-place case space must be reserved in
the destination for AAD, but that area is not supposed to be written.
So my preference would be to fix this in authencesn itself by removing the
decrypt-side `req->dst` scratch usage entirely and keeping the
ESN temporary state in private per-request memory.

Thanks,


> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


--
___

Taeyang Lee, Security Researcher
Theori, Inc. / Xint Code
Website. www.theori.io / xint.io
Email. 0wn@theori.io

[PATCH] crypto: algif_aead - Revert to operating out-of-place

[Herbert Xu]

On Thu, Mar 26, 2026 at 02:59:24AM +0900, Taeyang Lee wrote:
> 
> I don't think checking only `src != dst` is sufficient for the issue I
> reported.
> 
> In the AF_ALG AEAD decrypt path, the child AEAD request is intentionally
> set up to look in-place: `req->src == req->dst` on the RX SGL head, and
> the TX-backed authentication-tag pages are then chained behind that RX
> SGL. So from authencesn's point of view this still takes the `src == dst`
> path, while `dst[assoclen + cryptlen]` can still resolve to TX-backed
> pages, including splice()/MSG_SPLICE_PAGES-backed page-cache pages.

Right, that's a separate bug.  algif_aead should not attach a
read-only mapping to the dst SG list, which will be written to.

---8<---
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")
Reported-by: Taeyang Lee <0wn@theori.io>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 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(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 0bb609fbec7d..0d2305ac1b52 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -635,15 +635,13 @@ 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;
@@ -658,25 +656,11 @@ unsigned int af_alg_count_tsgl(struct sock *sk, size_t bytes, size_t offset)
 		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;
 		}
 	}
 
@@ -688,19 +672,14 @@ 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;
@@ -725,18 +704,10 @@ void af_alg_pull_tsgl(struct sock *sk, size_t used, struct scatterlist *dst,
 			 * 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;
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 79b016a899a1..dda15bb05e89 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -26,7 +26,6 @@
 #include <crypto/internal/aead.h>
 #include <crypto/scatterwalk.h>
 #include <crypto/if_alg.h>
-#include <crypto/skcipher.h>
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/kernel.h>
@@ -72,9 +71,8 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 	struct alg_sock *pask = alg_sk(psk);
 	struct af_alg_ctx *ctx = ask->private;
 	struct crypto_aead *tfm = pask->private;
-	unsigned int i, as = crypto_aead_authsize(tfm);
+	unsigned int as = crypto_aead_authsize(tfm);
 	struct af_alg_async_req *areq;
-	struct af_alg_tsgl *tsgl, *tmp;
 	struct scatterlist *rsgl_src, *tsgl_src = NULL;
 	int err = 0;
 	size_t used = 0;		/* [in]  TX bufs to be en/decrypted */
@@ -154,23 +152,24 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 		outlen -= less;
 	}
 
+	/*
+	 * Create a per request TX SGL for this request which tracks the
+	 * SG entries from the global TX SGL.
+	 */
 	processed = used + ctx->aead_assoclen;
-	list_for_each_entry_safe(tsgl, tmp, &ctx->tsgl_list, list) {
-		for (i = 0; i < tsgl->cur; i++) {
-			struct scatterlist *process_sg = tsgl->sg + i;
-
-			if (!(process_sg->length) || !sg_page(process_sg))
-				continue;
-			tsgl_src = process_sg;
-			break;
-		}
-		if (tsgl_src)
-			break;
-	}
-	if (processed && !tsgl_src) {
-		err = -EFAULT;
+	areq->tsgl_entries = af_alg_count_tsgl(sk, processed);
+	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, processed, areq->tsgl);
+	tsgl_src = areq->tsgl;
 
 	/*
 	 * Copy of AAD from source to destination
@@ -179,76 +178,15 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg,
 	 * when user space uses an in-place cipher operation, the kernel
 	 * will copy the data as it does not see whether such in-place operation
 	 * is initiated.
-	 *
-	 * To ensure efficiency, the following implementation ensure that the
-	 * ciphers are invoked to perform a crypto operation in-place. This
-	 * is achieved by memory management specified as follows.
 	 */
 
 	/* Use the RX SGL as source (and destination) for crypto op. */
 	rsgl_src = areq->first_rsgl.sgl.sgt.sgl;
 
-	if (ctx->enc) {
-		/*
-		 * Encryption operation - The in-place cipher operation is
-		 * achieved by the following operation:
-		 *
-		 * TX SGL: AAD || PT
-		 *	    |	   |
-		 *	    | copy |
-		 *	    v	   v
-		 * RX SGL: AAD || PT || Tag
-		 */
-		memcpy_sglist(areq->first_rsgl.sgl.sgt.sgl, tsgl_src,
-			      processed);
-		af_alg_pull_tsgl(sk, processed, NULL, 0);
-	} else {
-		/*
-		 * Decryption operation - To achieve an in-place cipher
-		 * operation, the following  SGL structure is used:
-		 *
-		 * TX SGL: AAD || CT || Tag
-		 *	    |	   |	 ^
-		 *	    | copy |	 | Create SGL link.
-		 *	    v	   v	 |
-		 * RX SGL: AAD || CT ----+
-		 */
-
-		/* Copy AAD || CT to RX SGL buffer for in-place operation. */
-		memcpy_sglist(areq->first_rsgl.sgl.sgt.sgl, tsgl_src, outlen);
-
-		/* Create TX SGL for tag and chain it to RX SGL. */
-		areq->tsgl_entries = af_alg_count_tsgl(sk, processed,
-						       processed - as);
-		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);
-
-		/* Release TX SGL, except for tag data and reassign tag data. */
-		af_alg_pull_tsgl(sk, processed, areq->tsgl, processed - as);
-
-		/* chain the areq TX SGL holding the tag with RX SGL */
-		if (usedpages) {
-			/* RX SGL present */
-			struct af_alg_sgl *sgl_prev = &areq->last_rsgl->sgl;
-			struct scatterlist *sg = sgl_prev->sgt.sgl;
-
-			sg_unmark_end(sg + sgl_prev->sgt.nents - 1);
-			sg_chain(sg, sgl_prev->sgt.nents + 1, areq->tsgl);
-		} else
-			/* no RX SGL present (e.g. authentication only) */
-			rsgl_src = areq->tsgl;
-	}
+	memcpy_sglist(rsgl_src, tsgl_src, ctx->aead_assoclen);
 
 	/* Initialize the crypto operation */
-	aead_request_set_crypt(&areq->cra_u.aead_req, rsgl_src,
+	aead_request_set_crypt(&areq->cra_u.aead_req, tsgl_src,
 			       areq->first_rsgl.sgl.sgt.sgl, used, ctx->iv);
 	aead_request_set_ad(&areq->cra_u.aead_req, ctx->aead_assoclen);
 	aead_request_set_tfm(&areq->cra_u.aead_req, tfm);
@@ -450,7 +388,7 @@ static void aead_sock_destruct(struct sock *sk)
 	struct crypto_aead *tfm = pask->private;
 	unsigned int ivlen = crypto_aead_ivsize(tfm);
 
-	af_alg_pull_tsgl(sk, ctx->used, NULL, 0);
+	af_alg_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);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 125d395c5e00..82735e51be10 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -138,7 +138,7 @@ 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),
@@ -149,7 +149,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
 		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);
@@ -363,7 +363,7 @@ static void skcipher_sock_destruct(struct sock *sk)
 	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));
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 107b797c33ec..0cc8fa749f68 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -230,9 +230,8 @@ 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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: algif_aead - Revert to operating out-of-place

[Taeyang Lee]

On Thu, Mar 26, 2026 at 3:30 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Mar 26, 2026 at 02:59:24AM +0900, Taeyang Lee wrote:
> >
> > I don't think checking only `src != dst` is sufficient for the issue I
> > reported.
> >
> > In the AF_ALG AEAD decrypt path, the child AEAD request is intentionally
> > set up to look in-place: `req->src == req->dst` on the RX SGL head, and
> > the TX-backed authentication-tag pages are then chained behind that RX
> > SGL. So from authencesn's point of view this still takes the `src == dst`
> > path, while `dst[assoclen + cryptlen]` can still resolve to TX-backed
> > pages, including splice()/MSG_SPLICE_PAGES-backed page-cache pages.
>
> Right, that's a separate bug.  algif_aead should not attach a
> read-only mapping to the dst SG list, which will be written to.

Agreed.

By removing the RX/TX tag-page chaining and turning the child request into
a genuine out-of-place AEAD request, this looks like it closes the AF_ALG
page-cache exposure path.

With that in place, I think the security impact I reported should be
addressed, even though the authencesn-side use of req->dst as temporary
scratch storage during ESN rearrangement would still remain as separate
cleanup for now.

Thanks.

-- 
___

Taeyang Lee, Security Researcher
Theori, Inc. / Xint Code
Website. www.theori.io / xint.io
Email. 0wn@theori.io

[v2 PATCH] crypto: authencesn - Do not place hiseq at end of dst for out-of-place decryption

[Herbert Xu]

On Wed, Mar 25, 2026 at 06:21:18PM +0900, Herbert Xu wrote:
> 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.

This doesn't actually work because the high sequence number are
missing in the out-of-place case.  Here is an updated version to
handle that properly.  I will revise the follow-up patch to get
rid of scatterwalk_map_and_copy.

---8<---
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.

Reported-by: Taeyang Lee <0wn@theori.io>
Fixes: 104880a6b470 ("crypto: authencesn - Convert to new AEAD interface")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index 542a978663b9..c0a01d738d9b 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -207,6 +207,7 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 	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];
@@ -214,23 +215,27 @@ static int crypto_authenc_esn_decrypt_tail(struct aead_request *req,
 	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);
 }
@@ -255,6 +260,7 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	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;
@@ -262,24 +268,28 @@ static int crypto_authenc_esn_decrypt(struct aead_request *req)
 	if (assoclen < 8)
 		return -EINVAL;
 
-	cryptlen -= authsize;
-
-	if (req->src != dst)
-		memcpy_sglist(dst, req->src, assoclen + cryptlen);
-
-	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);
+	cryptlen -= authsize;
+	scatterwalk_map_and_copy(ihash, req->src, assoclen + cryptlen,
+				 authsize, 0);
 
-	sg_init_table(areq_ctx->dst, 2);
-	dst = scatterwalk_ffwd(areq_ctx->dst, dst, 4);
+	/* Move high-order bits of sequence number to the end. */
+	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);

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt