Re: [PATCH v2] crypto/ecc: Actually remove stack VLA usage
On Fri, Mar 30, 2018 at 09:55:44AM -0700, Kees Cook wrote: > On the quest to remove all VLAs from the kernel[1], this avoids VLAs > by just using the maximum allocation size (4 bytes) for stack arrays. > All the VLAs in ecc were either 3 or 4 bytes (or a multiple), so just > make it 4 bytes all the time. Initialization routines are adjusted to > check that ndigits does not end up larger than the arrays. > > This includes a removal of the earlier attempt at this fix from > commit a963834b4742 ("crypto/ecc: Remove stack VLA usage") > > [1] https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Kees CookPatch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto/ecc: Actually remove stack VLA usage
On Fri, Mar 30, 2018 at 09:55:44AM -0700, Kees Cook wrote: > On the quest to remove all VLAs from the kernel[1], this avoids VLAs > by just using the maximum allocation size (4 bytes) for stack arrays. > All the VLAs in ecc were either 3 or 4 bytes (or a multiple), so just > make it 4 bytes all the time. Initialization routines are adjusted to > check that ndigits does not end up larger than the arrays. > > This includes a removal of the earlier attempt at this fix from > commit a963834b4742 ("crypto/ecc: Remove stack VLA usage") > > [1] https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Kees Cook Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto/ecc: Actually remove stack VLA usage
On Fri, Mar 30, 2018 at 9:55 AM, Kees Cookwrote: > On the quest to remove all VLAs from the kernel[1], this avoids VLAs > by just using the maximum allocation size (4 bytes) for stack arrays. > All the VLAs in ecc were either 3 or 4 bytes (or a multiple), so just > make it 4 bytes all the time. Initialization routines are adjusted to > check that ndigits does not end up larger than the arrays. > > This includes a removal of the earlier attempt at this fix from > commit a963834b4742 ("crypto/ecc: Remove stack VLA usage") > > [1] https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Kees Cook Friendly ping. Please apply to fix a963834b4742 ("crypto/ecc: Remove stack VLA usage"). Thanks! -Kees > --- > v2: > - Squash revert (herbert) > --- > crypto/ecc.c | 66 > +-- > crypto/ecc.h | 4 +++- > crypto/ecdh.c | 4 ++-- > 3 files changed, 33 insertions(+), 41 deletions(-) > > diff --git a/crypto/ecc.c b/crypto/ecc.c > index 9c066b5ac12d..815541309a95 100644 > --- a/crypto/ecc.c > +++ b/crypto/ecc.c > @@ -515,7 +515,7 @@ static void vli_mmod_fast_256(u64 *result, const u64 > *product, > static bool vli_mmod_fast(u64 *result, u64 *product, > const u64 *curve_prime, unsigned int ndigits) > { > - u64 tmp[2 * ndigits]; > + u64 tmp[2 * ECC_MAX_DIGITS]; > > switch (ndigits) { > case 3: > @@ -536,7 +536,7 @@ static bool vli_mmod_fast(u64 *result, u64 *product, > static void vli_mod_mult_fast(u64 *result, const u64 *left, const u64 *right, > const u64 *curve_prime, unsigned int ndigits) > { > - u64 product[2 * ndigits]; > + u64 product[2 * ECC_MAX_DIGITS]; > > vli_mult(product, left, right, ndigits); > vli_mmod_fast(result, product, curve_prime, ndigits); > @@ -546,7 +546,7 @@ static void vli_mod_mult_fast(u64 *result, const u64 > *left, const u64 *right, > static void vli_mod_square_fast(u64 *result, const u64 *left, > const u64 *curve_prime, unsigned int ndigits) > { > - u64 product[2 * ndigits]; > + u64 product[2 * ECC_MAX_DIGITS]; > > vli_square(product, left, ndigits); > vli_mmod_fast(result, product, curve_prime, ndigits); > @@ -560,8 +560,8 @@ static void vli_mod_square_fast(u64 *result, const u64 > *left, > static void vli_mod_inv(u64 *result, const u64 *input, const u64 *mod, > unsigned int ndigits) > { > - u64 a[ndigits], b[ndigits]; > - u64 u[ndigits], v[ndigits]; > + u64 a[ECC_MAX_DIGITS], b[ECC_MAX_DIGITS]; > + u64 u[ECC_MAX_DIGITS], v[ECC_MAX_DIGITS]; > u64 carry; > int cmp_result; > > @@ -649,8 +649,8 @@ static void ecc_point_double_jacobian(u64 *x1, u64 *y1, > u64 *z1, > u64 *curve_prime, unsigned int ndigits) > { > /* t1 = x, t2 = y, t3 = z */ > - u64 t4[ndigits]; > - u64 t5[ndigits]; > + u64 t4[ECC_MAX_DIGITS]; > + u64 t5[ECC_MAX_DIGITS]; > > if (vli_is_zero(z1, ndigits)) > return; > @@ -711,7 +711,7 @@ static void ecc_point_double_jacobian(u64 *x1, u64 *y1, > u64 *z1, > static void apply_z(u64 *x1, u64 *y1, u64 *z, u64 *curve_prime, > unsigned int ndigits) > { > - u64 t1[ndigits]; > + u64 t1[ECC_MAX_DIGITS]; > > vli_mod_square_fast(t1, z, curve_prime, ndigits);/* z^2 */ > vli_mod_mult_fast(x1, x1, t1, curve_prime, ndigits); /* x1 * z^2 */ > @@ -724,7 +724,7 @@ static void xycz_initial_double(u64 *x1, u64 *y1, u64 > *x2, u64 *y2, > u64 *p_initial_z, u64 *curve_prime, > unsigned int ndigits) > { > - u64 z[ndigits]; > + u64 z[ECC_MAX_DIGITS]; > > vli_set(x2, x1, ndigits); > vli_set(y2, y1, ndigits); > @@ -750,7 +750,7 @@ static void xycz_add(u64 *x1, u64 *y1, u64 *x2, u64 *y2, > u64 *curve_prime, > unsigned int ndigits) > { > /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ > - u64 t5[ndigits]; > + u64 t5[ECC_MAX_DIGITS]; > > /* t5 = x2 - x1 */ > vli_mod_sub(t5, x2, x1, curve_prime, ndigits); > @@ -791,9 +791,9 @@ static void xycz_add_c(u64 *x1, u64 *y1, u64 *x2, u64 > *y2, u64 *curve_prime, >unsigned int ndigits) > { > /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ > - u64 t5[ndigits]; > - u64 t6[ndigits]; > - u64 t7[ndigits]; > + u64 t5[ECC_MAX_DIGITS]; > + u64 t6[ECC_MAX_DIGITS]; > + u64 t7[ECC_MAX_DIGITS]; > > /* t5 = x2 - x1 */ > vli_mod_sub(t5, x2, x1, curve_prime, ndigits); > @@ -846,9 +846,9 @@ static void ecc_point_mult(struct ecc_point *result, >unsigned int ndigits) > { > /* R0 and R1 */ > - u64 rx[2][ndigits]; > - u64
Re: [PATCH v2] crypto/ecc: Actually remove stack VLA usage
On Fri, Mar 30, 2018 at 9:55 AM, Kees Cook wrote: > On the quest to remove all VLAs from the kernel[1], this avoids VLAs > by just using the maximum allocation size (4 bytes) for stack arrays. > All the VLAs in ecc were either 3 or 4 bytes (or a multiple), so just > make it 4 bytes all the time. Initialization routines are adjusted to > check that ndigits does not end up larger than the arrays. > > This includes a removal of the earlier attempt at this fix from > commit a963834b4742 ("crypto/ecc: Remove stack VLA usage") > > [1] https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Kees Cook Friendly ping. Please apply to fix a963834b4742 ("crypto/ecc: Remove stack VLA usage"). Thanks! -Kees > --- > v2: > - Squash revert (herbert) > --- > crypto/ecc.c | 66 > +-- > crypto/ecc.h | 4 +++- > crypto/ecdh.c | 4 ++-- > 3 files changed, 33 insertions(+), 41 deletions(-) > > diff --git a/crypto/ecc.c b/crypto/ecc.c > index 9c066b5ac12d..815541309a95 100644 > --- a/crypto/ecc.c > +++ b/crypto/ecc.c > @@ -515,7 +515,7 @@ static void vli_mmod_fast_256(u64 *result, const u64 > *product, > static bool vli_mmod_fast(u64 *result, u64 *product, > const u64 *curve_prime, unsigned int ndigits) > { > - u64 tmp[2 * ndigits]; > + u64 tmp[2 * ECC_MAX_DIGITS]; > > switch (ndigits) { > case 3: > @@ -536,7 +536,7 @@ static bool vli_mmod_fast(u64 *result, u64 *product, > static void vli_mod_mult_fast(u64 *result, const u64 *left, const u64 *right, > const u64 *curve_prime, unsigned int ndigits) > { > - u64 product[2 * ndigits]; > + u64 product[2 * ECC_MAX_DIGITS]; > > vli_mult(product, left, right, ndigits); > vli_mmod_fast(result, product, curve_prime, ndigits); > @@ -546,7 +546,7 @@ static void vli_mod_mult_fast(u64 *result, const u64 > *left, const u64 *right, > static void vli_mod_square_fast(u64 *result, const u64 *left, > const u64 *curve_prime, unsigned int ndigits) > { > - u64 product[2 * ndigits]; > + u64 product[2 * ECC_MAX_DIGITS]; > > vli_square(product, left, ndigits); > vli_mmod_fast(result, product, curve_prime, ndigits); > @@ -560,8 +560,8 @@ static void vli_mod_square_fast(u64 *result, const u64 > *left, > static void vli_mod_inv(u64 *result, const u64 *input, const u64 *mod, > unsigned int ndigits) > { > - u64 a[ndigits], b[ndigits]; > - u64 u[ndigits], v[ndigits]; > + u64 a[ECC_MAX_DIGITS], b[ECC_MAX_DIGITS]; > + u64 u[ECC_MAX_DIGITS], v[ECC_MAX_DIGITS]; > u64 carry; > int cmp_result; > > @@ -649,8 +649,8 @@ static void ecc_point_double_jacobian(u64 *x1, u64 *y1, > u64 *z1, > u64 *curve_prime, unsigned int ndigits) > { > /* t1 = x, t2 = y, t3 = z */ > - u64 t4[ndigits]; > - u64 t5[ndigits]; > + u64 t4[ECC_MAX_DIGITS]; > + u64 t5[ECC_MAX_DIGITS]; > > if (vli_is_zero(z1, ndigits)) > return; > @@ -711,7 +711,7 @@ static void ecc_point_double_jacobian(u64 *x1, u64 *y1, > u64 *z1, > static void apply_z(u64 *x1, u64 *y1, u64 *z, u64 *curve_prime, > unsigned int ndigits) > { > - u64 t1[ndigits]; > + u64 t1[ECC_MAX_DIGITS]; > > vli_mod_square_fast(t1, z, curve_prime, ndigits);/* z^2 */ > vli_mod_mult_fast(x1, x1, t1, curve_prime, ndigits); /* x1 * z^2 */ > @@ -724,7 +724,7 @@ static void xycz_initial_double(u64 *x1, u64 *y1, u64 > *x2, u64 *y2, > u64 *p_initial_z, u64 *curve_prime, > unsigned int ndigits) > { > - u64 z[ndigits]; > + u64 z[ECC_MAX_DIGITS]; > > vli_set(x2, x1, ndigits); > vli_set(y2, y1, ndigits); > @@ -750,7 +750,7 @@ static void xycz_add(u64 *x1, u64 *y1, u64 *x2, u64 *y2, > u64 *curve_prime, > unsigned int ndigits) > { > /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ > - u64 t5[ndigits]; > + u64 t5[ECC_MAX_DIGITS]; > > /* t5 = x2 - x1 */ > vli_mod_sub(t5, x2, x1, curve_prime, ndigits); > @@ -791,9 +791,9 @@ static void xycz_add_c(u64 *x1, u64 *y1, u64 *x2, u64 > *y2, u64 *curve_prime, >unsigned int ndigits) > { > /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ > - u64 t5[ndigits]; > - u64 t6[ndigits]; > - u64 t7[ndigits]; > + u64 t5[ECC_MAX_DIGITS]; > + u64 t6[ECC_MAX_DIGITS]; > + u64 t7[ECC_MAX_DIGITS]; > > /* t5 = x2 - x1 */ > vli_mod_sub(t5, x2, x1, curve_prime, ndigits); > @@ -846,9 +846,9 @@ static void ecc_point_mult(struct ecc_point *result, >unsigned int ndigits) > { > /* R0 and R1 */ > - u64 rx[2][ndigits]; > - u64 ry[2][ndigits]; > - u64 z[ndigits]; > +