diff mbox

fdc: force the fifo access to be in bounds of the allocated buffer

Message ID 1431527602-29889-2-git-send-email-jsnow@redhat.com
State New
Headers show

Commit Message

John Snow May 13, 2015, 2:33 p.m. UTC 
From: Petr Matousek <pmatouse@redhat.com>

During processing of certain commands such as FD_CMD_READ_ID and
FD_CMD_DRIVE_SPECIFICATION_COMMAND the fifo memory access could
get out of bounds leading to memory corruption with values coming
from the guest.

Fix this by making sure that the index is always bounded by the
allocated memory.

This is CVE-2015-3456.

Signed-off-by: Petr Matousek <pmatouse@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

John Snow May 13, 2015, 2:35 p.m. UTC | #1 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 05/13/2015 10:33 AM, John Snow wrote:
> From: Petr Matousek <pmatouse@redhat.com>
> 
> During processing of certain commands such as FD_CMD_READ_ID and 
> FD_CMD_DRIVE_SPECIFICATION_COMMAND the fifo memory access could get
> out of bounds leading to memory corruption with values coming from
> the guest.
> 
> Fix this by making sure that the index is always bounded by the 
> allocated memory.
> 
> This is CVE-2015-3456.
> 
> Signed-off-by: Petr Matousek <pmatouse@redhat.com> Reviewed-by:
> John Snow <jsnow@redhat.com> Signed-off-by: John Snow
> <jsnow@redhat.com> ---
[snip]

Already sent the pull request (at 08:00 EDT this morning) for
inclusion in the master branch, but this will serve as the formal
patch discussion / and request for inclusion into any stable branches
still being maintained.

Thanks.

- --John Snow
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVU2EtAAoJEH3vgQaq/DkO+ogP/1D1W2F4hbqV+CDakrCLJagz
wC/XiGmixY+CUHr8z+OjXLtJLkSj2HprdbY3S1ogeJUOLXHUePYGBBEwjjH/Ed7b
TPYjzfEZlmw5UzMIGOIIZfHtOA5Xzsq0Ipqk5PXOXyprm0aDji9ZMwRTkdTbwuYI
kBps6ajkHNkzxIIRO11aWJjiRo0CfIEFZgLrYRVdtixzfgeEHJRfGJJvOA3VIrwD
5yS2tjgpkrj4C4tO/gdOeOUfmiwh5IjSHPVwgEkTABZxe4FFxEs9oGuReKyZFcq9
/60nqJ689+JxMxoPtPcQDvwf9tSmOWG1RRe3m+NwhY3lLuIhmpIDnjABSvFJhUye
v9gd52jf/mOO557iUh/I8JbdZLc8NPcR8C9JC1zGewYFk7lKEsVUUaAyw1QkrrVa
7GfpjjnXeys8HkBgNNmjtLnq6V15rFA5B8Oc0yyhSRXZimIIkF6C+G8pnv8GdonL
n7Sm1nsFnhVeinK37dSDMHBqKqRKGyJE6HRGniP9xMluycxf9mtNMKpBmPmmTHPd
QjjScqrWQTJd12Hlzsh7HnoNNBQ/nG6Om45/PKsoVWaByc7d7XQ0yw3BI3xLxQMb
yzmstCgAg5K+pbt2MJsPBMJCCOuda2scCSWAWVFAX306sdcV5ZUhr6wpnhlCV1lI
UEjPHAmhLUUqrZDQHuH0
=gUNa
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f72a392..d8a8edd 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1497,7 +1497,7 @@  static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
 {
     FDrive *cur_drv;
     uint32_t retval = 0;
-    int pos;
+    uint32_t pos;
 
     cur_drv = get_cur_drv(fdctrl);
     fdctrl->dsr &= ~FD_DSR_PWRDOWN;
@@ -1506,8 +1506,8 @@  static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
         return 0;
     }
     pos = fdctrl->data_pos;
+    pos %= FD_SECTOR_LEN;
     if (fdctrl->msr & FD_MSR_NONDMA) {
-        pos %= FD_SECTOR_LEN;
         if (pos == 0) {
             if (fdctrl->data_pos != 0)
                 if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
@@ -1852,10 +1852,13 @@  static void fdctrl_handle_option(FDCtrl *fdctrl, int direction)
 static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direction)
 {
     FDrive *cur_drv = get_cur_drv(fdctrl);
+    uint32_t pos;
 
-    if (fdctrl->fifo[fdctrl->data_pos - 1] & 0x80) {
+    pos = fdctrl->data_pos - 1;
+    pos %= FD_SECTOR_LEN;
+    if (fdctrl->fifo[pos] & 0x80) {
         /* Command parameters done */
-        if (fdctrl->fifo[fdctrl->data_pos - 1] & 0x40) {
+        if (fdctrl->fifo[pos] & 0x40) {
             fdctrl->fifo[0] = fdctrl->fifo[1];
             fdctrl->fifo[2] = 0;
             fdctrl->fifo[3] = 0;
@@ -1955,7 +1958,7 @@  static uint8_t command_to_handler[256];
 static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
 {
     FDrive *cur_drv;
-    int pos;
+    uint32_t pos;
 
     /* Reset mode */
     if (!(fdctrl->dor & FD_DOR_nRESET)) {
@@ -2004,7 +2007,9 @@  static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
     }
 
     FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
-    fdctrl->fifo[fdctrl->data_pos++] = value;
+    pos = fdctrl->data_pos++;
+    pos %= FD_SECTOR_LEN;
+    fdctrl->fifo[pos] = value;
     if (fdctrl->data_pos == fdctrl->data_len) {
         /* We now have all parameters
          * and will be able to treat the command