linux/lib/mpi/mpicoder.c, branch v4.7

lib/mpi: mpi_read_raw_from_sgl(): fix out-of-bounds buffer access

2016-04-05T12:35:51Z

Within the copying loop in mpi_read_raw_from_sgl(), the last input SGE's byte count gets artificially extended as follows: if (sg_is_last(sg) && (len % BYTES_PER_MPI_LIMB)) len += BYTES_PER_MPI_LIMB - (len % BYTES_PER_MPI_LIMB); Within the following byte copying loop, this causes reads beyond that SGE's allocated buffer: BUG: KASAN: slab-out-of-bounds in mpi_read_raw_from_sgl+0x331/0x650 at addr ffff8801e168d4d8 Read of size 1 by task systemd-udevd/721 [...] Call Trace: [] dump_stack+0xbc/0x117 [] ? _atomic_dec_and_lock+0x169/0x169 [] ? print_section+0x61/0xb0 [] print_trailer+0x179/0x2c0 [] object_err+0x34/0x40 [] kasan_report_error+0x307/0x8c0 [] ? kasan_unpoison_shadow+0x35/0x50 [] ? kasan_kmalloc+0x5e/0x70 [] kasan_report+0x71/0xa0 [] ? mpi_read_raw_from_sgl+0x331/0x650 [] __asan_load1+0x46/0x50 [] mpi_read_raw_from_sgl+0x331/0x650 [] rsa_verify+0x106/0x260 [] ? rsa_set_pub_key+0xf0/0xf0 [] ? sg_init_table+0x29/0x50 [] ? pkcs1pad_sg_set_buf+0xb2/0x2e0 [] pkcs1pad_verify+0x1f4/0x2b0 [] public_key_verify_signature+0x3a7/0x5e0 [] ? public_key_describe+0x80/0x80 [] ? keyring_search_aux+0x150/0x150 [] ? x509_request_asymmetric_key+0x114/0x370 [] ? kfree+0x220/0x370 [] public_key_verify_signature_2+0x32/0x50 [] verify_signature+0x7c/0xb0 [] pkcs7_validate_trust+0x42c/0x5f0 [] system_verify_data+0xca/0x170 [] ? top_trace_array+0x9b/0x9b [] ? __vfs_read+0x279/0x3d0 [] mod_verify_sig+0x1ff/0x290 [...] The exact purpose of the len extension isn't clear to me, but due to its form, I suspect that it's a leftover somehow accounting for leading zero bytes within the most significant output limb. Note however that without that len adjustement, the total number of bytes ever processed by the inner loop equals nbytes and thus, the last output limb gets written at this point. Thus the net effect of the len adjustement cited above is just to keep the inner loop running for some more iterations, namely < BYTES_PER_MPI_LIMB ones, reading some extra bytes from beyond the last SGE's buffer and discarding them afterwards. Fix this issue by purging the extension of len beyond the last input SGE's buffer length. Fixes: 2d4d1eea540b ("lib/mpi: Add mpi sgl helpers") Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_raw_from_sgl(): sanitize meaning of indices

2016-04-05T12:35:51Z

Within the byte reading loop in mpi_read_raw_sgl(), there are two housekeeping indices used, z and x. At all times, the index z represents the number of output bytes covered by the input SGEs for which processing has completed so far. This includes any leading zero bytes within the most significant limb. The index x changes its meaning after the first outer loop's iteration though: while processing the first input SGE, it represents "number of leading zero bytes in most significant output limb" + "current position within current SGE" For the remaining SGEs OTOH, x corresponds just to "current position within current SGE" After all, it is only the sum of z and x that has any meaning for the output buffer and thus, the "number of leading zero bytes in most significant output limb" part can be moved away from x into z from the beginning, opening up the opportunity for cleaner code. Before the outer loop iterating over the SGEs, don't initialize z with zero, but with the number of leading zero bytes in the most significant output limb. For the inner loop iterating over a single SGE's bytes, get rid of the buf_shift offset to x' bounds and let x run from zero to sg->length - 1. Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_raw_from_sgl(): fix nbits calculation

2016-04-05T12:35:51Z

