26ba25
From 63cee29a64ac6a02695a91f7a8f29ac0c17ef3f0 Mon Sep 17 00:00:00 2001
26ba25
From: Paolo Bonzini <pbonzini@redhat.com>
26ba25
Date: Tue, 26 Feb 2019 14:44:14 +0000
26ba25
Subject: [PATCH] scsi-generic: avoid possible out-of-bounds access to r->buf
26ba25
26ba25
RH-Author: Paolo Bonzini <pbonzini@redhat.com>
26ba25
Message-id: <20190226144414.5700-1-pbonzini@redhat.com>
26ba25
Patchwork-id: 84717
26ba25
O-Subject: [RHEL8.0 qemu-kvm PATCH] scsi-generic: avoid possible out-of-bounds access to r->buf
26ba25
Bugzilla: 1668162
26ba25
RH-Acked-by: Thomas Huth <thuth@redhat.com>
26ba25
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
26ba25
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
26ba25
26ba25
Bugzilla: 1668162
26ba25
26ba25
Brew build: 20372796
26ba25
26ba25
Whenever the allocation length of a SCSI request is shorter than the size of the
26ba25
VPD page list, page_idx is used blindly to index into r->buf.  Even though
26ba25
the stores in the insertion sort are protected against overflows, the same is not
26ba25
true of the reads and the final store of 0xb0.
26ba25
26ba25
This basically does the same thing as commit 57dbb58d80 ("scsi-generic: avoid
26ba25
out-of-bounds access to VPD page list", 2018-11-06), except that here the
26ba25
allocation length can be chosen by the guest.  Note that according to the SCSI
26ba25
standard, the contents of the PAGE LENGTH field are not altered based
26ba25
on the allocation length.
26ba25
26ba25
The code was introduced by commit 6c219fc8a1 ("scsi-generic: keep VPD
26ba25
page list sorted", 2018-11-06) but the overflow was already possible before.
26ba25
26ba25
Reported-by: Kevin Wolf <kwolf@redhat.com>
26ba25
Fixes: a71c775b24ebc664129eb1d9b4c360590353efd5
26ba25
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
26ba25
(cherry picked from commit e909ff93698851777faac3c45d03c1b73f311ea6)
26ba25
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
26ba25
---
26ba25
 hw/scsi/scsi-generic.c | 18 ++++++++++--------
26ba25
 1 file changed, 10 insertions(+), 8 deletions(-)
26ba25
26ba25
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
26ba25
index 4ac53e4..e21adf9 100644
26ba25
--- a/hw/scsi/scsi-generic.c
26ba25
+++ b/hw/scsi/scsi-generic.c
26ba25
@@ -183,7 +183,7 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s)
26ba25
             /* Also take care of the opt xfer len. */
26ba25
             stl_be_p(&r->buf[12],
26ba25
                     MIN_NON_ZERO(max_transfer, ldl_be_p(&r->buf[12])));
26ba25
-        } else if (s->needs_vpd_bl_emulation && page == 0x00) {
26ba25
+        } else if (s->needs_vpd_bl_emulation && page == 0x00 && r->buflen >= 4) {
26ba25
             /*
26ba25
              * Now we're capable of supplying the VPD Block Limits
26ba25
              * response if the hardware can't. Add it in the INQUIRY
26ba25
@@ -194,18 +194,20 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s)
26ba25
              * and will use it to proper setup the SCSI device.
26ba25
              *
26ba25
              * VPD page numbers must be sorted, so insert 0xb0 at the
26ba25
-             * right place with an in-place insert.  After the initialization
26ba25
-             * part of the for loop is executed, the device response is
26ba25
-             * at r[0] to r[page_idx - 1].
26ba25
+             * right place with an in-place insert.  When the while loop
26ba25
+             * begins the device response is at r[0] to r[page_idx - 1].
26ba25
              */
26ba25
-            for (page_idx = lduw_be_p(r->buf + 2) + 4;
26ba25
-                 page_idx > 4 && r->buf[page_idx - 1] >= 0xb0;
26ba25
-                 page_idx--) {
26ba25
+            page_idx = lduw_be_p(r->buf + 2) + 4;
26ba25
+            page_idx = MIN(page_idx, r->buflen);
26ba25
+            while (page_idx > 4 && r->buf[page_idx - 1] >= 0xb0) {
26ba25
                 if (page_idx < r->buflen) {
26ba25
                     r->buf[page_idx] = r->buf[page_idx - 1];
26ba25
                 }
26ba25
+                page_idx--;
26ba25
+            }
26ba25
+            if (page_idx < r->buflen) {
26ba25
+                r->buf[page_idx] = 0xb0;
26ba25
             }
26ba25
-            r->buf[page_idx] = 0xb0;
26ba25
             stw_be_p(r->buf + 2, lduw_be_p(r->buf + 2) + 1);
26ba25
         }
26ba25
     }
26ba25
-- 
26ba25
1.8.3.1
26ba25