On 6/26/20 7:43 PM, Philippe Mathieu-Daudé wrote:
> On 6/26/20 6:40 PM, Philippe Mathieu-Daudé wrote:
>> As a defense, assert if the requested address is out of the card area.
>>
>> Suggested-by: Peter Maydell <[email protected]>
>> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
>> ---
>> hw/sd/sd.c | 18 ++++++++++--------
>> 1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 22392e5084..2689a27b49 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -537,8 +537,10 @@ static void sd_response_r7_make(SDState *sd, uint8_t
>> *response)
>> stl_be_p(response, sd->vhs);
>> }
>>
>> -static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
>> +static uint64_t sd_addr_to_wpnum(SDState *sd, uint64_t addr)
>> {
>> + assert(addr < sd->size);
>
> This should be:
>
> assert(addr <= sd->size);
No, the current code is correct...
>
>> +
>> return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
>> }
>>
>> @@ -575,7 +577,7 @@ static void sd_reset(DeviceState *dev)
>> sd_set_cardstatus(sd);
>> sd_set_sdstatus(sd);
>>
>> - sect = sd_addr_to_wpnum(size) + 1;
>> + sect = sd_addr_to_wpnum(sd, size) + 1;
... but here this should be:
sect = sd_addr_to_wpnum(sd, size - 1) + 1;
>> g_free(sd->wp_groups);
>> sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false;
>> sd->wpgrps_size = sect;
>> @@ -759,8 +761,8 @@ static void sd_erase(SDState *sd)
>> erase_end *= HWBLOCK_SIZE;
>> }
>>
>> - erase_start = sd_addr_to_wpnum(erase_start);
>> - erase_end = sd_addr_to_wpnum(erase_end);
>> + erase_start = sd_addr_to_wpnum(sd, erase_start);
>> + erase_end = sd_addr_to_wpnum(sd, erase_end);
>> sd->erase_start = 0;
>> sd->erase_end = 0;
>> sd->csd[14] |= 0x40;
>> @@ -777,7 +779,7 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
>> uint32_t i, wpnum;
>> uint32_t ret = 0;
>>
>> - wpnum = sd_addr_to_wpnum(addr);
>> + wpnum = sd_addr_to_wpnum(sd, addr);
>>
>> for (i = 0; i < 32; i++, wpnum++, addr += WPGROUP_SIZE) {
>> if (addr < sd->size && test_bit(wpnum, sd->wp_groups)) {
>> @@ -819,7 +821,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
>>
>> static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
>> {
>> - return test_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
>> + return test_bit(sd_addr_to_wpnum(sd, addr), sd->wp_groups);
>> }
>>
>> static void sd_lock_command(SDState *sd)
>> @@ -1331,7 +1333,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>> SDRequest req)
>> }
>>
>> sd->state = sd_programming_state;
>> - set_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
>> + set_bit(sd_addr_to_wpnum(sd, addr), sd->wp_groups);
>> /* Bzzzzzzztt .... Operation complete. */
>> sd->state = sd_transfer_state;
>> return sd_r1b;
>> @@ -1350,7 +1352,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>> SDRequest req)
>> }
>>
>> sd->state = sd_programming_state;
>> - clear_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
>> + clear_bit(sd_addr_to_wpnum(sd, addr), sd->wp_groups);
>> /* Bzzzzzzztt .... Operation complete. */
>> sd->state = sd_transfer_state;
>> return sd_r1b;
>>
>
