* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y @ 2020-06-08 21:08 Heinrich Schuchardt 2020-06-09 1:42 ` AKASHI Takahiro 0 siblings, 1 reply; 7+ messages in thread From: Heinrich Schuchardt @ 2020-06-08 21:08 UTC (permalink / raw) To: u-boot 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. Could you, please, have a look. Best regards Heinrich ^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y 2020-06-08 21:08 [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y Heinrich Schuchardt @ 2020-06-09 1:42 ` AKASHI Takahiro 2020-06-09 3:54 ` Heinrich Schuchardt 0 siblings, 1 reply; 7+ messages in thread From: AKASHI Takahiro @ 2020-06-09 1:42 UTC (permalink / raw) To: u-boot 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 > Could you, please, have a look. > > Best regards > > Heinrich ^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y 2020-06-09 1:42 ` AKASHI Takahiro @ 2020-06-09 3:54 ` Heinrich Schuchardt 2020-06-09 9:22 ` AKASHI Takahiro 0 siblings, 1 reply; 7+ messages in thread From: Heinrich Schuchardt @ 2020-06-09 3:54 UTC (permalink / raw) To: u-boot Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro <takahiro.akashi@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. Best regards Heinrich > >> Could you, please, have a look. >> >> Best regards >> >> Heinrich ^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y 2020-06-09 3:54 ` Heinrich Schuchardt @ 2020-06-09 9:22 ` AKASHI Takahiro 2020-06-09 10:11 ` Heiko Stübner 0 siblings, 1 reply; 7+ messages in thread From: AKASHI Takahiro @ 2020-06-09 9:22 UTC (permalink / raw) To: u-boot 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@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. -Takahiro Akashi > Best regards > > Heinrich > > > > > >> Could you, please, have a look. > >> > >> Best regards > >> > >> Heinrich > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y 2020-06-09 9:22 ` AKASHI Takahiro @ 2020-06-09 10:11 ` Heiko Stübner 2020-06-09 10:22 ` Heinrich Schuchardt 0 siblings, 1 reply; 7+ messages in thread From: Heiko Stübner @ 2020-06-09 10:11 UTC (permalink / raw) To: u-boot 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@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 ;-) . Heiko ^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y 2020-06-09 10:11 ` Heiko Stübner @ 2020-06-09 10:22 ` Heinrich Schuchardt 2020-06-09 10:45 ` Heiko Stübner 0 siblings, 1 reply; 7+ messages in thread From: Heinrich Schuchardt @ 2020-06-09 10:22 UTC (permalink / raw) To: u-boot 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@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y 2020-06-09 10:22 ` Heinrich Schuchardt @ 2020-06-09 10:45 ` Heiko Stübner 0 siblings, 0 replies; 7+ messages in thread From: Heiko Stübner @ 2020-06-09 10:45 UTC (permalink / raw) To: u-boot Am Dienstag, 9. Juni 2020, 12:22:36 CEST schrieb 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@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. valid point :-) . I guess I got sidetracked with u-boot signature handling only doing up to 4096bit right now and was not questioning the existing usage of max_rsa_size enough. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-09 10:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-08 21:08 [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y Heinrich Schuchardt 2020-06-09 1:42 ` AKASHI Takahiro 2020-06-09 3:54 ` Heinrich Schuchardt 2020-06-09 9:22 ` AKASHI Takahiro 2020-06-09 10:11 ` Heiko Stübner 2020-06-09 10:22 ` Heinrich Schuchardt 2020-06-09 10:45 ` Heiko Stübner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox