| Message ID | 20210728181728.2012952-1-f4bug@amsat.org |
|---|---|
| Headers | show |
| Series | hw/sd/sdcard: Fix assertion accessing out-of-range addresses with CMD30 | expand |
On Wed, 28 Jul 2021 at 19:17, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Fix an assertion reported by OSS-Fuzz, add corresponding qtest. > > The change simple enough for the next rc. > > Philippe Mathieu-Daudé (3): > hw/sd/sdcard: Document out-of-range addresses for SEND_WRITE_PROT > hw/sd/sdcard: Fix assertion accessing out-of-range addresses with > CMD30 > hw/sd/sdcard: Rename Write Protect Group variables I've left review comments on individual patches, but my suspicion is that the fix for this assertion failure is just "the assert should be after the test for 'addr < sd->size', not before", something like: @@ -821,8 +821,12 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr) wpnum = sd_addr_to_wpnum(addr); for (i = 0; i < 32; i++, wpnum++, addr += WPGROUP_SIZE) { + if (addr >= sd->size) { + /* Out of range groups report as zero */ + continue; + } assert(wpnum < sd->wpgrps_size); - if (addr < sd->size && test_bit(wpnum, sd->wp_groups)) { + if (test_bit(wpnum, sd->wp_groups)) { ret |= (1 << i); } } -- PMM
On 8/2/21 2:10 PM, Peter Maydell wrote: > On Wed, 28 Jul 2021 at 19:17, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> Fix an assertion reported by OSS-Fuzz, add corresponding qtest. >> >> The change simple enough for the next rc. >> >> Philippe Mathieu-Daudé (3): >> hw/sd/sdcard: Document out-of-range addresses for SEND_WRITE_PROT >> hw/sd/sdcard: Fix assertion accessing out-of-range addresses with >> CMD30 >> hw/sd/sdcard: Rename Write Protect Group variables > > I've left review comments on individual patches, but my suspicion > is that the fix for this assertion failure is just "the > assert should be after the test for 'addr < sd->size', not before", > something like: > > @@ -821,8 +821,12 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr) > wpnum = sd_addr_to_wpnum(addr); > > for (i = 0; i < 32; i++, wpnum++, addr += WPGROUP_SIZE) { > + if (addr >= sd->size) { > + /* Out of range groups report as zero */ > + continue; > + } > assert(wpnum < sd->wpgrps_size); > - if (addr < sd->size && test_bit(wpnum, sd->wp_groups)) { > + if (test_bit(wpnum, sd->wp_groups)) { > ret |= (1 << i); > } > } It is simpler and works :) Thanks!
Fix an assertion reported by OSS-Fuzz, add corresponding qtest. The change simple enough for the next rc. Philippe Mathieu-Daudé (3): hw/sd/sdcard: Document out-of-range addresses for SEND_WRITE_PROT hw/sd/sdcard: Fix assertion accessing out-of-range addresses with CMD30 hw/sd/sdcard: Rename Write Protect Group variables hw/sd/sd.c | 37 ++++++++++++++++++++-------------- tests/qtest/fuzz-sdcard-test.c | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 15 deletions(-)