From dc4ff14029538f4f2787271b98d6e8e403cbfcc5 Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Fri, 26 Feb 2021 01:06:42 -0500
Subject: [PATCH] ide: atapi: check logical block address and read size
 (CVE-2020-29443)

RH-Author: Jon Maloy <jmaloy@redhat.com>
Message-id: <20210226010642.3200257-2-jmaloy@redhat.com>
Patchwork-id: 101270
O-Subject: [RHEL-7.9.z qemu-kvm PATCH v2 1/1] ide: atapi: check logical block address and read size (CVE-2020-29443)
Bugzilla: 1917449
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
RH-Acked-by: Danilo de Paula <ddepaula@redhat.com>
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

From: Prasad J Pandit <pjp@fedoraproject.org>

While processing ATAPI cmd_read/cmd_read_cd commands,
Logical Block Address (LBA) maybe invalid OR closer to the last block,
leading to an OOB access issues. Add range check to avoid it.

Fixes: CVE-2020-29443
Reported-by: Wenxiang Qian <leonwxqian@gmail.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-Id: <20210118115130.457044-1-ppandit@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

(cherry picked from commit b8d7f1bc59276fec85e4d09f1567613a3e14d31e)
Conflict: There is a conflict in cmd_read_cd(), because
commit e7bd708ec85e ("atapi: classify read_cd as conditionally
returning data") is missing in this code version. That seems to be an
unrelated fix that drags in further changes, so instead of applying it
we choose to adapt the commit directly to the current code version.
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: Jon Maloy <jmaloy.redhat.com>
---
 hw/ide/atapi.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 490070a1b4..bef74914d1 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -267,6 +267,8 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size)
 static void ide_atapi_cmd_read_pio(IDEState *s, int lba, int nb_sectors,
                                    int sector_size)
 {
+    assert(0 <= lba && lba < (s->nb_sectors >> 2));
+
     s->lba = lba;
     s->packet_transfer_size = nb_sectors * sector_size;
     s->elementary_transfer_size = 0;
@@ -365,6 +367,8 @@ eot:
 static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors,
                                    int sector_size)
 {
+    assert(0 <= lba && lba < (s->nb_sectors >> 2));
+
     s->lba = lba;
     s->packet_transfer_size = nb_sectors * sector_size;
     s->io_buffer_index = 0;
@@ -823,7 +827,10 @@ static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
 
 static void cmd_read(IDEState *s, uint8_t* buf)
 {
-    int nb_sectors, lba;
+    unsigned int nb_sectors, lba;
+
+    /* Total logical sectors of ATAPI_SECTOR_SIZE(=2048) bytes */
+    uint64_t total_sectors = s->nb_sectors >> 2;
 
     if (buf[0] == GPCMD_READ_10) {
         nb_sectors = ube16_to_cpu(buf + 7);
@@ -831,27 +838,39 @@ static void cmd_read(IDEState *s, uint8_t* buf)
         nb_sectors = ube32_to_cpu(buf + 6);
     }
 
-    lba = ube32_to_cpu(buf + 2);
     if (nb_sectors == 0) {
         ide_atapi_cmd_ok(s);
         return;
     }
 
+    lba = ldl_be_p(buf + 2);
+    if (lba >= total_sectors || lba + nb_sectors - 1 >= total_sectors) {
+        ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR);
+        return;
+    }
+
     ide_atapi_cmd_read(s, lba, nb_sectors, 2048);
 }
 
 static void cmd_read_cd(IDEState *s, uint8_t* buf)
 {
-    int nb_sectors, lba, transfer_request;
+    unsigned int nb_sectors, lba, transfer_request;
 
-    nb_sectors = (buf[6] << 16) | (buf[7] << 8) | buf[8];
-    lba = ube32_to_cpu(buf + 2);
+    /* Total logical sectors of ATAPI_SECTOR_SIZE(=2048) bytes */
+    uint64_t total_sectors = s->nb_sectors >> 2;
 
+    nb_sectors = (buf[6] << 16) | (buf[7] << 8) | buf[8];
     if (nb_sectors == 0) {
         ide_atapi_cmd_ok(s);
         return;
     }
 
+    lba = ldl_be_p(buf + 2);
+    if (lba >= total_sectors || lba + nb_sectors - 1 >= total_sectors) {
+        ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR);
+        return;
+    }
+
     transfer_request = buf[9];
     switch(transfer_request & 0xf8) {
     case 0x00:
-- 
2.18.2