USB MSC initiator: fix re-entrancy issues

scsiInitiatorRunCommand() calls platform_poll(), which in turn
could end up calling tusb_task(). This could cause re-entrant
handling of a USB MSC command while the previous one wasn't complete.

Added a mechanism to delay new USB events until the previous one
has been fully handled.

lib/ZuluSCSI_platform_RP2MCU/ZuluSCSI_platform.cpp

@@ -51,6 +51,10 @@ extern "C" {
 }
 #endif // ZULUSCSI_NETWORK
 
+#ifdef PLATFORM_MASS_STORAGE
+#include "ZuluSCSI_platform_msc.h"
+#endif
+
 #ifdef ENABLE_AUDIO_OUTPUT
 #  include "audio.h"
 #endif // ENABLE_AUDIO_OUTPUT
@@ -646,6 +650,11 @@ static void usb_log_poll()
 {
 #ifndef PIO_FRAMEWORK_ARDUINO_NO_USB
     static uint32_t logpos = 0;
+
+#ifdef PLATFORM_MASS_STORAGE
+    if (platform_msc_lock_get()) return; // Avoid re-entrant USB events
+#endif
+
     if (Serial.availableForWrite())
     {
         // Retrieve pointer to log start and determine number of bytes available.
@@ -670,6 +679,11 @@ static void usb_log_poll()
 static void usb_input_poll()
 {
     #ifndef PIO_FRAMEWORK_ARDUINO_NO_USB
+
+#ifdef PLATFORM_MASS_STORAGE
+    if (platform_msc_lock_get()) return; // Avoid re-entrant USB events
+#endif
+
     // Caputure reboot key sequence
     static bool mass_storage_reboot_keyed = false;
     static bool basic_reboot_keyed = false;

lib/ZuluSCSI_platform_RP2MCU/ZuluSCSI_platform_msc.cpp

@@ -34,6 +34,9 @@
 #include <class/msc/msc.h>
 #include <class/msc/msc_device.h>
 
+#include <pico/mutex.h>
+extern mutex_t __usb_mutex;
+
 #if CFG_TUD_MSC_EP_BUFSIZE < SD_SECTOR_SIZE
   #error "CFG_TUD_MSC_EP_BUFSIZE is too small! It needs to be at least 512 (SD_SECTOR_SIZE)"
 #endif
@@ -44,6 +47,51 @@
 extern SdFs SD;
 static bool unitReady = false;
 
+static bool g_msc_lock; // To block re-entrant calls
+static bool g_msc_usb_mutex_held;
+
+void platform_msc_lock_set(bool block)
+{
+  if (block)
+  {
+    if (g_msc_lock)
+    {
+      logmsg("Re-entrant MSC lock!");
+      assert(false);
+    }
+
+    g_msc_usb_mutex_held = mutex_try_enter(&__usb_mutex, NULL); // Blocks USB IRQ if not already blocked
+    g_msc_lock = true; // Blocks platform USB polling
+  }
+  else
+  {
+    if (!g_msc_lock)
+    {
+      logmsg("MSC lock released when not held!");
+      assert(false);
+    }
+
+    g_msc_lock = false;
+
+    if (g_msc_usb_mutex_held)
+    {
+      g_msc_usb_mutex_held = false;
+      mutex_exit(&__usb_mutex);
+    }
+  }
+}
+
+bool platform_msc_lock_get()
+{
+  return g_msc_lock;
+}
+
+struct MSCScopedLock {
+public:
+  MSCScopedLock() {  platform_msc_lock_set(true); }
+  ~MSCScopedLock() { platform_msc_lock_set(false); }
+};
+
 /* return true if USB presence detected / eligble to enter CR mode */
 bool platform_sense_msc() {
 
@@ -111,6 +159,7 @@ void __USBInstallMassStorage() { }
 extern "C" void tud_msc_inquiry_cb(uint8_t lun, uint8_t vendor_id[8],
                         uint8_t product_id[16], uint8_t product_rev[4]) {
 
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_inquiry_cb(lun, vendor_id, product_id, product_rev);
 
   const char vid[] = "ZuluSCSI";
@@ -124,7 +173,9 @@ extern "C" void tud_msc_inquiry_cb(uint8_t lun, uint8_t vendor_id[8],
 
 // max LUN supported
 // we only have the one SD card
-extern "C" uint8_t tud_msc_get_maxlun_cb(void) {
+extern "C" uint8_t tud_msc_get_maxlun_cb(void)
+{
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_get_maxlun_cb();
 
   return 1; // number of LUNs supported
@@ -135,6 +186,7 @@ extern "C" uint8_t tud_msc_get_maxlun_cb(void) {
 // otherwise this is not actually needed
 extern "C" bool tud_msc_is_writable_cb (uint8_t lun)
 {
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_is_writable_cb(lun);
 
   (void) lun;
@@ -144,6 +196,7 @@ extern "C" bool tud_msc_is_writable_cb (uint8_t lun)
 // see https://www.seagate.com/files/staticfiles/support/docs/manual/Interface%20manuals/100293068j.pdf pg 221
 extern "C" bool tud_msc_start_stop_cb(uint8_t lun, uint8_t power_condition, bool start, bool load_eject)
 {
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_start_stop_cb(lun, power_condition, start, load_eject);
 
   if (load_eject)  {
@@ -159,7 +212,9 @@ extern "C" bool tud_msc_start_stop_cb(uint8_t lun, uint8_t power_condition, bool
 }
 
 // return true if we are ready to service reads/writes
-extern "C" bool tud_msc_test_unit_ready_cb(uint8_t lun) {
+extern "C" bool tud_msc_test_unit_ready_cb(uint8_t lun)
+{
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_test_unit_ready_cb(lun);
 
   return unitReady;
@@ -167,7 +222,9 @@ extern "C" bool tud_msc_test_unit_ready_cb(uint8_t lun) {
 
 // return size in blocks and block size
 extern "C" void tud_msc_capacity_cb(uint8_t lun, uint32_t *block_count,
-                         uint16_t *block_size) {
+                         uint16_t *block_size)
+{
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_capacity_cb(lun, block_count, block_size);
 
   *block_count = unitReady ? (SD.card()->sectorCount()) : 0;
@@ -177,7 +234,9 @@ extern "C" void tud_msc_capacity_cb(uint8_t lun, uint32_t *block_count,
 // Callback invoked when received an SCSI command not in built-in list (below) which have their own callbacks
 // - READ_CAPACITY10, READ_FORMAT_CAPACITY, INQUIRY, MODE_SENSE6, REQUEST_SENSE, READ10 and WRITE10
 extern "C" int32_t tud_msc_scsi_cb(uint8_t lun, const uint8_t scsi_cmd[16], void *buffer,
-                        uint16_t bufsize) {
+                        uint16_t bufsize)
+{
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_scsi_cb(lun, scsi_cmd, buffer, bufsize);
 
   const void *response = NULL;
@@ -216,6 +275,7 @@ extern "C" int32_t tud_msc_scsi_cb(uint8_t lun, const uint8_t scsi_cmd[16], void
 extern "C" int32_t tud_msc_read10_cb(uint8_t lun, uint32_t lba, uint32_t offset, 
                             void* buffer, uint32_t bufsize)
 {
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_read10_cb(lun, lba, offset, buffer, bufsize);
 
   bool rc = SD.card()->readSectors(lba, (uint8_t*) buffer, bufsize/SD_SECTOR_SIZE);
@@ -230,7 +290,9 @@ extern "C" int32_t tud_msc_read10_cb(uint8_t lun, uint32_t lba, uint32_t offset,
 // Callback invoked when receive WRITE10 command.
 // Process data in buffer to disk's storage and return number of written bytes (must be multiple of block size)
 extern "C" int32_t tud_msc_write10_cb(uint8_t lun, uint32_t lba, uint32_t offset,
-                           uint8_t *buffer, uint32_t bufsize) {
+                           uint8_t *buffer, uint32_t bufsize)
+{
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_read10_cb(lun, lba, offset, buffer, bufsize);
 
   bool rc = SD.card()->writeSectors(lba, buffer, bufsize/SD_SECTOR_SIZE);
@@ -243,7 +305,9 @@ extern "C" int32_t tud_msc_write10_cb(uint8_t lun, uint32_t lba, uint32_t offset
 
 // Callback invoked when WRITE10 command is completed (status received and accepted by host).
 // used to flush any pending cache to storage
-extern "C" void tud_msc_write10_complete_cb(uint8_t lun) {
+extern "C" void tud_msc_write10_complete_cb(uint8_t lun)
+{
+  MSCScopedLock lock;
   if (g_msc_initiator) return init_msc_write10_complete_cb(lun);
 }
 

lib/ZuluSCSI_platform_RP2MCU/ZuluSCSI_platform_msc.h

@@ -37,4 +37,10 @@ bool platform_run_msc();
 /* perform any cleanup tasks for the MSC-specific functionality */
 void platform_exit_msc();
 
-#endif
+/* Block re-entrant msc poll calls.
+   This avoids starting another command handler while first one is running.
+   */
+void platform_msc_lock_set(bool block);
+bool platform_msc_lock_get();
+
+#endif