[PATCH] rsa: fix alignment issue when getting public exponent

[Heiko Stübner]

Am Montag, 4. Mai 2020, 16:17:52 CEST schrieb Simon Glass:
> +Tom Rini
> 
> On Sun, 3 May 2020 at 05:26, Heiko Stuebner <heiko at sntech.de> wrote:
> >
> > From: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
> >
> > To fill the exponent field of the rsa_public_key struct, rsa_mod_exp_sw
> > did a cast to uint64_t of the key_prop->public_exponent field.
> > But that alignment is not guaranteed in all cases.
> >
> > This came to light when in my spl-fit-signature the key-name exceeded
> > a certain length and with it the verification then started failing.
> > (naming it "integrity" worked fine, "integrity-uboot" failed)
> >
> > key_prop.public_exponent itself is actually a void-pointer, fdt_getprop()
> > also just returns such a void-pointer and inside the devicetree the 64bit
> > exponent is represented as 2 32bit numbers, so assuming a 64bit alignment
> > can lead to false reads.
> >
> > So just use the already existing rsa_convert_big_endian() to do the actual
> > conversion from the dt's big-endian to the needed uint64 value.
> >
> > Fixes: fc2f4246b4b3 ("rsa: Split the rsa-verify to separate the modular exponentiation")
> > Signed-off-by: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
> > ---
> >  lib/rsa/rsa-mod-exp.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> 
> Nice find! This probably changed when we updated the DT recently since
> the unaligned-access thing was reverted I think. Or has this problem
> been there forever?

To me it looks like it must've been present since the beginning.
In commit e0f2f1553414 ("Implement generalised RSA public exponents for verified boot")
which introduced the exponent handling it already did:

const uint64_t *public_exponent;
public_exponent = fdt_getprop(blob, node, "rsa,exponent", &length);

So if the fdt_getprop internals didn't change since then it would've
even then cast the void* to an uint64*


I really don't understand yet why the longer key-name would trigger it
though ... names like "dev", "integrity" work fine and seemingly starting
at a certain length the alignment changed.


Heiko

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