crypto/ecc: Remove stack VLA usage
Subsystems (2)
PATCH v2 1/1 crypto/ecc: Remove stack VLA usage
On the quest to remove all VLAs from the kernel[1], this switches to a pair of kmalloc regions instead of using the stack. This also moves the get_random_bytes() after all allocations (and drops the needless "nbytes" variable).
[1] https://lkml.org/lkml/2018/3/7/621
--- crypto/ecc.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
crypto/ecc.c +17 -6
@@ -1025,9 +1025,7 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits, {int ret =0 ;struct ecc_point *product, *pk; -u64 priv[ndigits]; -u64 rand_z[ndigits]; -unsigned int nbytes; +u64 *priv, *rand_z;const struct ecc_curve *curve =ecc_get_curve (curve_id);if (!private_key || !public_key || !curve) {
@@ -1035,14 +1033,22 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits, goto out; } - nbytes = ndigits <<ECC_DIGITS_TO_BYTES_SHIFT ; + priv =kmalloc_array (ndigits,sizeof (*priv),GFP_KERNEL ); +if (!priv) { + ret = -ENOMEM ; + goto out; + } -get_random_bytes (rand_z, nbytes); + rand_z =kmalloc_array (ndigits,sizeof (*rand_z),GFP_KERNEL ); +if (!rand_z) { + ret = -ENOMEM ; + goto kfree_out; + } pk =ecc_alloc_point (ndigits);if (!pk) { ret = -ENOMEM ; - goto out; + goto kfree_out; } product =ecc_alloc_point (ndigits);
@@ -1051,6 +1057,8 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits, goto err_alloc_product; } +get_random_bytes (rand_z, ndigits <<ECC_DIGITS_TO_BYTES_SHIFT ); +ecc_swap_digits (public_key, pk->x, ndigits);ecc_swap_digits (&public_key[ndigits], pk->y, ndigits);ecc_swap_digits (private_key, priv, ndigits);
@@ -1065,6 +1073,9 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,ecc_free_point (product); err_alloc_product:ecc_free_point (pk); +kfree_out: +kzfree (priv); +kzfree (rand_z); out:return ret; }
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
Quoted 8 lines · Kees Cook
On the quest to remove all VLAs from the kernel[1], this switches to a pair of kmalloc regions instead of using the stack. This also moves the
get_random_bytes()after all allocations (and drops the needless "nbytes" variable).[1]https://lkml.org/lkml/2018/3/7/621
Signed-off-by Kees Cook<[email protected]>
Herbert Xu
maintainerQuoted 8 lines · Kees Cook
On the quest to remove all VLAs from the kernel[1], this switches to a pair of kmalloc regions instead of using the stack. This also moves the
get_random_bytes()after all allocations (and drops the needless "nbytes" variable).[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Kees Cook <[email protected]>
Patch applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 11 lines · Herbert Xu
Quoted 8 lines · Kees Cook
On the quest to remove all VLAs from the kernel[1], this switches to a pair of kmalloc regions instead of using the stack. This also moves the
get_random_bytes()after all allocations (and drops the needless "nbytes" variable).[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Kees Cook <[email protected]>Patch applied. Thanks.
Hi, sorry for the noise on this one: I messed up looking at the ecc code (I confused myself into thinking there was only a single instance of the problem). The applied patch is both incomplete and inefficient. I have a much simpler solution, and I'll send that with a revert...
-Kees
Signature · 2 lines
Kees Cook Pixel Security