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

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Apr 15, 2026 at 12:14:55AM +0200, Lukas Wunner wrote:
> 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;

Well, now you're passing an uninitialized pointer as a function
parameter.  (The loop body still operates on data from the previous
iteration.)  So no, that isn't a step forwards.  This should be
rewritten as a conventional loop and be switched to the scatterwalk API.

But this is really just a symptom of the larger issue with the pointless
virtaddr => scatterlist => virtaddr conversion that is happening.  And
lib/crypto/mpi/ is broken in other ways too, e.g. it's not constant-time
but is being used for RSA signing.  And it has no tests.

This is on my TODO list to fix up at some point, but it's a big task.
Secure bignum implementations aren't easy; userspace projects have spent
a long time to get them right.  So far I've instead been focusing on
symmetric crypto, which is what actually matters in the kernel.  If
someone is using the broken kernel RSA signing UAPIs (which should never
have been added), they hopefully know what they're getting...

- Eric