[BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y

[Heinrich Schuchardt]

On 09.06.20 12:11, Heiko Stübner wrote:
> Hi Akashi,
>
> Am Dienstag, 9. Juni 2020, 11:22:35 CEST schrieb AKASHI Takahiro:
>> Heinrich,
>>
>> On Tue, Jun 09, 2020 at 03:54:44AM +0000, Heinrich Schuchardt wrote:
>>> Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
>>>> Heinrich,
>>>>
>>>> On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
>>>>> Hello Takahiro,
>>>>>
>>>>> when trying to execute command
>>>>>
>>>>> ut lib lib_rsa_verify_valid
>>>>>
>>>>> on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and
>>>>> CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
>>>>>
>>>>> free((void *)prop->modulus) called from
>>>>> rsa_free_key_prop() called from
>>>>> rsa_verify_key() called from
>>>>> rsa_verify_with_pkey().
>>>>>
>>>>> Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur.
>>>>> On qemu_arm64_defconfig the problem does not occur.
>>>>
>>>> I can't reproduce your problem on v2020.07-rc4 exactly with
>>>> qemu_arm64_defconfig + PKEY=y:
>>>>
>>>> U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
>>>>
>>>> ...
>>>>
>>>> => ut lib
>>>> Running 11 lib tests
>>>> Test: lib_asn1_pkcs7
>>>> Test: lib_asn1_pkey
>>>> Test: lib_asn1_x509
>>>> Test: lib_memcpy
>>>> Test: lib_memmove
>>>> Test: lib_memset
>>>> Test: lib_rsa_verify_invalid
>>>> Test: lib_rsa_verify_valid
>>>> Test: lib_test_bin2hex
>>>> Test: lib_test_hex2bin
>>>> Test: lib_test_hex_to_bin
>>>> Failures: 0
>>>>
>>>>
>>>> -Takahiro Akashi
>>>
>>> As said I only see the problem with 32 bit qemu_arm_defconfig.
>>
>> Okay. I think that the size of rrtmp variable is not good enough
>> and when it is handed over to br_i32_decode(), it accidentally
>> destroys (*prop)->modulus.
>>
>> Heiko's patch:
>>   https://lists.denx.de/pipermail/u-boot/2020-May/413101.html
>> will also fix this issue, but I'm not yet confident that
>> the solution here, doubling max_rsa_size, is a right approach.
>
> The algorithm does write outside its memory area with a big enough key
> and I have to confess I had a hard time understanding it ;-) .
>
> So what would be needed to get more confidence?
>
> Because for me it looks like either the algorithm does strange things
> or the memory area is too small - I don't really see a third option ;-) .

Your patch allocates unnecessary memory for key sizes below 4096 and
allocates too little for key sizes above 4096 bits. So with a bad key
size you can crash the code again.

Best regards

Heinrich