* [Qemu-devel] [PATCH] sd: limit sd_cmd_type array index 'req.cmd'
@ 2016-01-20 13:14 P J P
2016-01-20 14:00 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: P J P @ 2016-01-20 13:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Qinghao Tang, Peter Maydell, Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
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 <luodalongde@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
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) {
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] sd: limit sd_cmd_type array index 'req.cmd'
2016-01-20 13:14 [Qemu-devel] [PATCH] sd: limit sd_cmd_type array index 'req.cmd' P J P
@ 2016-01-20 14:00 ` Peter Maydell
2016-01-20 18:53 ` P J P
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2016-01-20 14:00 UTC (permalink / raw)
To: P J P; +Cc: Qinghao Tang, QEMU Developers, Prasad J Pandit
On 20 January 2016 at 13:14, P J P <ppandit@redhat.com> wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> 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 <luodalongde@gmail.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] sd: limit sd_cmd_type array index 'req.cmd'
2016-01-20 14:00 ` Peter Maydell
@ 2016-01-20 18:53 ` P J P
0 siblings, 0 replies; 3+ messages in thread
From: P J P @ 2016-01-20 18:53 UTC (permalink / raw)
To: Peter Maydell; +Cc: Qinghao Tang, QEMU Developers
+-- On Wed, 20 Jan 2016, Peter Maydell wrote --+
| 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[].
Ah yes; I looked for 'req.cmd', but missed 'req->cmd'. I've sent a revised
patch v2.
Thank you.
--
- P J P
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-20 18:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-20 13:14 [Qemu-devel] [PATCH] sd: limit sd_cmd_type array index 'req.cmd' P J P
2016-01-20 14:00 ` Peter Maydell
2016-01-20 18:53 ` P J P
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.