diff mbox series

[v5,4/8] lib: rsa: fix allocated size for rr and rrtmp in rsa_gen_key_prop()

Message ID 20200522141937.3523692-4-heiko@sntech.de
State New
Headers show
Series [v5,1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY | expand

Commit Message

Heiko Stuebner May 22, 2020, 2:19 p.m. UTC 
From: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>

When calculating rrtmp/rr rsa_gen_key_prop() tries to make
(((rlen + 31) >> 5) + 1) steps in the rr uint32_t array and
(((rlen + 7) >> 3) + 1) / 4 steps in uint32_t rrtmp[]
with rlen being num_bits * 2

On a 4096bit key this comes down to to 257 uint32_t elements
in rr and 256 elements in rrtmp but with the current allocation
rr and rrtmp only have 129 uint32_t elements.

On 2048bit keys this works by chance as the defined max_rsa_size=4096
allocates a suitable number of elements, but with an actual 4096bit key
this results in other memory parts getting overwritten.

so double the number of elements in rr and rrtmp so that it matches
the needed number and should increase nicely if max_rsa_size gets
increased in the future.

Signed-off-by: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
---
changes in v4:
- new patch

 lib/rsa/rsa-keyprop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Heiko Stuebner May 25, 2020, 5:18 p.m. UTC | #1 
Am Freitag, 22. Mai 2020, 16:19:33 CEST schrieb Heiko Stuebner:
> From: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
> 
> When calculating rrtmp/rr rsa_gen_key_prop() tries to make
> (((rlen + 31) >> 5) + 1) steps in the rr uint32_t array and
> (((rlen + 7) >> 3) + 1) / 4 steps in uint32_t rrtmp[]
> with rlen being num_bits * 2
> 
> On a 4096bit key this comes down to to 257 uint32_t elements
> in rr and 256 elements in rrtmp but with the current allocation
> rr and rrtmp only have 129 uint32_t elements.
> 
> On 2048bit keys this works by chance as the defined max_rsa_size=4096
> allocates a suitable number of elements, but with an actual 4096bit key
> this results in other memory parts getting overwritten.
> 
> so double the number of elements in rr and rrtmp so that it matches
> the needed number and should increase nicely if max_rsa_size gets
> increased in the future.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>

transplanting a tag from v4:
Reviewed-by: Simon Glass <sjg at chromium.org>
Philipp Tomsich May 25, 2020, 8:43 p.m. UTC | #2 
> On 22.05.2020, at 16:19, Heiko Stuebner <heiko at sntech.de> wrote:
> 
> From: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
> 
> When calculating rrtmp/rr rsa_gen_key_prop() tries to make
> (((rlen + 31) >> 5) + 1) steps in the rr uint32_t array and
> (((rlen + 7) >> 3) + 1) / 4 steps in uint32_t rrtmp[]
> with rlen being num_bits * 2
> 
> On a 4096bit key this comes down to to 257 uint32_t elements
> in rr and 256 elements in rrtmp but with the current allocation
> rr and rrtmp only have 129 uint32_t elements.
> 
> On 2048bit keys this works by chance as the defined max_rsa_size=4096
> allocates a suitable number of elements, but with an actual 4096bit key
> this results in other memory parts getting overwritten.
> 
> so double the number of elements in rr and rrtmp so that it matches
> the needed number and should increase nicely if max_rsa_size gets
> increased in the future.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
diff mbox series

Patch

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 4b54db44c4..e28fbb7472 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -659,8 +659,8 @@  int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 
 	*prop = calloc(sizeof(**prop), 1);
 	n = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
-	rr = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
-	rrtmp = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
+	rr = calloc(sizeof(uint32_t), 1 + ((max_rsa_size * 2) >> 5));
+	rrtmp = calloc(sizeof(uint32_t), 1 + ((max_rsa_size * 2) >> 5));
 	if (!(*prop) || !n || !rr || !rrtmp) {
 		ret = -ENOMEM;
 		goto err;