On 8/20/26 12:08, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain
> it was sent from.
> Do not click links or open attachments unless it is an email you expected to
> receive.
> 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 10 and 11 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.
>
> v1:
> https://lore.kernel.org/qemu-devel/[email protected]/
>
> Changes in v2
> -------------
>
> - patch 1, patch 5: Cc qemu-stable. (Philippe)
> - patch 6: new, the "XXX: retired, remove ?" on IDENTIFY DEVICE words 4,
> 5 and 20 is answered rather than carried. ATA-5 3.2.3.6 keeps the
> ATA-1 meaning of a retired word that a device still fills in, so the
> comments name that meaning; word 21 is a buffer size, not a cache
> size. (Philippe)
> - patch 7: new, splits the extraction of ide_identify_chs() out of what
> is now patch 8, leaving that one with the two call sites that keep the
> words in sync. (Philippe)
>
> Signed-off-by: Denis V. Lunev <[email protected]>
> Cc: John Snow <[email protected]>
> Cc: Peter Maydell <[email protected]>
> Cc: Philippe Mathieu-Daudé <[email protected]>
>
> Denis V. Lunev (17):
> 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: name the retired IDENTIFY DEVICE words the device fills in
> hw/ide: factor out the IDENTIFY DEVICE current geometry words
> 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 | 147 ++++++++---
> 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, 690 insertions(+), 53 deletions(-)
>
>
> base-commit: ae4f3443209ab154b48b706a146e5f557ab147cb
queued
