[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 7/8] fdc: Fix MSR.RQM flag
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v2 7/8] fdc: Fix MSR.RQM flag |
Date: |
Thu, 21 May 2015 17:27:23 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 05/21/2015 09:19 AM, Kevin Wolf wrote:
> 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(-)
>
Reviewed-by: John Snow <address@hidden>
- [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, 2015/05/21
- Re: [Qemu-devel] [PATCH v2 7/8] fdc: Fix MSR.RQM flag,
John Snow <=
- [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
- Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase, Peter Maydell, 2015/05/29