Message ID | 20200520152450.200362-2-ppandit@redhat.com |
---|---|
State | New |
Headers | show |
Series | avoid OOB access in SD card emulator | expand |
Hi Prasad, On 5/20/20 5:24 PM, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > SD card emulator sets 'sd->card_status' while performing block > write commands. While doing so, it tests the corresponding bit > derived from 's->data_start' address. This may lead to OOB access. > Add check to avoid it. Ah, this is different that the one reported recently: https://bugs.launchpad.net/qemu/+bug/1878054 Do you have a reproducer? Is this a CVE? > > Reported-by: Alex <alxndr@bu.edu> This is not Alexander complete name. > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/sd/sd.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 71a9af09ab..916e9fff58 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -802,7 +802,12 @@ 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); > + uint64_t bit = sd_addr_to_wpnum(addr); > + > + if (bit < sd->wpgrps_size) { This should never be called with a such address, so I'd simply use an assertion here. The problem is earlier where the address should be validated and a protocol error returned. > + return test_bit(bit, sd->wp_groups); > + } > + return true; > } > > static void sd_lock_command(SDState *sd) >
diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 71a9af09ab..916e9fff58 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -802,7 +802,12 @@ 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); + uint64_t bit = sd_addr_to_wpnum(addr); + + if (bit < sd->wpgrps_size) { + return test_bit(bit, sd->wp_groups); + } + return true; } static void sd_lock_command(SDState *sd)