On 20 January 2016 at 13:14, P J P <[email protected]> wrote: > From: Prasad J Pandit <[email protected]> > > While processing standard SD commands, the 'req.cmd' value could > lead to OOB read when used as an index into 'sd_cmd_type' array. > Limit 'req.cmd' value to avoid such an access. > > Reported-by: Qinghao Tang <[email protected]> > Signed-off-by: Prasad J Pandit <[email protected]> > --- > hw/sd/sd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 1a9935c..b800ced 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -668,8 +668,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, > /* Not interpreting this as an app command */ > sd->card_status &= ~APP_CMD; > > - if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc) > + if (sd_cmd_type[req.cmd & 0x3F] == sd_ac > + || sd_cmd_type[req.cmd & 0x3F] == sd_adtc) { > rca = req.arg >> 16; > + } > > DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state); > switch (req.cmd) {
This isn't the only bit of sd.c that assumes that the controller model hasn't passed it an oversize command number -- sd_do_command() calls cmd_valid_while_locked() which uses req.cmd as an index into sd_cmd_class[]. (I think it's right for sd.c to be defensive about this, since it's one place to get right rather than having to have all the controller models DTRT.) thanks -- PMM
