[PATCH 1/1] lib: rsa: fix data abort in br_i32_decode()

[PATCH 1/1] lib: rsa: fix data abort in br_i32_decode()

[Heinrich Schuchardt]

After removing leading zeros the RSA modulus may be unaligned. On
architectures like ARM 32bit unaligned access may lead to a data abort,
e.g. when executing 'ut lib lib_asn1_pkcs7'.

Use memcpy() to transfer from unaligned to aligned memory.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/rsa/rsa-keyprop.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 1e83eedc82..6153cb00b3 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -17,23 +17,29 @@
 #include <u-boot/rsa-mod-exp.h>

 /**
- * br_dec16be() - Convert 16-bit big-endian integer to native
- * @src:	Pointer to data
- * Return:	Native-endian integer
+ * br_dec16be() - convert unaligned 16-bit big-endian integer to native
+ * @src:	unaligned pointer to data
+ * Return:	native-endian 16-bit integer
  */
 static unsigned br_dec16be(const void *src)
 {
-	return be16_to_cpup(src);
+	u16 buf;
+
+	memcpy(&buf, src, sizeof(buf));
+	return be16_to_cpu(buf);
 }

 /**
- * br_dec32be() - Convert 32-bit big-endian integer to native
- * @src:	Pointer to data
- * Return:	Native-endian integer
+ * br_dec32be() - convert unaligned 32-bit big-endian integer to native
+ * @src:	unaligned pointer to data
+ * Return:	native-endian 32-bit integer
  */
 static uint32_t br_dec32be(const void *src)
 {
-	return be32_to_cpup(src);
+	u32 buf;
+
+	memcpy(&buf, src, sizeof(buf));
+	return be32_to_cpu(buf);
 }

 /**
--
2.20.1

[PATCH 1/1] lib: rsa: fix data abort in br_i32_decode()

[Simon Glass]

HI Heinrich,

On Tue, 8 Sep 2020 at 04:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> After removing leading zeros the RSA modulus may be unaligned. On
> architectures like ARM 32bit unaligned access may lead to a data abort,
> e.g. when executing 'ut lib lib_asn1_pkcs7'.
>
> Use memcpy() to transfer from unaligned to aligned memory.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/rsa/rsa-keyprop.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
> index 1e83eedc82..6153cb00b3 100644
> --- a/lib/rsa/rsa-keyprop.c
> +++ b/lib/rsa/rsa-keyprop.c
> @@ -17,23 +17,29 @@
>  #include <u-boot/rsa-mod-exp.h>
>
>  /**
> - * br_dec16be() - Convert 16-bit big-endian integer to native
> - * @src:       Pointer to data
> - * Return:     Native-endian integer
> + * br_dec16be() - convert unaligned 16-bit big-endian integer to native
> + * @src:       unaligned pointer to data
> + * Return:     native-endian 16-bit integer
>   */
>  static unsigned br_dec16be(const void *src)
>  {
> -       return be16_to_cpup(src);
> +       u16 buf;
> +
> +       memcpy(&buf, src, sizeof(buf));
> +       return be16_to_cpu(buf);

Is it possible to use __get_unaligned_be() ?

Regards,
Simon

[PATCH 1/1] lib: rsa: fix data abort in br_i32_decode()

[AKASHI Takahiro]

On Tue, Sep 08, 2020 at 05:56:06PM -0600, Simon Glass wrote:
> HI Heinrich,
> 
> On Tue, 8 Sep 2020 at 04:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > After removing leading zeros the RSA modulus may be unaligned. On
> > architectures like ARM 32bit unaligned access may lead to a data abort,
> > e.g. when executing 'ut lib lib_asn1_pkcs7'.
> >
> > Use memcpy() to transfer from unaligned to aligned memory.
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> >  lib/rsa/rsa-keyprop.c | 22 ++++++++++++++--------
> >  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> >
> > diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
> > index 1e83eedc82..6153cb00b3 100644
> > --- a/lib/rsa/rsa-keyprop.c
> > +++ b/lib/rsa/rsa-keyprop.c
> > @@ -17,23 +17,29 @@
> >  #include <u-boot/rsa-mod-exp.h>
> >
> >  /**
> > - * br_dec16be() - Convert 16-bit big-endian integer to native
> > - * @src:       Pointer to data
> > - * Return:     Native-endian integer
> > + * br_dec16be() - convert unaligned 16-bit big-endian integer to native
> > + * @src:       unaligned pointer to data
> > + * Return:     native-endian 16-bit integer
> >   */
> >  static unsigned br_dec16be(const void *src)
> >  {
> > -       return be16_to_cpup(src);
> > +       u16 buf;
> > +
> > +       memcpy(&buf, src, sizeof(buf));
> > +       return be16_to_cpu(buf);
> 
> Is it possible to use __get_unaligned_be() ?

Robert has sent the fix here:
https://lists.denx.de/pipermail/u-boot/2020-September/425949.html

I think it's better.

-Takahiro Akashi


> Regards,
> Simon