[PATCH] crypto: skcipher - Elinimate duplicate virt.addr field

[Herbert Xu <herbert@gondor.apana.org.au>]

On Wed, Mar 05, 2025 at 07:59:37PM -0800, Eric Biggers wrote:
>
> I don't think it will be quite that simple, since the skcipher_walk code relies
> on the different parts being split up so that it can do things like calculate
> the length before it starts mapping anything.  If you can make it work, we can
> do that.  But until that additional patch is ready I don't think it makes sense
> to merge this one, as it leaves things half-baked with the redundant pointers.

Sure, fixing it might not be easy, partly because the new interface
wasn't designed for its needs.

But getting rid of the duplicate field isn't hard, because we're
already assuming that the user does not modify walk->XXX.virt.addr,
at least not far enough to break the unmap (see the WALK_DIFF
clause).  In fact, grepping through the arch code seems to show
that nobody actually modifies them at all.  So we could even
simplify the WALK_SLOW done path.

---8<---
Reuse the addr field from struct scatter_walk for skcipher_walk.
In order to maintain backwards compatibility with existing users,
retain the original virt.addr fields through unions.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 crypto/skcipher.c                  | 25 ++++++++++---------------
 include/crypto/algapi.h            |  3 ++-
 include/crypto/internal/skcipher.h | 20 +++++++++++++++-----
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index d321c8746950..f770307abb8e 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -43,14 +43,12 @@ static inline void skcipher_map_src(struct skcipher_walk *walk)
 {
 	/* XXX */
 	walk->in.addr = scatterwalk_map(&walk->in);
-	walk->src.virt.addr = walk->in.addr;
 }
 
 static inline void skcipher_map_dst(struct skcipher_walk *walk)
 {
 	/* XXX */
 	walk->out.addr = scatterwalk_map(&walk->out);
-	walk->dst.virt.addr = walk->out.addr;
 }
 
 static inline gfp_t skcipher_walk_gfp(struct skcipher_walk *walk)
@@ -100,8 +98,7 @@ int skcipher_walk_done(struct skcipher_walk *walk, int res)
 				    SKCIPHER_WALK_DIFF)))) {
 		scatterwalk_advance(&walk->in, n);
 	} else if (walk->flags & SKCIPHER_WALK_DIFF) {
-		scatterwalk_unmap(walk->src.virt.addr);
-		scatterwalk_advance(&walk->in, n);
+		scatterwalk_done_src(&walk->in, n);
 	} else if (walk->flags & SKCIPHER_WALK_COPY) {
 		scatterwalk_advance(&walk->in, n);
 		skcipher_map_dst(walk);
@@ -116,11 +113,8 @@ int skcipher_walk_done(struct skcipher_walk *walk, int res)
 			 */
 			res = -EINVAL;
 			total = 0;
-		} else {
-			u8 *buf = PTR_ALIGN(walk->buffer, walk->alignmask + 1);
-
-			memcpy_to_scatterwalk(&walk->out, buf, n);
-		}
+		} else
+			memcpy_to_scatterwalk(&walk->out, walk->out.addr, n);
 		goto dst_done;
 	}
 
@@ -176,10 +170,11 @@ static int skcipher_next_slow(struct skcipher_walk *walk, unsigned int bsize)
 			return skcipher_walk_done(walk, -ENOMEM);
 		walk->buffer = buffer;
 	}
-	walk->dst.virt.addr = PTR_ALIGN(buffer, alignmask + 1);
-	walk->src.virt.addr = walk->dst.virt.addr;
 
-	memcpy_from_scatterwalk(walk->src.virt.addr, &walk->in, bsize);
+	buffer = PTR_ALIGN(buffer, alignmask + 1);
+	memcpy_from_scatterwalk(buffer, &walk->in, bsize);
+	walk->out.addr = buffer;
+	walk->in.addr = walk->out.addr;
 
 	walk->nbytes = bsize;
 	walk->flags |= SKCIPHER_WALK_SLOW;
@@ -199,8 +194,8 @@ static int skcipher_next_copy(struct skcipher_walk *walk)
 	 * processed (which might be less than walk->nbytes) is known.
 	 */
 
-	walk->src.virt.addr = tmp;
-	walk->dst.virt.addr = tmp;
+	walk->in.addr = tmp;
+	walk->out.addr = tmp;
 	return 0;
 }
 
@@ -214,7 +209,7 @@ static int skcipher_next_fast(struct skcipher_walk *walk)
 		(u8 *)(sg_page(walk->out.sg) + (walk->out.offset >> PAGE_SHIFT));
 
 	skcipher_map_src(walk);
-	walk->dst.virt.addr = walk->src.virt.addr;
+	walk->out.addr = walk->in.addr;
 
 	if (diff) {
 		walk->flags |= SKCIPHER_WALK_DIFF;
diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index 41733a0b45dd..94147ea8c14d 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -120,9 +120,10 @@ struct crypto_queue {
 };
 
 struct scatter_walk {
+	/* Must be the first member, see struct skcipher_walk. */
+	void *addr;
 	struct scatterlist *sg;
 	unsigned int offset;
-	void *addr;
 };
 
 struct crypto_attr_alg {
diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h
index d6ae7a86fed2..357441b56c1e 100644
--- a/include/crypto/internal/skcipher.h
+++ b/include/crypto/internal/skcipher.h
@@ -57,14 +57,24 @@ struct crypto_lskcipher_spawn {
 struct skcipher_walk {
 	union {
 		struct {
-			void *addr;
-		} virt;
-	} src, dst;
+			struct {
+				void *const addr;
+			} virt;
+		} src;
+		struct scatter_walk in;
+	};
 
-	struct scatter_walk in;
 	unsigned int nbytes;
 
-	struct scatter_walk out;
+	union {
+		struct {
+			struct {
+				void *const addr;
+			} virt;
+		} dst;
+		struct scatter_walk out;
+	};
+
 	unsigned int total;
 
 	u8 *page;
-- 
2.39.5

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