The number of bits, nbits, is calculated in mpi_read_raw_from_sgl() as follows: nbits = nbytes * 8; Afterwards, the number of leading zero bits of the first byte get subtracted: nbits -= count_leading_zeros(*(u8 *)(sg_virt(sgl) + lzeros)); However, count_leading_zeros() takes an unsigned long and thus, the u8 gets promoted to an unsigned long. Thus, the above doesn't subtract the number of leading zeros in the most significant nonzero input byte from nbits, but the number of leading zeros of the most significant nonzero input byte promoted to unsigned long, i.e. BITS_PER_LONG - 8 too many. Fix this by subtracting count_leading_zeros(...) - (BITS_PER_LONG - 8) from nbits only. Fixes: 2d4d1eea540b ("lib/mpi: Add mpi sgl helpers") Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_raw_from_sgl(): purge redundant clearing of nbits

2016-04-05T12:35:51Z

In mpi_read_raw_from_sgl(), unsigned nbits is calculated as follows: nbits = nbytes * 8; and redundantly cleared later on if nbytes == 0: if (nbytes > 0) ... else nbits = 0; Purge this redundant clearing for the sake of clarity. Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_raw_from_sgl(): don't include leading zero SGEs in nbytes

2016-04-05T12:35:50Z

At the very beginning of mpi_read_raw_from_sgl(), the leading zeros of the input scatterlist are counted: lzeros = 0; for_each_sg(sgl, sg, ents, i) { ... if (/* sg contains nonzero bytes */) break; /* sg contains nothing but zeros here */ ents--; lzeros = 0; } Later on, the total number of trailing nonzero bytes is calculated by subtracting the number of leading zero bytes from the total number of input bytes: nbytes -= lzeros; However, since lzeros gets reset to zero for each completely zero leading sg in the loop above, it doesn't include those. Besides wasting resources by allocating a too large output buffer, this mistake propagates into the calculation of x, the number of leading zeros within the most significant output limb: x = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB; What's more, the low order bytes of the output, equal in number to the extra bytes in nbytes, are left uninitialized. Fix this by adjusting nbytes for each completely zero leading scatterlist entry. Fixes: 2d4d1eea540b ("lib/mpi: Add mpi sgl helpers") Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_raw_from_sgl(): replace len argument by nbytes

2016-04-05T12:35:49Z

Currently, the nbytes local variable is calculated from the len argument as follows: ... mpi_read_raw_from_sgl(..., unsigned int len) { unsigned nbytes; ... if (!ents) nbytes = 0; else nbytes = len - lzeros; ... } Given that nbytes is derived from len in a trivial way and that the len argument is shadowed by a local len variable in several loops, this is just confusing. Rename the len argument to nbytes and get rid of the nbytes local variable. Do the nbytes calculation in place. Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_buffer(): fix buffer overflow

2016-04-05T12:35:49Z

Currently, mpi_read_buffer() writes full limbs to the output buffer and moves memory around to purge leading zero limbs afterwards. However, with commit 9cbe21d8f89d ("lib/mpi: only require buffers as big as needed for the integer") the caller is only required to provide a buffer large enough to hold the result without the leading zeros. This might result in a buffer overflow for small MP numbers with leading zeros. Fix this by coping the result to its final destination within the output buffer and not copying the leading zeros at all. Fixes: 9cbe21d8f89d ("lib/mpi: only require buffers as big as needed for the integer") Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_buffer(): replace open coded endian conversion

2016-04-05T12:35:49Z

Currently, the endian conversion from CPU order to BE is open coded in mpi_read_buffer(). Replace this by the centrally provided cpu_to_be*() macros. Copy from the temporary storage on stack to the destination buffer by means of memcpy(). Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_read_buffer(): optimize skipping of leading zero limbs

2016-04-05T12:35:49Z

Currently, if the number of leading zeros is greater than fits into a complete limb, mpi_read_buffer() skips them by iterating over them limb-wise. Instead of skipping the high order zero limbs within the loop as shown above, adjust the copying loop's bounds. Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu

lib/mpi: mpi_write_sgl(): replace open coded endian conversion

2016-04-05T12:35:48Z

Currently, the endian conversion from CPU order to BE is open coded in mpi_write_sgl(). Replace this by the centrally provided cpu_to_be*() macros. Signed-off-by: Nicolai Stange Signed-off-by: Herbert Xu