[Qemu-commits] [qemu/qemu] 07e415: fdc: Rename fdctrl_reset_fifo() to fdctrl_to_comma...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 07e415f2398d9cfb21cdd5ef902445032ba54556
      
https://github.com/qemu/qemu/commit/07e415f2398d9cfb21cdd5ef902445032ba54556
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Rename fdctrl_reset_fifo() to fdctrl_to_command_phase()

What all callers of fdctrl_reset_fifo() really want to do is to start
the command phase, where writes to the data port initiate a new command.

The function doesn't only clear the FIFO, but also sets up the state so
that a new command can be received. Rename it to reflect this.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 83a260135f13db8b5d7df72090864a5ebcef2845
      
https://github.com/qemu/qemu/commit/83a260135f13db8b5d7df72090864a5ebcef2845
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Rename fdctrl_set_fifo() to fdctrl_to_result_phase()

What callers really do with this function is to switch from execution
phase (including data transfers) to result phase where the guest can
read out one or more status bytes from the FIFO (the number depends on
the command).

Rename the function accordingly.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 85d291a08c91c07927bbbd29f72a27d3ad7478f3
      
https://github.com/qemu/qemu/commit/85d291a08c91c07927bbbd29f72a27d3ad7478f3
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Introduce fdctrl->phase

The floppy controller spec describes three different controller phases,
which are currently not explicitly modelled in our emulation. Instead,
each phase is represented by a combination of flags in registers.

This patch makes explicit in which phase the controller currently is.

Signed-off-by: Kevin Wolf <address@hidden>
Acked-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 5b0a25e8d2f15f89255c745c71d297b5b24d138c
      
https://github.com/qemu/qemu/commit/5b0a25e8d2f15f89255c745c71d297b5b24d138c
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Use phase in fdctrl_write_data()

Instead of relying on a flag in the MSR to distinguish controller phases,
use the explicit phase that we store now. Assertions of the right MSR
flags are added.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: d275b33d76c8ed9d5a3dca22ea0fdec8d5a5c8e6
      
https://github.com/qemu/qemu/commit/d275b33d76c8ed9d5a3dca22ea0fdec8d5a5c8e6
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Code cleanup in fdctrl_write_data()

Factor out a few common lines of code, reformat, improve comments.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: f6c2d1d8425fd0ca450d515b06821e2224d4b43c
      
https://github.com/qemu/qemu/commit/f6c2d1d8425fd0ca450d515b06821e2224d4b43c
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Disentangle phases in fdctrl_read_data()

This commit makes similar improvements as have already been made to the
write function: Instead of relying on a flag in the MSR to distinguish
controller phases, use the explicit phase that we store now. Assertions
of the right MSR flags are added.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 6cc8a11c84ddc18c64fc88d54c8e9dca24ada489
      
https://github.com/qemu/qemu/commit/6cc8a11c84ddc18c64fc88d54c8e9dca24ada489
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: Fix MSR.RQM flag

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>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 4964e18e490f3ecad35c9e4cc9b613316a98755e
      
https://github.com/qemu/qemu/commit/4964e18e490f3ecad35c9e4cc9b613316a98755e
  Author: Kevin Wolf <address@hidden>
  Date:   2015-06-02 (Tue, 02 Jun 2015)

  Changed paths:
    M tests/fdc-test.c

  Log Message:
  -----------
  fdc-test: Test state for existing cases more thoroughly

This just adds a few additional checks of the MSR and interrupt pin to
the already existing test cases.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 0389b8f8c7688fe512e16bdc00c5f35d2d8df12c
      
https://github.com/qemu/qemu/commit/0389b8f8c7688fe512e16bdc00c5f35d2d8df12c
  Author: Mark Cave-Ayland <address@hidden>
  Date:   2015-06-04 (Thu, 04 Jun 2015)

  Changed paths:
    M hw/ide/macio.c

  Log Message:
  -----------
  macio: switch pmac_dma_read() over to new offset/len implementation

For better handling of unaligned block device accesses.

