Page MenuHomePhabricator

cannot read beyond 4Gbyte space from eMMC, NXP's sd_mmc driver
Open, NormalPublic

Description

Hello,
I think I found a bug in the NXP's driver code for eMMC reading on the ATF code ( Arm Trusted Firmware ).
The affected file is in /drivers/nxp/sd:
https://github.com/ARM-software/arm-trusted-firmware/blob/master/drivers/nxp/sd/sd_mmc.c
I am compiling ATF as per default settings for LS1046, AARCH64.
In the function :

static size_t ls_sd_emmc_read(int lba, uintptr_t buf, size_t size)
{
struct mmc *mmc = NULL;
int ret;

mmc = &mmc_drv_data;
lab *= BLOCK_LEN_512;
ret = esdhc_read(mmc, lba, buf, size);
return ret ? 0 : size;
}
lba parameter is int, that is signed int 32 bit. But then it gets multiplied by 512 ( block size ) and the result is passed as parameter to this function as a 32 bit unsigned :
int esdhc_read(struct mmc *mmc, uint32_t src_offset, uintptr_t dst, size_t size)

This ends up in far away lba multiplied by 512 to get the byte offset to read from ( beyond 4GByte ) resulting into a negative 32 bit value and then truncated to positive 32 bit offset.
End result is that it reads other area and this is wrong.
The whole sequence looks a bit weird as lba is multiplied by 512 to get the byte offset to be passed to esdhc_read as parameter but then this function divides the byte offset to get again back the lba logical block address.
The high-level layer of the driver thinks with IO_SEEK in byte offset not block offset and it is correctly handling the 64 bit range wide for the offset.
However, problem happens at that ls_sd_emmc_read level of the driver, the function described above here.
This is called by high level layer in io_block.c :
static int block_seek(io_entity_t *entity, int mode, ssize_t offset)
and
static int block_read(io_entity_t *entity, uintptr_t buffer, size_t length, size_t *length_read)
which correctly handles the offset with 64 bit wide data type ( ssize_t offset ) and stores it in internal structure with 64 bit wide data type.
In the end this io_block.c is used by the highest level part of the driver functions that are :
int io_seek(uintptr_t handle, io_seek_mode_t mode, ssize_t offset)
int io_read(uintptr_t handle,uintptr_t buffer,size_t length,size_t *length_read)
Same problem appears to be on the int esdhc_write(struct mmc *mmc, uintptr_t src, uint32_t dst_offset, size_t size)
accepting 32 bit offset only.
This write however is not used at the moment by anything but ota_status_check() that uses an offset in range of 32 bit. We don't use the write function at ATF level.
Read function problem beyond 64bit range is confirmed by real tests.
I have now a patch fixing this that allows reading correctly from space beyond 4Gbyte range.

Event Timeline

AntonioSanta triaged this task as Normal priority.Fri, Jul 29, 2:09 PM
AntonioSanta created this task.