[ros-diffs] [reactos] 05/05: [FREELDR] i386/pc/pcdisk.c: Fix LBA reads retry loop (#7367)

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=840d39b9d09a1bce487c6…

commit 840d39b9d09a1bce487c651e880049ee577cf8be
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Sun Sep 22 21:37:36 2024 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Wed Sep 25 13:24:08 2024 +0200

    [FREELDR] i386/pc/pcdisk.c: Fix LBA reads retry loop (#7367)
    
    When the Int 13h AH=42h "Extended read" function fails, the disk address
    packet's LBA block count is reset to the number of blocks that have been
    successfully transferred. This is more or less fine, unless one wants to
    ensure the exact number of sectors gets read.
    
    If the function fails so that zero sectors were read, the retry loop is
    restarted, but with the packet's LBA block count member reset, as per
    the documentation. (In this example, it is reset to zero.) Then, at the
    next retry attempt, zero sectors are requested to be read, and this time
    of course, the call succeeds... Wrongly, of course, this is not what's
    expected.
    
    Therefore, for each retry, the LBA block count member should be set
    again to the correct number of sectors to read. There are maximum 3
    retries, so the retry loop will stop anyway, but the LBA read will now
    correctly fail and return FALSE, as expected.
    
    This problem doesn't exist in the retry loop for the Int 13h, AH=02h
    "Read Disk Sectors" CHS function, because here, the call is made only
    using registers, and we use a pair of RegsIn/RegsOut. RegsOut receives
    the modified register values, but the input RegsIn stays unchanged.
---
 boot/freeldr/freeldr/arch/i386/pc/pcdisk.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/boot/freeldr/freeldr/arch/i386/pc/pcdisk.c
b/boot/freeldr/freeldr/arch/i386/pc/pcdisk.c
index 40492457c86..28e7fd66a76 100644
--- a/boot/freeldr/freeldr/arch/i386/pc/pcdisk.c
+++ b/boot/freeldr/freeldr/arch/i386/pc/pcdisk.c
@@ -570,14 +570,13 @@ PcDiskReadLogicalSectorsLBA(
     RtlZeroMemory(Packet, sizeof(*Packet));
     Packet->PacketSize = sizeof(*Packet);
     Packet->Reserved = 0;
-    Packet->LBABlockCount = (USHORT)SectorCount;
-    ASSERT(Packet->LBABlockCount == SectorCount);
+    // Packet->LBABlockCount set in the loop.
     Packet->TransferBufferOffset = ((ULONG_PTR)Buffer) & 0x0F;
     Packet->TransferBufferSegment = (USHORT)(((ULONG_PTR)Buffer) >> 4);
     Packet->LBAStartBlock = SectorNumber;
 
     /*
-     * BIOS int 0x13, function 42h - IBM/MS INT 13 Extensions - EXTENDED READ
+     * BIOS Int 13h, function 42h - IBM/MS INT 13 Extensions - EXTENDED READ
      * Return:
      * CF clear if successful
      * AH = 00h
@@ -586,7 +585,7 @@ PcDiskReadLogicalSectorsLBA(
      * Disk address packet's block count field set to the
      * number of blocks successfully transferred.
      */
-    RegsIn.b.ah = 0x42;                 // Subfunction 42h
+    RegsIn.b.ah = 0x42;
     RegsIn.b.dl = DriveNumber;          // Drive number in DL (0 - floppy, 0x80 -
harddisk)
     RegsIn.x.ds = BIOSCALLBUFSEGMENT;   // DS:SI -> disk address packet
     RegsIn.w.si = BIOSCALLBUFOFFSET;
@@ -594,6 +593,12 @@ PcDiskReadLogicalSectorsLBA(
     /* Retry 3 times */
     for (RetryCount = 0; RetryCount < 3; ++RetryCount)
     {
+        /* Restore the number of blocks to transfer, since it gets reset
+         * on failure with the number of blocks that were successfully
+         * transferred (and which could be zero). */
+        Packet->LBABlockCount = (USHORT)SectorCount;
+        ASSERT(Packet->LBABlockCount == SectorCount);
+
         Int386(0x13, &RegsIn, &RegsOut);
 
         /* If it worked return TRUE */