Signed-off-by: Mark Cave-Ayland <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: ac58fe7b2c67a9be142beacd4c6ee51f3264d90f
      
https://github.com/qemu/qemu/commit/ac58fe7b2c67a9be142beacd4c6ee51f3264d90f
  Author: Mark Cave-Ayland <address@hidden>
  Date:   2015-06-04 (Thu, 04 Jun 2015)

  Changed paths:
    M hw/ide/macio.c
    M include/hw/ppc/mac_dbdma.h

  Log Message:
  -----------
  macio: switch pmac_dma_write() over to new offset/len implementation

In particular, this fixes a bug whereby chains of overlapping head/tail chains
would incorrectly write over each other's remainder cache. This is the access
pattern used by OS X/Darwin and fixes an issue with a corrupt Darwin
installation in my local tests.

While we are here, rename the DBDMA_io struct property remainder to
head_remainder for clarification.

Signed-off-by: Mark Cave-Ayland <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: b01d44cd0623dec66e583d6cd2438451443261df
      
https://github.com/qemu/qemu/commit/b01d44cd0623dec66e583d6cd2438451443261df
  Author: Mark Cave-Ayland <address@hidden>
  Date:   2015-06-04 (Thu, 04 Jun 2015)

  Changed paths:
    M hw/ide/macio.c

  Log Message:
  -----------
  macio: update comment/constants to reflect the new code

With the offset/len functions taking care of all of the alignment mapping
in isolation from the DMA tranasaction, many comments are now unnecessary.
Remove these and tidy up a few constants at the same time.

Signed-off-by: Mark Cave-Ayland <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 0ba98885a0e965a17df214ab12b819ef630d8a14
      
https://github.com/qemu/qemu/commit/0ba98885a0e965a17df214ab12b819ef630d8a14
  Author: Mark Cave-Ayland <address@hidden>
  Date:   2015-06-04 (Thu, 04 Jun 2015)

  Changed paths:
    M hw/ide/macio.c
    M include/hw/ppc/mac_dbdma.h

  Log Message:
  -----------
  macio: remove remainder_len DBDMA_io property

Since the block alignment code is now effectively independent of the DMA
implementation, this variable is no longer required and can be removed.

Signed-off-by: Mark Cave-Ayland <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 2e29dd7c44db30e3d3c108ab2a622cbdac6d16f0
      
https://github.com/qemu/qemu/commit/2e29dd7c44db30e3d3c108ab2a622cbdac6d16f0
  Author: Peter Maydell <address@hidden>
  Date:   2015-06-08 (Mon, 08 Jun 2015)

  Changed paths:
    M hw/block/fdc.c
    M hw/ide/macio.c
    M include/hw/ppc/mac_dbdma.h
    M tests/fdc-test.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into 
staging

# gpg: Signature made Fri Jun  5 20:59:07 2015 BST using RSA key ID AAFC390E
# gpg: Good signature from "John Snow (John Huston) <address@hidden>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: FAEB 9711 A12C F475 812F  18F2 88A9 064D 1835 61EB
#      Subkey fingerprint: F9B7 ABDB BCAC DF95 BE76  CBD0 7DEF 8106 AAFC 390E

* remotes/jnsnow/tags/ide-pull-request:
  macio: remove remainder_len DBDMA_io property
  macio: update comment/constants to reflect the new code
  macio: switch pmac_dma_write() over to new offset/len implementation
  macio: switch pmac_dma_read() over to new offset/len implementation
  fdc-test: Test state for existing cases more thoroughly
  fdc: Fix MSR.RQM flag
  fdc: Disentangle phases in fdctrl_read_data()
  fdc: Code cleanup in fdctrl_write_data()
  fdc: Use phase in fdctrl_write_data()
  fdc: Introduce fdctrl->phase
  fdc: Rename fdctrl_set_fifo() to fdctrl_to_result_phase()
  fdc: Rename fdctrl_reset_fifo() to fdctrl_to_command_phase()

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/0daba1f037ab...2e29dd7c44db