On Mon Apr 29, 2024 at 7:13 PM EEST, Stefan Berger wrote:
> Prevent ecc_digits_from_bytes from reading too many bytes from the input
> byte array in case an insufficient number of bytes is provided to fill the
> output digit array of ndigits. Therefore, initialize the most significant
> digits with 0 to avoid trying to read too many bytes later on. Convert the
> function into a regular function since it is getting too big for an inline
> function.
>
> If too many bytes are provided on the input byte array the extra bytes
> are ignored since the input variable 'ndigits' limits the number of digits
> that will be filled.
>
> Fixes: d67c96fb97b5 ("crypto: ecdsa - Convert byte arrays with key coordinates to digits")
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
>
> ---
>
> v2:
> - un-inline function
> - use memset
> ---
> crypto/ecc.c | 22 ++++++++++++++++++++++
> include/crypto/internal/ecc.h | 15 ++-------------
> 2 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index c1d2e884be1e..fe761256e335 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -68,6 +68,28 @@ const struct ecc_curve *ecc_get_curve(unsigned int curve_id)
> }
> EXPORT_SYMBOL(ecc_get_curve);
>
Just a minor nit:
For exported symbol you need to document the function,including
the parameters [1].
> +void ecc_digits_from_bytes(const u8 *in, unsigned int nbytes,
> + u64 *out, unsigned int ndigits)
> +{
> + int diff = ndigits - DIV_ROUND_UP(nbytes, sizeof(u64));
> + unsigned int o = nbytes & 7;
> + __be64 msd = 0;
> +
> + /* diff > 0: not enough input bytes: set most significant digits to 0 */
> + if (diff > 0) {
> + ndigits -= diff;
> + memset(&out[ndigits - 1], 0, diff * sizeof(u64));
> + }
> +
> + if (o) {
> + memcpy((u8 *)&msd + sizeof(msd) - o, in, o);
> + out[--ndigits] = be64_to_cpu(msd);
> + in += o;
> + }
> + ecc_swap_digits(in, out, ndigits);
> +}
> +EXPORT_SYMBOL(ecc_digits_from_bytes);
> +
> static u64 *ecc_alloc_digits_space(unsigned int ndigits)
> {
> size_t len = ndigits * sizeof(u64);
> diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h
> index 7ca1f463d1ec..f7e75e1e71f3 100644
> --- a/include/crypto/internal/ecc.h
> +++ b/include/crypto/internal/ecc.h
> @@ -64,19 +64,8 @@ static inline void ecc_swap_digits(const void *in, u64 *out, unsigned int ndigit
> * @out Output digits array
> * @ndigits: Number of digits to create from byte array
> */
> -static inline void ecc_digits_from_bytes(const u8 *in, unsigned int nbytes,
> - u64 *out, unsigned int ndigits)
> -{
> - unsigned int o = nbytes & 7;
> - __be64 msd = 0;
> -
> - if (o) {
> - memcpy((u8 *)&msd + sizeof(msd) - o, in, o);
> - out[--ndigits] = be64_to_cpu(msd);
> - in += o;
> - }
> - ecc_swap_digits(in, out, ndigits);
> -}
> +void ecc_digits_from_bytes(const u8 *in, unsigned int nbytes,
> + u64 *out, unsigned int ndigits);
>
> /**
> * ecc_is_key_valid() - Validate a given ECDH private key
[1] https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
BR, Jarkko
