[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