Re: [PATCH] crypto: lib/mpi - Fix integer underflow in mpi_read_raw_from_sgl()

[Lukas Wunner <lukas@wunner.de>]

On Tue, Apr 14, 2026 at 10:59:03AM -0700, Eric Biggers wrote:
> On Sun, Apr 12, 2026 at 04:19:47PM +0200, Lukas Wunner wrote:
> > Yiming reports an integer underflow in mpi_read_raw_from_sgl() when
> > subtracting "lzeros" from the unsigned "nbytes".
[...]
> 
> This code (which has no tests...) is unnecessarily hard to understand,
> though.  I haven't been able to fully understand the logic yet, but it
> looks like it still has bugs, including still reading past the given
> nbytes.  It should be possible to replace it with something simpler and
> less error-prone.

mpi_read_raw_from_sgl() skips over leading zero bytes in the scatterlist
and then over leading zero bits in the first non-zero byte, before it
starts copying data to the newly-allocated MPI.

One possible simplification is to use memchr_inv() to search for the
first non-zero byte, instead of open coding it.  See the patch below.
If you think it's a step in the right direction, I can submit the patch
in the next cycle.

-- >8 --

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index cb001aa..72e7ac7 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -17,6 +17,7 @@
 #include "sg_sw_sec4.h"
 #include "caampkc.h"
 #include <crypto/internal/engine.h>
+#include <linux/count_zeros.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
@@ -219,12 +220,9 @@ static int caam_rsa_count_leading_zeros(struct scatterlist *sgl,
 	lzeros = 0;
 	len = 0;
 	while (nbytes > 0) {
-		/* do not strip more than given bytes */
-		while (len && !*buff && lzeros < nbytes) {
-			lzeros++;
-			len--;
-			buff++;
-		}
+		lzeros = count_leading_zerobytes(buff, min(len, nbytes));
+		len -= lzeros;
+		buff += lzeros;
 
 		if (len && *buff)
 			break;
diff --git a/include/linux/count_zeros.h b/include/linux/count_zeros.h
index 5b8ff5a..29d9d67 100644
--- a/include/linux/count_zeros.h
+++ b/include/linux/count_zeros.h
@@ -8,6 +8,7 @@
 #ifndef _LINUX_BITOPS_COUNT_ZEROS_H_
 #define _LINUX_BITOPS_COUNT_ZEROS_H_
 
+#include <linux/string.h>
 #include <asm/bitops.h>
 
 /**
@@ -50,4 +51,14 @@ static inline int count_trailing_zeros(unsigned long x)
 		return (x != 0) ? __ffs(x) : COUNT_TRAILING_ZEROS_0;
 }
 
+static inline size_t count_leading_zerobytes(const void *start, size_t len)
+{
+	void *first = memchr_inv(start, 0, len);
+
+	if (!first)
+		return len;
+
+	return first - start;
+}
+
 #endif /* _LINUX_BITOPS_COUNT_ZEROS_H_ */
diff --git a/lib/crypto/mpi/mpicoder.c b/lib/crypto/mpi/mpicoder.c
index 9359a58..011434d 100644
--- a/lib/crypto/mpi/mpicoder.c
+++ b/lib/crypto/mpi/mpicoder.c
@@ -347,11 +347,9 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes)
 	lzeros = 0;
 	len = 0;
 	while (nbytes > 0) {
-		while (len && !*buff && lzeros < nbytes) {
-			lzeros++;
-			len--;
-			buff++;
-		}
+		lzeros = count_leading_zerobytes(buff, min(len, nbytes));
+		len -= lzeros;
+		buff += lzeros;
 
 		if (len && *buff)
 			break;