Re: [Qemu-devel] [PATCH v2 0/8] fdc: Clean up and fix command processing

[John Snow]

On 05/21/2015 09:19 AM, Kevin Wolf wrote:
> The hotfix for CVE-2015-3456 fixed the security problem, but didn't
> fully correct the behaviour of the emulated floppy controller.  This
> series fixes the bug that was the root cause for the problem, and does
> some cleanup in the FIFO access functions to make the command processing
> more obvious.
> 
> v2:
> - Patch 3: Include fdctrl->phase in the migration state. [Peter]
> - Patch 4: Added a comment to clarify an assertion [Peter]
> - Patch 5: Check pos == 0 instead of fdctrl->data_pos == 1 [John]
> - Patch 7: Improved commit message [John]
> 
> FWIW, when testing this, I found that migration with active I/O on a
> floppy drive doesn't work very reliably. These problems were there
> before the series and they stay after the series. I verified as good
> as I could that the subsection magic does its job, and I'll leave
> fixing the other floppy migration bugs for someone else.
> 
> 
> Kevin Wolf (8):
>   fdc: Rename fdctrl_reset_fifo() to fdctrl_to_command_phase()
>   fdc: Rename fdctrl_set_fifo() to fdctrl_to_result_phase()
>   fdc: Introduce fdctrl->phase
>   fdc: Use phase in fdctrl_write_data()
>   fdc: Code cleanup in fdctrl_write_data()
>   fdc: Disentangle phases in fdctrl_read_data()
>   fdc: Fix MSR.RQM flag
>   fdc-test: Test state for existing cases more thoroughly
> 
>  hw/block/fdc.c   | 296 
> ++++++++++++++++++++++++++++++++++++++++---------------
>  tests/fdc-test.c |  34 +++++++
>  2 files changed, 253 insertions(+), 77 deletions(-)
> 

>From what I can tell, it seems like Kevin's current migration approach
is appropriate for now, regardless of the migration policy debate that's
still ongoing.

It looks okay to me and David Gilbert gave it his ACK, so I have staged
this in my increasingly inaccurately named IDE branch, thanks.

https://github.com/jnsnow/qemu/commits/ide

--js