Re: [PATCH v2] lib/crypto/mpi: Fix heap OOB write in mpi_powm() when res aliases input

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] lib/crypto/mpi: NULL dereference in mpi_powm() due to unassigned rp_alloc
--

commit eb877144fed936eb853d022502854e67c6b7e317
Author: Maholi Tumanggor <maholitumanggor@gmail.com>

lib/crypto/mpi: Fix heap OOB write in mpi_powm() when res aliases input

This commit attempts to fix an out-of-bounds write in mpi_powm() when res
aliases an input operand and requires reallocation by introducing rp_alloc
as the write-back destination buffer when assign_rp is set.

> diff --git a/lib/crypto/mpi/mpi-pow.c b/lib/crypto/mpi/mpi-pow.c
> index 9e695a3bda3a0..f4df9fbcbc2c4 100644
> --- a/lib/crypto/mpi/mpi-pow.c
> +++ b/lib/crypto/mpi/mpi-pow.c
> @@ -28,6 +28,7 @@ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod)
>  	mpi_ptr_t mp_marker = NULL, bp_marker = NULL, ep_marker = NULL;
>  	struct karatsuba_ctx karactx = {};
>  	mpi_ptr_t xp_marker = NULL;
> +	mpi_ptr_t rp_alloc = NULL;
>  	mpi_ptr_t tspace = NULL;
>  	mpi_ptr_t rp, ep, mp, bp;
>  	mpi_size_t esize, msize, bsize, rsize;
> @@ -256,16 +257,23 @@ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod)
>  		 * might be, see above).
>  		 */
>  		if (mod_shift_cnt) {
> -			carry_limb =
> -			    mpihelp_lshift(res->d, rp, rsize, mod_shift_cnt);
> -			rp = res->d;
> +			mpi_ptr_t dst = assign_rp ? rp_alloc : res->d;
> +
> +			carry_limb = mpihelp_lshift(dst, rp, rsize, mod_shift_cnt);
> +			rp = dst;
>  			if (carry_limb) {
>  				rp[rsize] = carry_limb;
>  				rsize++;
>  			}
>  		} else {
> -			MPN_COPY(res->d, rp, rsize);
> -			rp = res->d;
> +			if (assign_rp) {
> +				if (rp != rp_alloc)
> +					MPN_COPY(rp_alloc, rp, rsize);
> +				rp = rp_alloc;
> +			} else {
> +				MPN_COPY(res->d, rp, rsize);
> +				rp = res->d;
> +			}
>  		}

[Severity: Critical]
As reported by Sashiko on v1 [1], rp_alloc is initialized to NULL in
mpi_powm(), but is it ever assigned when the new limb buffer is allocated?

In lib/crypto/mpi/mpi-pow.c:mpi_powm(), when res->alloced < size and rp
aliases an input operand:

	if (res->alloced < size) {
		if (rp == ep || rp == mp || rp == bp) {
			rp = mpi_alloc_limb_space(size);
			if (!rp)
				goto enomem;
			assign_rp = 1;
		}

rp_alloc is not updated here and remains NULL.

When execution reaches the write-back block with assign_rp set to 1, won't
mpihelp_lshift() (when mod_shift_cnt is non-zero) or MPN_COPY() (when
mod_shift_cnt is zero) dereference a NULL rp_alloc pointer, and overwrite
rp with NULL so the limb buffer allocated via mpi_alloc_limb_space() is
also leaked?

[1] https://lore.kernel.org/r/20261102064225.2271-1-maholitumanggor@gmail.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102092550.2633-1-maholitumanggor@gmail.com?part=1