[Qemu-devel] [PATCH v2] sd: limit 'req.cmd' while using as an array index

[Qemu-devel] [PATCH v2] sd: limit 'req.cmd' while using as an array index

[P J P]

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' or
'sd_cmd_class' arrays. 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 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Update as per review:
  -> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03769.html

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1a9935c..5ee3e54 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) {
@@ -1340,7 +1342,8 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
     if (req->cmd == 16 || req->cmd == 55) {
         return 1;
     }
-    return sd_cmd_class[req->cmd] == 0 || sd_cmd_class[req->cmd] == 7;
+    return sd_cmd_class[req->cmd & 0x3F] == 0
+            || sd_cmd_class[req->cmd & 0x3F] == 7;
 }
 
 int sd_do_command(SDState *sd, SDRequest *req,
-- 
2.5.0

Re: [Qemu-devel] [PATCH v2] sd: limit 'req.cmd' while using as an array index

[Peter Maydell]

On 20 January 2016 at 18:50, 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' or
> 'sd_cmd_class' arrays. 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 | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> Update as per review:
>   -> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03769.html

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] sd: limit 'req.cmd' while using as an array index

[Peter Maydell]

On 20 January 2016 at 18:50, 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' or
> 'sd_cmd_class' arrays. 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>



Applied to target-arm.next, thanks.

-- PMM