Skip to contentSkip to conversation

crypto: Use nth_page instead of doing it by hand

PATCH In review Herbert Xu 2 participants 6 messages Mar 14, 2025 – Mar 16, 2025 Crypto API

lore
Share & tools
lore

Herbert Xu

author maintainer
#0 lore raw

v4 cover crypto: Use nth_page instead of doing it by hand

v4 removes the unrelated changes and restores nth_page.

This patch series is based on top of:

https://patchwork.kernel.org/project/linux-crypto/patch/b4b00e0fed2fe0e48a0d9b2270bed7e29b119f6a.1741842470.git.herbert@gondor.apana.org.au/

Curiously, the Crypto API scatterwalk incremented pages by hand rather than using nth_page. Possibly because scatterwalk predates nth_page (the following commit is from the history tree):

commit 3957f2b34960d85b63e814262a8be7d5ad91444d
Author: James Morris <[email protected]>
Date: Sun Feb 2 07:35:32 2003 -0800

[CRYPTO]: in/out scatterlist support for ciphers.

Fix this by using nth_page.

Herbert Xu (2):
crypto: scatterwalk - Use nth_page instead of doing it by hand
crypto: hash - Use nth_page instead of doing it by hand

 crypto/ahash.c               |  4 ++--
 include/crypto/scatterwalk.h | 30 ++++++++++++++++++++----------
 2 files changed, 22 insertions(+), 12 deletions(-)
Signature · 1 line
2.39.5
author maintainer
#1 lore raw

PATCH v4 1/2 crypto: scatterwalk - Use nth_page instead of doing it by hand

Curiously, the Crypto API scatterwalk incremented pages by hand rather than using nth_page. Possibly because scatterwalk predates nth_page (the following commit is from the history tree):

	commit 3957f2b34960d85b63e814262a8be7d5ad91444d
	Author: James Morris <[email protected]>
	Date:   Sun Feb 2 07:35:32 2003 -0800

	    [CRYPTO]: in/out scatterlist support for ciphers.

Fix this by using nth_page.

Signed-off-by Herbert Xu <[email protected]>
---
 include/crypto/scatterwalk.h | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)
include/crypto/scatterwalk.h +20 -10
index b7e617ae4442..94a8585f26b2 100644
@@ -100,11 +100,15 @@ static inline void scatterwalk_get_sglist(struct scatter_walk *walk,
 static inline void scatterwalk_map(struct scatter_walk *walk)
 {
 	struct page *base_page = sg_page(walk->sg);
+	unsigned int offset = walk->offset;
+	void *addr;
 
 	if (IS_ENABLED(CONFIG_HIGHMEM)) {
-		walk->__addr = kmap_local_page(base_page +
-					       (walk->offset >> PAGE_SHIFT)) +
-			       offset_in_page(walk->offset);
+		struct page *page;
+
+		page = nth_page(base_page, offset >> PAGE_SHIFT);
+		offset = offset_in_page(offset);
+		addr = kmap_local_page(page) + offset;
 	} else {
 		/*
 		 * When !HIGHMEM we allow the walker to return segments that

@@ -117,8 +121,10 @@ static inline void scatterwalk_map(struct scatter_walk *walk)
 		 * in the direct map, but this makes it clearer what is really
 		 * going on.
 		 */
-		walk->__addr = page_address(base_page) + walk->offset;
+		addr = page_address(base_page) + offset;
 	}
+
+	walk->__addr = addr;
 }
 
 /**

@@ -189,14 +195,18 @@ static inline void scatterwalk_done_dst(struct scatter_walk *walk,
 	 * reliably optimized out or not.
 	 */
 	if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE) {
-		struct page *base_page, *start_page, *end_page, *page;
+		struct page *base_page;
+		unsigned int offset;
+		int start, end, i;
 
 		base_page = sg_page(walk->sg);
-		start_page = base_page + (walk->offset >> PAGE_SHIFT);
-		end_page = base_page + ((walk->offset + nbytes +
-					 PAGE_SIZE - 1) >> PAGE_SHIFT);
-		for (page = start_page; page < end_page; page++)
-			flush_dcache_page(page);
+		offset = walk->offset;
+		start = offset >> PAGE_SHIFT;
+		end = start + (nbytes >> PAGE_SHIFT);
+		end += (offset_in_page(offset) + offset_in_page(nbytes) +
+			PAGE_SIZE - 1) >> PAGE_SHIFT;
+		for (i = start; i < end; i++)
+			flush_dcache_page(nth_page(base_page, i));
 	}
 	scatterwalk_advance(walk, nbytes);
 }
