RockPi4B: spl_load_fit_image() writing past end of buffer & MMC errors
Hi,
I am trying to enable FIT image verification by SPL on Rockchip 4B (U-Boot v2022.04) and I hit a couple of issues.
First, I noticed that spl_load_fit_image() may write memory outside the range given in the image node. For example on RockPi 4B I have the following FIT (irrelevant parts omitted):
/ { images { atf_2 { /* File size is 8024 bytes */ data = /incbin/("bl31_0xff3b0000.bin"); compression = "none"; load = <0xff3b0000> } } }
I expect SPL to load atf_2 into 0xff3b0000 - 0xff3b200b but due to the alignment of the source data in the MMC, spl_load_fit_image() writes one more block and later moves the data in place with memcpy(). What are the guarantees that it is a safe thing to do?
In my case, the image starts at offset 308 in the 512-byte MMC block (that offset is called 'overhead' in spl_load_fit_image()). As a result, (8204 + 308) / 512 + 1 = 17 blocks = 8704 bytes are read.
For some reason I can't explain, on my board only the first 8K of the 64K SRAM range 0xff3b0000 - 0xff3c0000 (INTMEM1) can be safely written to. Any data written after this point corrupt the lower 8K. I found nothing in the rk3399 TRM about this [1].
Anyways, I tried using a temporary buffer allocated on the heap to handle the first and last blocks at least (the load address is properly aligned for info->read() so the middle blocks can be read in one go). It works but not reliably. And that is the second problem. mmc_read_blocks() in mmc_bread() sometimes returns an error. If I read blocks one by one in a loop THE read randomly fails after a few blocks only. The error is -110 (-ETIMEDOUT) from dwmci_send_cmd().
Am I using the MMC read incorrectly?
[1] https://www.rockchip.fr/Rockchip%20RK3399%20TRM%20V1.4%20Part1.pdf
Thanks,
El Fri, Jun 03, 2022 at 03:17:05PM +0200, Jerome Forissier deia:
First, I noticed that spl_load_fit_image() may write memory outside the range given in the image node. For example on RockPi 4B I have the following FIT (irrelevant parts omitted):
/ { images { atf_2 { /* File size is 8024 bytes */ data = /incbin/("bl31_0xff3b0000.bin"); compression = "none"; load = <0xff3b0000> } } }
I expect SPL to load atf_2 into 0xff3b0000 - 0xff3b200b but due to the alignment of the source data in the MMC, spl_load_fit_image() writes one more block and later moves the data in place with memcpy(). What are the guarantees that it is a safe thing to do?
In my case bl31_0xff3b0000.bin is 8020 bytes in size.
In my case, the image starts at offset 308 in the 512-byte MMC block (that offset is called 'overhead' in spl_load_fit_image()). As a result, (8204 + 308) / 512 + 1 = 17 blocks = 8704 bytes are read.
My overhead is 352, it's reading 17 blocks too.
For some reason I can't explain, on my board only the first 8K of the 64K SRAM range 0xff3b0000 - 0xff3c0000 (INTMEM1) can be safely written to. Any data written after this point corrupt the lower 8K. I found nothing in the rk3399 TRM about this [1].
In my Rock Pi 4B this does not happen. It happily writes the 17 blocks. Or it's just that I'm not noticing the problem ? Does it hang or give you an error, or do you just find it out by reading the SRAM?
I'm using the patch I sent yesterday:
[PATCH 0/3] arm: rockchip: rk3399: rock-pi-4: power domain driver to boot from MMCSD https://lists.denx.de/pipermail/u-boot/2022-June/485498.html
(instead of the patch you can simply configure CONFIG_ROCKCHIP_SPL_RESERVE_IRAM from make nconfig or so)
Which reserves 0x0 - 0x14fff bytes in DDRAM for bl31. I found that otherwise, SPL memory in that area gets overwritten by bl31_0x00000000.bin. I don't remember exactly what was there, but in my tests, bl31_0x00000000 gets loaded before bl31_0xff3b0000.bin. Depending on what data or code is overwritten, SPL might be doing something funny.
When I didn't use that patch, my SPL hanged after "Trying to boot from MMC1", before loading the ATF2 image, just when loading ATF1. But maybe depening on what's in that area, or the particular layout resulting from a particular build, the effect might be different. It might be a little random.
Anyways, I tried using a temporary buffer allocated on the heap to handle the first and last blocks at least (the load address is properly aligned for info->read() so the middle blocks can be read in one go). It works but not reliably. And that is the second problem. mmc_read_blocks() in mmc_bread() sometimes returns an error. If I read blocks one by one in a loop THE read randomly fails after a few blocks only. The error is -110 (-ETIMEDOUT) from dwmci_send_cmd().
Am I using the MMC read incorrectly?
It's not that you're using the read incorrectly.
It's that the change shouldn't be needed ? Why can one only use the first 8K from the 64K INTMEM1 ? Did you find any reason the rest shouldn't be writable?
Could corrupted memory explain what you're seeing ?
Hi Xavier,
On 6/3/22 16:58, Xavier Drudis Ferran wrote:
El Fri, Jun 03, 2022 at 03:17:05PM +0200, Jerome Forissier deia:
First, I noticed that spl_load_fit_image() may write memory outside the range given in the image node. For example on RockPi 4B I have the following FIT (irrelevant parts omitted):
/ { images { atf_2 { /* File size is 8024 bytes */ data = /incbin/("bl31_0xff3b0000.bin"); compression = "none"; load = <0xff3b0000> } } }
I expect SPL to load atf_2 into 0xff3b0000 - 0xff3b200b but due to the alignment of the source data in the MMC, spl_load_fit_image() writes one more block and later moves the data in place with memcpy(). What are the guarantees that it is a safe thing to do?
In my case bl31_0xff3b0000.bin is 8020 bytes in size.
In my case, the image starts at offset 308 in the 512-byte MMC block (that offset is called 'overhead' in spl_load_fit_image()). As a result, (8204 + 308) / 512 + 1 = 17 blocks = 8704 bytes are read.
My overhead is 352, it's reading 17 blocks too.
For some reason I can't explain, on my board only the first 8K of the 64K SRAM range 0xff3b0000 - 0xff3c0000 (INTMEM1) can be safely written to. Any data written after this point corrupt the lower 8K. I found nothing in the rk3399 TRM about this [1].
In my Rock Pi 4B this does not happen. It happily writes the 17 blocks. Or it's just that I'm not noticing the problem ? Does it hang or give you an error, or do you just find it out by reading the SRAM?
I found out when trying to use signed FIT images. The board would boot fine otherwise.
I am setting CONFIG_SPL_FIT_SIGNATURE=y and I have modified the script arch/arm/mach-rockchip/make_fit_atf.py so that it generates the required hash and signature nodes in u-boot.its. Then I am basically following the instructions in doc/uImage.FIT/signature.txt -- generating a RSA key pair with OpenSSL and using 'mkimage -k keys -K -r $(other-args...)'.
Please refer to the patches at:
https://git.codelinaro.org/linaro/dependable-boot/meta-ts/-/merge_requests/7...
It is work in progress really but it should give you an idea of what I am doing, and possibly help reproduce the issue.
I also confirmed the issue with a small test case. Could you please try the following patch?
---8<----------------------------------------------------------------------- diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index f6ccd837aa..810d5fa6db 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -401,6 +401,12 @@ static int mmc_read_blocks(struct mmc *mmc, void *dst, lbaint_t start, struct mmc_cmd cmd; struct mmc_data data;
+ printf("== 0x%08x\n", *(u32 *)0xff3b0100); + *(u32 *)0xff3b0100 = 0; + printf("== 0x%08x\n", *(u32 *)0xff3b0100); + *(u32 *)0xff3b2100 = 0xdeadbeef; + printf("== 0x%08x\n", *(u32 *)0xff3b0100); + if (blkcnt > 1) cmd.cmdidx = MMC_CMD_READ_MULTIPLE_BLOCK; else ---8<-----------------------------------------------------------------------
Guess what, this **does** print "0xdeadbeef" on the console on my board! :O
I'm using the patch I sent yesterday:
[PATCH 0/3] arm: rockchip: rk3399: rock-pi-4: power domain driver to boot from MMCSD https://lists.denx.de/pipermail/u-boot/2022-June/485498.html
(instead of the patch you can simply configure CONFIG_ROCKCHIP_SPL_RESERVE_IRAM from make nconfig or so)
Which reserves 0x0 - 0x14fff bytes in DDRAM for bl31. I found that otherwise, SPL memory in that area gets overwritten by bl31_0x00000000.bin. I don't remember exactly what was there, but in my tests, bl31_0x00000000 gets loaded before bl31_0xff3b0000.bin. Depending on what data or code is overwritten, SPL might be doing something funny.
I don't have bl31_0x00000000.bin. The bl31*.bin files come from bl31.elf which is split by arch/arm/mach-rockchip/make_fit_atf.py. It could be that we are using different versions of TF-A or different build parameters, but that seems unrelated I think.
Anyways, I tried using a temporary buffer allocated on the heap to handle the first and last blocks at least (the load address is properly aligned for info->read() so the middle blocks can be read in one go). It works but not reliably. And that is the second problem. mmc_read_blocks() in mmc_bread() sometimes returns an error. If I read blocks one by one in a loop THE read randomly fails after a few blocks only. The error is -110 (-ETIMEDOUT) from dwmci_send_cmd().
Am I using the MMC read incorrectly?
It's not that you're using the read incorrectly.
It's that the change shouldn't be needed ?
Ideally, yes, but it doesn't seem right to overflow the specified range. Who knows what might be immediately after the end of the buffer for a given image? Perhaps another image has been written there already, or the image might be right before a reserved address range such is some mapped I/O area?
Why can one only use the first 8K from the 64K INTMEM1 ? Did you find any reason the rest shouldn't be writable?
No, I don't understand. As I said the rest seems writable (at least up to some point) but writing to it corrupts the memory below...
Could corrupted memory explain what you're seeing ?
I found no evidence of corruption until SPL writes to 0xff3b2000 and above.
Thanks,
Sorry for the noise.
El Fri, Jun 03, 2022 at 04:58:38PM +0200, Xavier Drudis Ferran deia:
In my Rock Pi 4B this does not happen. It happily writes the 17 blocks. Or it's just that I'm not noticing the problem ? Does it hang or give you an error, or do you just find it out by reading the SRAM?
It was just that I didn't notice the problem.
I'm using the patch I sent yesterday:
[PATCH 0/3] arm: rockchip: rk3399: rock-pi-4: power domain driver to boot from MMCSD https://lists.denx.de/pipermail/u-boot/2022-June/485498.html
It doesn't matter, it seems it's just me who gets ATF1 at address 0.
It's that the change shouldn't be needed ? Why can one only use the first 8K from the 64K INTMEM1 ? Did you find any reason the rest shouldn't be writable?
Because INTMEM1 is only 8K long in RK3399. 64K is just the length of the addressing page it's in, but the SRAM is just 8Kb in size. INTMEM0 is 192 Kb, but not INTMEM1
participants (2)
-
Jerome Forissier -
Xavier Drudis Ferran