Re: [PATCH RESEND v2 00/14] lib/mpi: bug fixes and cleanup

Hi Nicolai,
On 03/21/2016 06:26 AM, Nicolai Stange wrote:

This is a resend of v2 with the crypto people properly CC'd.

The original v1 can be found here:

  http://lkml.kernel.org/g/1458237606-4954-1-git-send-email-nicstange@gmail.com


While v1 (hopefully) fixed some issues in mpi_write_sgl() and
mpi_read_buffer() introduced by
  commit 2d4d1eea540b ("lib/mpi: Add mpi sgl helpers") and by
  commit 9cbe21d8f89d ("lib/mpi: only require buffers as big as needed for
                        the integer"),
I missed that there are some, including out-of-bounds buffer accesses,
in mpi_read_raw_from_sgl() as well.

Hence v2, which includes the original stuff from v1 plus my new fixes to
mpi_read_raw_from_sgl().


Applicable to linux-next-20160318.


Changes to v1:
  - [1-8/14]
    former [1-8/8], unchanged.

  - [9-14/14]
    Added in v2. Fixes to mpi_read_raw_from_sgl().

Nicolai Stange (14):
  lib/mpi: mpi_write_sgl(): fix skipping of leading zero limbs
  lib/mpi: mpi_write_sgl(): fix style issue with lzero decrement
  lib/mpi: mpi_write_sgl(): purge redundant pointer arithmetic
  lib/mpi: mpi_write_sgl(): fix out-of-bounds stack access
  lib/mpi: mpi_write_sgl(): replace open coded endian conversion
  lib/mpi: mpi_read_buffer(): optimize skipping of leading zero limbs
  lib/mpi: mpi_read_buffer(): replace open coded endian conversion
  lib/mpi: mpi_read_buffer(): fix buffer overflow
  lib/mpi: mpi_read_raw_from_sgl(): replace len argument by nbytes
  lib/mpi: mpi_read_raw_from_sgl(): don't include leading zero SGEs in
    nbytes
  lib/mpi: mpi_read_raw_from_sgl(): purge redundant clearing of nbits
  lib/mpi: mpi_read_raw_from_sgl(): fix nbits calculation
  lib/mpi: mpi_read_raw_from_sgl(): sanitize meaning of indices
  lib/mpi: mpi_read_raw_from_sgl(): fix out-of-bounds buffer access

 lib/mpi/mpicoder.c | 122 +++++++++++++++++++----------------------------------
 1 file changed, 43 insertions(+), 79 deletions(-)

Thanks for sending this. Nice work. In fact the mpi_write_sgl() function
worked only because the mpi_normalize() stripped all MSB zero limbs.
Given that this will hold for all cases we can simplify this even more.
Unfortunately I don't know if this will be true for mpi_sub() or
mpi_set_ui() because they are declared in include/linux/mpi.h,
but never defined anywhere.

I've found one problem in 08/14 in mpi_read_buffer()
It's a pointer arithmetic issue, which causes the first limb to be
written at an invalid address if lzeros > 0. This incremental patch
fixes it.

---8<---
Subject: [PATCH] lib/mpi: fix pointer arithmetic issue in mpi_read_buffer

Fix pointer arithmetic issue, which causes the first limb to be
written at invalid address if lzeros > 0.

Signed-off-by: Tadeusz Struk <redacted>
---
 lib/mpi/mpicoder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 0c534ac..747606f 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c

@@ -201,7 +201,7 @@ int mpi_read_buffer(MPI a, uint8_t *buf, unsigned buf_len, unsigned *nbytes,
 #else
 #error please implement for this limb size.
 #endif
-		memcpy(p, &alimb + lzeros, BYTES_PER_MPI_LIMB - lzeros);
+		memcpy(p, (u8 *)&alimb + lzeros, BYTES_PER_MPI_LIMB - lzeros);
 		p += BYTES_PER_MPI_LIMB - lzeros;
 		lzeros = 0;
 	}

---
Other than that  please include my 
Tested-by: Tadeusz Struk <redacted>
for the whole series.
Thanks,
-- 
TS