READ(6) bug in Initiator mode with oddly-sized HDD

[ZigZagJoe]

ZuluSCSI currently handles oddly sized drives >1GB incorrectly in initiator mode.

Scenario: IBM DCAS32160 - 4226725 sectors in length. As this is not evenly divisible by 512 (g_initiator_state.max_sector_per_transfer), it trys to use READ(6) rather than READ(10) to read the last 165 sectors. However, READ(6) has a maximum sector of 0x1FFFFF (21 bits), but the code is checking start_sector < 0xFFFFFF (24 bits). This results in failure to read each of these remaining 165 sectors and an incomplete image.

Verified issue is corrected when checking for start_sector < 0x1FFFFF for READ(6) path.

Please see below for appropriate patch.

From 43b9e6e566e5d192940d71e75343f3196d669be7 Mon Sep 17 00:00:00 2001
From: zigzagjoe <[email protected]>
Date: Thu, 2 Nov 2023 21:57:00 -0500
Subject: [PATCH] Set correct limit for read(6) in initiator mode

---
 src/ZuluSCSI_initiator.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/ZuluSCSI_initiator.cpp b/src/ZuluSCSI_initiator.cpp
index 0ca296d..de004b6 100644
--- a/src/ZuluSCSI_initiator.cpp
+++ b/src/ZuluSCSI_initiator.cpp
@@ -767,7 +767,9 @@ bool scsiInitiatorReadDataToFile(int target_id, uint32_t start_sector, uint32_t
 {
     int status = -1;
 
-    if (start_sector < 0xFFFFFF && sectorcount <= 256)
+    // Read6 command supports 21 bit LBA - max of 0x1FFFFF
+    // ref: https://www.seagate.com/files/staticfiles/support/docs/manual/Interface%20manuals/100293068j.pdf pg 134
+    if (start_sector < 0x1FFFFF && sectorcount <= 256)
     {
         // Use READ6 command for compatibility with old SCSI1 drives
         uint8_t command[6] = {0x08,
-- 
2.42.0.windows.2

[aperezbios]

@ZigZagJoe thanks for the contribution. We should be able to merge this shortly.

[ZigZagJoe]

Happy to help. As an aside: the Seagate SCSI reference manual I cite there states a 2GB max for READ(6), but with 21 bits for the LBA [max of 0x1FFFFF] * LBA size of 512 bytes that gives an actual max address of 1GB. So that's where that value I actually used came from.

[PetteriAimonen]

Yeah, this patch looks correct. https://www.staff.uni-mainz.de/tacke/scsi/SCSI2-09.html#9.2.5 agrees on the 21 bit limit for block address.

[aperezbios]

@ZigZagJoe this has been merged in to the repository, and will be incorporated in the next release. Until then, CI-built nightly/on-demand binaries can be had at https://github.com/ZuluSCSI/ZuluSCSI-firmware/releases/tag/latest