Signature · 1 line
2.39.5
#2 lore raw

Re: [v4 PATCH 1/2] crypto: scatterwalk - Use nth_page instead of doing it by hand

Quoted 11 lines · Herbert Xu

Curiously, the Crypto API scatterwalk incremented pages by hand rather than using nth_page. Possibly because scatterwalk predates nth_page (the following commit is from the history tree):

	commit 3957f2b34960d85b63e814262a8be7d5ad91444d
	Author: James Morris <[email protected]>
	Date:   Sun Feb 2 07:35:32 2003 -0800

	    [CRYPTO]: in/out scatterlist support for ciphers.

Fix this by using nth_page.

As I said on v2, this needs an explanation of what it's actually fixing.

Quoted 10 lines
include/crypto/scatterwalk.h +3 -1
@@ -189,14 +195,18 @@ static inline void scatterwalk_done_dst(struct scatter_walk *walk,
 	 * reliably optimized out or not.
 	 */
 	if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE) {
-		struct page *base_page, *start_page, *end_page, *page;
+		struct page *base_page;
+		unsigned int offset;
+		int start, end, i;
 
 		base_page = sg_page(walk->sg);

include/crypto/scatterwalk.h static inline void scatterwalk_done_dst(struct scatter_walk *walk, · line 207

Quoted 10 lines
include/crypto/scatterwalk.h +5 -5
-		start_page = base_page + (walk->offset >> PAGE_SHIFT);
-		end_page = base_page + ((walk->offset + nbytes +
-					 PAGE_SIZE - 1) >> PAGE_SHIFT);
-		for (page = start_page; page < end_page; page++)
-			flush_dcache_page(page);
+		offset = walk->offset;
+		start = offset >> PAGE_SHIFT;
+		end = start + (nbytes >> PAGE_SHIFT);
+		end += (offset_in_page(offset) + offset_in_page(nbytes) +
+			PAGE_SIZE - 1) >> PAGE_SHIFT;

The change to how the end page index is calculated is unrelated to the nth_page fix, and it makes the code slower and harder to understand. My original code just rounded the new offset up to a page boundary to get the end page index.

- Eric

author maintainer
#3 lore raw

Re: [v4 PATCH 1/2] crypto: scatterwalk - Use nth_page instead of doing it by hand

Quoted 10 lines · Eric Biggers
Quoted 10 lines
include/crypto/scatterwalk.h +3 -1
@@ -189,14 +195,18 @@ static inline void scatterwalk_done_dst(struct scatter_walk *walk,
 	 * reliably optimized out or not.
 	 */
 	if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE) {
-		struct page *base_page, *start_page, *end_page, *page;
+		struct page *base_page;
+		unsigned int offset;
+		int start, end, i;
Quoted 15 lines

include/crypto/scatterwalk.h static inline void scatterwalk_done_dst(struct scatter_walk *walk, · line 207

Quoted 11 lines
include/crypto/scatterwalk.h +5 -5
 		base_page = sg_page(walk->sg);
-		start_page = base_page + (walk->offset >> PAGE_SHIFT);
-		end_page = base_page + ((walk->offset + nbytes +
-					 PAGE_SIZE - 1) >> PAGE_SHIFT);
-		for (page = start_page; page < end_page; page++)
-			flush_dcache_page(page);
+		offset = walk->offset;
+		start = offset >> PAGE_SHIFT;
+		end = start + (nbytes >> PAGE_SHIFT);
+		end += (offset_in_page(offset) + offset_in_page(nbytes) +
+			PAGE_SIZE - 1) >> PAGE_SHIFT;

The change to how the end page index is calculated is unrelated to the nth_page fix, and it makes the code slower and harder to understand. My original code just rounded the new offset up to a page boundary to get the end page index.

The original code is open to overflows in the addition. The new version is not.

Cheers,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#4 lore raw

Re: [v4 PATCH 1/2] crypto: scatterwalk - Use nth_page instead of doing it by hand

Quoted 27 lines · Herbert Xu
Quoted 10 lines · Eric Biggers
Quoted 10 lines
include/crypto/scatterwalk.h +3 -1
@@ -189,14 +195,18 @@ static inline void scatterwalk_done_dst(struct scatter_walk *walk,
 	 * reliably optimized out or not.
 	 */
 	if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE) {
-		struct page *base_page, *start_page, *end_page, *page;
+		struct page *base_page;
+		unsigned int offset;
+		int start, end, i;
Quoted 15 lines

include/crypto/scatterwalk.h static inline void scatterwalk_done_dst(struct scatter_walk *walk, · line 207

Quoted 11 lines
include/crypto/scatterwalk.h +5 -5
 		base_page = sg_page(walk->sg);
-		start_page = base_page + (walk->offset >> PAGE_SHIFT);
-		end_page = base_page + ((walk->offset + nbytes +
-					 PAGE_SIZE - 1) >> PAGE_SHIFT);
-		for (page = start_page; page < end_page; page++)
-			flush_dcache_page(page);
+		offset = walk->offset;
+		start = offset >> PAGE_SHIFT;
+		end = start + (nbytes >> PAGE_SHIFT);
+		end += (offset_in_page(offset) + offset_in_page(nbytes) +
+			PAGE_SIZE - 1) >> PAGE_SHIFT;

The change to how the end page index is calculated is unrelated to the nth_page fix, and it makes the code slower and harder to understand. My original code just rounded the new offset up to a page boundary to get the end page index.

Quoted 2 lines

The original code is open to overflows in the addition. The new version is not.

If you think you are fixing a separate issue, you need to say so.

But I also don't think it's worth worrying about lengths so close to UINT_MAX. No one is using them, they have no testing, and there are likely to be other overflows like this one too.

- Eric

author maintainer
#5 lore raw

PATCH v4 2/2 crypto: hash - Use nth_page instead of doing it by hand

In reply to Herbert Xu

Use nth_page instead of adding n to the page pointer.

Signed-off-by Herbert Xu <[email protected]>
---
 crypto/ahash.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
crypto/ahash.c +2 -2
index 1fe594880295..06559e5a715b 100644
@@ -80,7 +80,7 @@ static int hash_walk_new_entry(struct crypto_hash_walk *walk)
 
 	sg = walk->sg;
 	walk->offset = sg->offset;
-	walk->pg = sg_page(walk->sg) + (walk->offset >> PAGE_SHIFT);
+	walk->pg = nth_page(sg_page(walk->sg), (walk->offset >> PAGE_SHIFT));
 	walk->offset = offset_in_page(walk->offset);
 	walk->entrylen = sg->length;
 

@@ -221,7 +221,7 @@ int shash_ahash_digest(struct ahash_request *req, struct shash_desc *desc)
 	if (!IS_ENABLED(CONFIG_HIGHMEM))
 		return crypto_shash_digest(desc, data, nbytes, req->result);
 
-	page += offset >> PAGE_SHIFT;
+	page = nth_page(page, offset >> PAGE_SHIFT);
 	offset = offset_in_page(offset);
 
 	if (nbytes > (unsigned int)PAGE_SIZE - offset)
Signature · 1 line
2.39.5