[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 7/8] fdc: Fix MSR.RQM flag
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PATCH v2 7/8] fdc: Fix MSR.RQM flag |
Date: |
Thu, 21 May 2015 15:19:37 +0200 |
The RQM bit in MSR should be set whenever the guest is supposed to
access the FIFO, and it should be cleared in all other cases. This is
important so the guest can't continue writing/reading the FIFO beyond
the length that it's suppossed to access (see CVE-2015-3456).
Commit e9077462 fixed the CVE by adding code that avoids the buffer
overflow; however it doesn't correct the wrong behaviour of the floppy
controller which should already have cleared RQM.
Currently, RQM stays set all the time and during all phases while a
command is being processed. This is error-prone because the command has
to explicitly clear the flag if it doesn't need data (and indeed, the
two buggy commands that are the culprits for the CVE just forgot to do
that).
This patch clears RQM immediately as soon as all bytes that are expected
have been received. If the the FIFO is used in the next phase, the flag
has to be set explicitly there.
It also clear RQM after receiving all bytes even if the phase transition
immediately sets it again. While it's technically not necessary at the
moment because the state between clearing and setting RQM is not
observable by the guest, this is more explicit and matches how real
hardware works. It will actually become necessary in qemu once
asynchronous code paths are introduced.
This alone should have been enough to fix the CVE, but now we have two
lines of defense - even better.
Signed-off-by: Kevin Wolf <address@hidden>
---
hw/block/fdc.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 3c64194..6e79459 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1223,7 +1223,9 @@ static void fdctrl_to_command_phase(FDCtrl *fdctrl)
fdctrl->phase = FD_PHASE_COMMAND;
fdctrl->data_dir = FD_DIR_WRITE;
fdctrl->data_pos = 0;
+ fdctrl->data_len = 1; /* Accept command byte, adjust for params later */
fdctrl->msr &= ~(FD_MSR_CMDBUSY | FD_MSR_DIO);
+ fdctrl->msr |= FD_MSR_RQM;
}
/* Update the state to allow the guest to read out the command status.
@@ -1438,7 +1440,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int
direction)
}
}
FLOPPY_DPRINTF("start non-DMA transfer\n");
- fdctrl->msr |= FD_MSR_NONDMA;
+ fdctrl->msr |= FD_MSR_NONDMA | FD_MSR_RQM;
if (direction != FD_DIR_WRITE)
fdctrl->msr |= FD_MSR_DIO;
/* IO based transfer: calculate len */
@@ -1618,6 +1620,7 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
}
if (++fdctrl->data_pos == fdctrl->data_len) {
+ fdctrl->msr &= ~FD_MSR_RQM;
fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
}
break;
@@ -1625,6 +1628,7 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
case FD_PHASE_RESULT:
assert(!(fdctrl->msr & FD_MSR_NONDMA));
if (++fdctrl->data_pos == fdctrl->data_len) {
+ fdctrl->msr &= ~FD_MSR_RQM;
fdctrl_to_command_phase(fdctrl);
fdctrl_reset_irq(fdctrl);
}
@@ -2094,6 +2098,10 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t
value)
pos %= FD_SECTOR_LEN;
fdctrl->fifo[pos] = value;
+ if (fdctrl->data_pos == fdctrl->data_len) {
+ fdctrl->msr &= ~FD_MSR_RQM;
+ }
+
switch (fdctrl->phase) {
case FD_PHASE_EXECUTION:
/* For DMA requests, RQM should be cleared during execution phase, so
@@ -2132,6 +2140,9 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t
value)
* as many parameters as this command requires. */
cmd = get_command(value);
fdctrl->data_len = cmd->parameters + 1;
+ if (cmd->parameters) {
+ fdctrl->msr |= FD_MSR_RQM;
+ }
fdctrl->msr |= FD_MSR_CMDBUSY;
}
--
1.8.3.1
- [Qemu-devel] [PATCH v2 0/8] fdc: Clean up and fix command processing, Kevin Wolf, 2015/05/21
- [Qemu-devel] [PATCH v2 1/8] fdc: Rename fdctrl_reset_fifo() to fdctrl_to_command_phase(), Kevin Wolf, 2015/05/21
- [Qemu-devel] [PATCH v2 5/8] fdc: Code cleanup in fdctrl_write_data(), Kevin Wolf, 2015/05/21
- [Qemu-devel] [PATCH v2 2/8] fdc: Rename fdctrl_set_fifo() to fdctrl_to_result_phase(), Kevin Wolf, 2015/05/21
- [Qemu-devel] [PATCH v2 4/8] fdc: Use phase in fdctrl_write_data(), Kevin Wolf, 2015/05/21
- [Qemu-devel] [PATCH v2 6/8] fdc: Disentangle phases in fdctrl_read_data(), Kevin Wolf, 2015/05/21
- [Qemu-devel] [PATCH v2 7/8] fdc: Fix MSR.RQM flag,
Kevin Wolf <=
- [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Kevin Wolf, 2015/05/21
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, John Snow, 2015/05/21
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Dr. David Alan Gilbert, 2015/05/28
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Markus Armbruster, 2015/05/29
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Dr. David Alan Gilbert, 2015/05/29
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Kevin Wolf, 2015/05/29
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Dr. David Alan Gilbert, 2015/05/29
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Kevin Wolf, 2015/05/29
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Dr. David Alan Gilbert, 2015/05/29