From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 4 May 2020 11:06:27 -0600 Subject: [PATCH] rsa: fix alignment issue when getting public exponent In-Reply-To: <7460039.SgiU42vdzZ@diego> References: <20200503112634.590399-1-heiko@sntech.de> <7460039.SgiU42vdzZ@diego> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Heiko, On Mon, 4 May 2020 at 09:40, Heiko St?bner wrote: > > Am Montag, 4. Mai 2020, 16:17:52 CEST schrieb Simon Glass: > > +Tom Rini > > > > On Sun, 3 May 2020 at 05:26, Heiko Stuebner wrote: > > > > > > From: Heiko Stuebner > > > > > > 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 > > > --- > > > 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* > See this patch: e8c2d25845 libfdt: Revert 6dcb8ba4 from upstream libfdt > 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. My guess is that these lengths work: 1-3 chars (aligns to 4 bytes) 8-11 chars (aligns to 12 bytes) 16-19 chars (aligns to 20 bytes) etc. Regards, Simon