On 10/26/20 2:40 AM, Bin Meng wrote:
> Hi Philippe,
>
> On Mon, Oct 26, 2020 at 2:56 AM Philippe Mathieu-Daudé <f4...@amsat.org>
> wrote:
>>
>> On 10/25/20 4:23 PM, Bin Meng wrote:
>>> From: Bin Meng <bin.m...@windriver.com>
>>>
>>> Per the SD spec, to indicate a 2 GiB card, BLOCK_LEN shall be 1024
>>> bytes, hence the READ_BL_LEN field in the CSD register shall be 10
>>> instead of 9.
>>>
>>> This fixes the acceptance test error for the NetBSD 9.0 test of the
>>> Orange Pi PC that has an expanded SD card image of 2 GiB size.
>>>
>>> Fixes: 6d2d4069c47e ("hw/sd: Correct the maximum size of a Standard
>>> Capacity SD Memory Card")
>>> Reported-by: Niek Linnenbank <nieklinnenb...@gmail.com>
>>> Signed-off-by: Bin Meng <bin.m...@windriver.com>
>>> ---
>>>
>>> hw/sd/sd.c | 15 +++++++++++----
>>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index bd10ec8fc4..732fcb5ff0 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -386,10 +386,17 @@ static const uint8_t sd_csd_rw_mask[16] = {
>>>
>>> static void sd_set_csd(SDState *sd, uint64_t size)
>>> {
>>> - uint32_t csize = (size >> (CMULT_SHIFT + HWBLOCK_SHIFT)) - 1;
>>> + int hwblock_shift = HWBLOCK_SHIFT;
>>> + uint32_t csize;
>>> uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
>>> uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
>>>
>>> + /* To indicate 2 GiB card, BLOCK_LEN shall be 1024 bytes */
>>> + if (size == SDSC_MAX_CAPACITY) {
>>> + hwblock_shift += 1;
>>
>> This is going in the good direction, however now we have an huge
>> security hole, as SDState::data[] is 512 bytes, and you announce the
>> guest it can use 1024 bytes. See sd_blk_read() and sd_blk_write().
>
> Currently sd_normal_command() ensures that the maximum block length is
> 512 bytes as the response to cmd 16.
>
> The spec also says in chapter4.3.2 2 GByte Card:
>
> "However, the Block Length, set by CMD16, shall be up to 512 bytes to
> keep consistency with 512 bytes Maximum Block Length cards (Less than
> or equal 2GBytes cards).
>
> I don't see any issue here. Am I missing anything?
You are not missing anything, I was confused by the spec.
Patch applied.
Regards,
Phil.
