[PATCH 00/15] hw/ide: fix the logical CHS translation a guest selects

"Denis V. Lunev" <[email protected]>
Newsgroups gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu
Message-ID <[email protected]>
INITIALIZE DEVICE PARAMETERS lets a guest replace the logical CHS
translation that turns the CHS registers into an LBA. cmd_specify() has
implemented it since 176e4961bb33, released in v7.2, and the geometry it
stores is mishandled four ways.

It is not validated: zero sectors per logical track kills QEMU with SIGFPE
in ide_set_sector(), which is issue 2399, and since handle_cmd() takes the
count from a 16 bit field of the register FIS, an AHCI guest can drive
ide_get_sector() into an int overflow as well. It is not reported:
IDENTIFY DEVICE is built once and cached, so both the words describing the
default geometry and the words describing the current one go wrong. It
does not travel, leaving a migrated guest addressing the disk in a
translation the destination does not have. And it does not revert on a
hardware reset, which ATA-5 9.1 requires, so it outlives the reset of the
machine it was selected on.

The two subsections patches 8 and 9 add are the only change to the
migration stream, and both are sent only when the guest replaced the
default geometry. An unaffected guest migrates to an older QEMU as before;
an affected one now fails the load on the unknown subsection instead of
silently addressing the wrong sectors.

Tested with ide-test (25 cases), ahci-test (74), a full make check, a
clang build, and real guests on both controllers: FreeDOS reading a FAT16
disk through INT 13h CHS, Red Hat Linux 9 whose 2.4 IDE driver prints the
geometry it read, and Ubuntu 26.04 over libata, each with a CD and a disk,
plus a machine reset in the middle.

Signed-off-by: Denis V. Lunev <[email protected]>
Cc: John Snow <[email protected]>
Cc: Peter Maydell <[email protected]>

Denis V. Lunev (15):
  hw/ide: reject an unsupported CHS translation
  tests/qtest/ide-test: cover a CHS translation with zero sectors
  tests/qtest/libqos/ahci: allow a count and an expected error
  tests/qtest/ahci: cover the sector count of INITIALIZE DEVICE
    PARAMETERS
  hw/ide: report the default CHS translation in IDENTIFY DEVICE
  hw/ide: keep the IDENTIFY DEVICE current geometry in sync
  hw/ide: restore the power-on device state before loading
  hw/ide: migrate the logical CHS translation
  hw/ide: migrate the power-on defaults revert flag
  tests/qtest/ide-test: cover the CHS translation across migration
  tests/qtest/ide-test: cover a rejected CHS translation in the stream
  tests/qtest/ide-test: cover the IDENTIFY DEVICE geometry words
  hw/ide: revert the CHS translation on a hardware reset
  tests/qtest/ide-test: cover the CHS translation across resets
  hw/ide: drop a redundant interrupt from INITIALIZE DEVICE PARAMETERS

 hw/ide/ahci.c             |  12 +-
 hw/ide/cmd646.c           |   2 +-
 hw/ide/core.c             | 141 ++++++++---
 hw/ide/ide-internal.h     |   7 +-
 hw/ide/isa.c              |   2 +-
 hw/ide/macio.c            |   2 +-
 hw/ide/mmio.c             |   2 +-
 hw/ide/piix.c             |   2 +-
 hw/ide/sii3112.c          |   6 +-
 hw/ide/via.c              |   2 +-
 tests/qtest/ahci-test.c   |  40 +++
 tests/qtest/ide-test.c    | 499 ++++++++++++++++++++++++++++++++++++++
 tests/qtest/libqos/ahci.c |  14 ++
 tests/qtest/libqos/ahci.h |   6 +
 14 files changed, 687 insertions(+), 50 deletions(-)


base-commit: 9696bf5dc5a5bf0b4a9d05b6cdfe5f13990f97aa
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.