[pve-devel] [PATCH] pve-qemu-kvm: fix VENOM qemu security flaw (CVE-2015-3456)

[Stefan Priebe]

Am 14.05.2015 um 08:22 schrieb Dietmar Maurer:
> We not not really support FDC, so this is not really high priority (why is it)?

This does not matter. The controller is always there. An the bug is in 
the controller. See the discussion on the qemu mailing list.

Stefan

>
>> On May 13, 2015 at 7:14 PM Stefan Priebe <s.priebe at profihost.ag> wrote:
>>
>>
>>
>> Signed-off-by: Stefan Priebe <s.priebe at profihost.ag>
>> ---
>>   ...he-fifo-access-to-be-in-bounds-of-the-all.patch |   87
>> ++++++++++++++++++++
>>   debian/patches/series                              |    1 +
>>   2 files changed, 88 insertions(+)
>>   create mode 100644
>> debian/patches/0001-fdc-force-the-fifo-access-to-be-in-bounds-of-the-all.patch
>>
>> diff --git
>> a/debian/patches/0001-fdc-force-the-fifo-access-to-be-in-bounds-of-the-all.patch
>> b/debian/patches/0001-fdc-force-the-fifo-access-to-be-in-bounds-of-the-all.patch
>> new file mode 100644
>> index 0000000..d28b0ea
>> --- /dev/null
>> +++
>> b/debian/patches/0001-fdc-force-the-fifo-access-to-be-in-bounds-of-the-all.patch
>> @@ -0,0 +1,87 @@
>> +From 8cb8ea32396f52e01cdb38c00b5b7f5a0141f97f Mon Sep 17 00:00:00 2001
>> +From: Petr Matousek <pmatouse at redhat.com>
>> +Date: Wed, 6 May 2015 09:48:59 +0200
>> +Subject: [PATCH] fdc: force the fifo access to be in bounds of the allocated
>> + buffer
>> +
>> +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 at redhat.com>
>> +Reviewed-by: John Snow <jsnow at redhat.com>
>> +Signed-off-by: John Snow <jsnow at redhat.com>
>> +(cherry picked from commit e907746266721f305d67bc0718795fedee2e824c)
>> +Signed-off-by: Stefan Priebe <s.priebe at profihost.ag>
>> +---
>> + hw/block/fdc.c |   17 +++++++++++------
>> + 1 file changed, 11 insertions(+), 6 deletions(-)
>> +
>> +diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> +index 739a03e..1168e8c 100644
>> +--- a/hw/block/fdc.c
>> ++++ b/hw/block/fdc.c
>> +@@ -1512,7 +1512,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;
>> +@@ -1521,8 +1521,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)) {
>> +@@ -1867,10 +1867,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;
>> +@@ -1970,7 +1973,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)) {
>> +@@ -2019,7 +2022,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
>> +--
>> +1.7.10.4
>> +
>> diff --git a/debian/patches/series b/debian/patches/series
>> index 51b8c2f..436a07d 100644
>> --- a/debian/patches/series
>> +++ b/debian/patches/series
>> @@ -34,3 +34,4 @@ virtio-balloon-dimmfix2.patch
>>   virtio-balloon-dimmfix3.patch
>>   add-qmp-get-link-status.patch
>>   virtio-scsi_fix_assert.patch
>> +0001-fdc-force-the-fifo-access-to-be-in-bounds-of-the-all.patch
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>