Parcourir la source

Toolbox fix with Adaptec cards and a bounds check

From Niel Martin Hansen https://github.com/BlueSCSI/BlueSCSI-v2/pull/223
> This should resolve two issues:
>
>     * There is no bounds check in `onSendFile10`, so a faulty host could try to indicate/send buffers larger than 512 bytes and cause an overrun. Also use the buffer size as a constant, and use the `bytes_sent` value from the command for both the bus read and the file write.
>
>     * The way the filename is read in `onSendFilePrep` appears to cause bus timing issues with at least my Adaptec HA, leading to the first `scsiReadByte()` call to trigger watchdog timeout error, losing the first byte of the filename (but reading and handling the rest), and the HA (or OS driver?) never registers the command as completed properly.

Co-authored-by: Niels Martin Hansen <nielsm@indvikleren.dk>
Morio il y a 9 mois
Parent
commit
9f5a2a5b7e
1 fichiers modifiés avec 16 ajouts et 11 suppressions
  1. 16 11
      src/Toolbox.cpp

+ 16 - 11
src/Toolbox.cpp

@@ -1,6 +1,8 @@
 /** 
  * Copyright (C) 2023 Eric Helgeson
  * Copyright (C) 2024 Rabbit Hole Computing
+ * Copyright (C) 2025 Niels Martin Hansen
+ * 
  * This file is originally part of BlueSCSI adopted for ZuluSCSI
  * 
  * This program is free software: you can redistribute it and/or modify
@@ -272,12 +274,12 @@ void onGetFile10(char * dir_name) {
 static void onSendFilePrep(char * dir_name)
 {
     char file_name[32+1];
-    memset(file_name, '\0', 32+1);
+
     scsiEnterPhase(DATA_OUT);
-    for (int i = 0; i < 32+1; ++i)
-    {
-        file_name[i] = scsiReadByte();
-    }
+    scsiRead(static_cast<uint8_t *>(static_cast<void *>(file_name)), 32+1, NULL);
+    file_name[32] = '\0';
+
+    dbgmsg("TOOLBOX OPEN FILE FOR WRITE: '", file_name, "'");
     SD.chdir(dir_name);
     gFile.open(file_name, FILE_WRITE);
     SD.chdir("/");
@@ -317,19 +319,22 @@ static void onSendFile10(void)
     uint16_t bytes_sent = ((uint16_t)scsiDev.cdb[1] << 8)  | scsiDev.cdb[2];
     // 512 byte offset of where to put these bytes.
     uint32_t offset     = ((uint32_t)scsiDev.cdb[3] << 16) | ((uint32_t)scsiDev.cdb[4] << 8) | scsiDev.cdb[5];
-    uint16_t buf_size   = 512;
-    uint8_t buf[512];
+    const uint16_t BUFSIZE   = 512;
+    uint8_t buf[BUFSIZE];
 
-    // Check if last block of file, and not the only bock in file.
-    if(bytes_sent < buf_size)
+    // Do not allow buffer overrun
+    if (bytes_sent > BUFSIZE)
     {
-        buf_size = bytes_sent;
+        dbgmsg("TOOLBOX SEND FILE 10 ILLEGAL DATA SIZE");
+        gFile.close();
+        scsiDev.status = CHECK_CONDITION;
+        scsiDev.target->sense.code = ILLEGAL_REQUEST;
     }
 
     scsiEnterPhase(DATA_OUT);
     scsiRead(buf, bytes_sent, NULL);
     gFile.seekCur(offset * 512);
-    gFile.write(buf, buf_size);
+    gFile.write(buf, bytes_sent);
     if(gFile.getWriteError())
     {
         gFile.clearWriteError();