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