[viostor] Fix for clobbered SRB Extension IDs

Refactors SRB Extension ID assignment:

1. Moves assignment from VirtIoStartIo() in virtio_stor.c to virtio_stor_hw_helper.c:
  - RhelDoFlush()
  - RhelDoReadWrite()
  - RhelDoUnMap()
  - RhelGetSerialNumber()
2. Moves ULONG_PTR from _ADAPTER_EXTENSION stuct to _REQUEST_LIST struct
3. Renames ULONG_PTR from last_srb_id to next_id
4. Refactors element assignment to be under spinlock
5. Moves ULONG_PTR id towards top of _SRB_EXTENSION struct to resolve memory alignment issue.

The refactor solves the problem because the assignment now occurs under spinlock.

Resolves #1466

Reported by: @iops-hunter

Signed-off-by: benyamin-codez <115509179+benyamin-codez@users.noreply.github.com>

Author: benyamin-codez

viostor/virtio_stor.c

@@ -259,7 +259,6 @@ VirtIoFindAdapter(IN PVOID DeviceExtension,
 
     adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
 
-    adaptExt->last_srb_id = 1;
     adaptExt->system_io_bus_number = ConfigInfo->SystemIoBusNumber;
     adaptExt->slot_number = ConfigInfo->SlotNumber;
     adaptExt->dump_mode = IsCrashDumpMode;
@@ -895,17 +894,9 @@ VirtIoStartIo(IN PVOID DeviceExtension, IN PSCSI_REQUEST_BLOCK Srb)
 {
     PCDB cdb = SRB_CDB(Srb);
     PADAPTER_EXTENSION adaptExt;
-    PSRB_EXTENSION srbExt;
     UCHAR ScsiStatus = SCSISTAT_GOOD;
 
     adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
-    srbExt = SRB_EXTENSION(Srb);
-    srbExt->id = adaptExt->last_srb_id;
-    adaptExt->last_srb_id++;
-    if (adaptExt->last_srb_id == 0)
-    {
-        adaptExt->last_srb_id++;
-    }
 
     SRB_SET_SCSI_STATUS(((PSRB_TYPE)Srb), ScsiStatus);
 
@@ -2136,6 +2127,7 @@ VOID VioStorCompleteRequest(IN PVOID DeviceExtension, IN ULONG MessageID, IN BOO
 
                 Srb = (PSRB_TYPE)req->req;
                 srbExt = SRB_EXTENSION(Srb);
+
                 // Only SRBs with existing (i.e. non-NULL) extension
                 // are inserted into our queues, thus, we may help
                 // the Code Analysis and provide it with this information
@@ -2144,8 +2136,8 @@ VOID VioStorCompleteRequest(IN PVOID DeviceExtension, IN ULONG MessageID, IN BOO
                 if (srbExt->id == srbId)
                 {
                     RemoveEntryList(le);
-                    element->srb_cnt--;
                     bFound = TRUE;
+                    element->srb_cnt--;
                     break;
                 }
             }

viostor/virtio_stor.h

@@ -204,6 +204,7 @@ typedef struct _REQUEST_LIST
 {
     LIST_ENTRY srb_list;
     ULONG srb_cnt;
+    ULONG_PTR next_id;
 } REQUEST_LIST, *PREQUEST_LIST;
 
 typedef struct _ADAPTER_EXTENSION
@@ -256,7 +257,6 @@ typedef struct _ADAPTER_EXTENSION
     REQUEST_LIST processing_srbs[MAX_CPU];
     BOOLEAN reset_in_progress;
     ULONGLONG fw_ver;
-    ULONG_PTR last_srb_id;
 #ifdef DBG
     LONG srb_cnt;
     LONG inqueue_cnt;
@@ -274,11 +274,11 @@ typedef struct _VRING_DESC_ALIAS
 typedef struct _SRB_EXTENSION
 {
     blk_req vbr;
+    ULONG_PTR id;
     ULONG out;
     ULONG in;
     ULONG MessageID;
     BOOLEAN fua;
-    ULONG_PTR id;
     VIO_SG sg[VIRTIO_MAX_SG];
     VRING_DESC_ALIAS desc[VIRTIO_MAX_SG];
 } SRB_EXTENSION, *PSRB_EXTENSION;

viostor/virtio_stor_hw_helper.c

@@ -127,11 +127,27 @@ RhelDoFlush(PVOID DeviceExtension, PSRB_TYPE Srb, BOOLEAN resend, BOOLEAN bIsr)
     srbExt->sg[1].physAddr = StorPortGetPhysicalAddress(DeviceExtension, NULL, &srbExt->vbr.status, &fragLen);
     srbExt->sg[1].length = sizeof(srbExt->vbr.status);
 
-    element = &adaptExt->processing_srbs[QueueNumber];
     if (!resend)
     {
         VioStorVQLock(DeviceExtension, MessageId, &LockHandle, FALSE);
+
+        element = &adaptExt->processing_srbs[QueueNumber];
+
+        ULONG_PTR id = element->next_id;
+
+        if (id == 0)
+        {
+            id++;
+        }
+
+        srbExt->id = id;
+        element->next_id = ++id;
+    }
+    else
+    {
+        element = &adaptExt->processing_srbs[QueueNumber];
     }
+
     if (virtqueue_add_buf(vq, &srbExt->sg[0], srbExt->out, srbExt->in, (void *)srbExt->id, va, pa) ==
         VQ_ADD_BUFFER_SUCCESS)
     {
@@ -216,8 +232,20 @@ RhelDoReadWrite(PVOID DeviceExtension, PSRB_TYPE Srb)
     vq = adaptExt->vq[QueueNumber];
     RhelDbgPrint(TRACE_LEVEL_VERBOSE, " QueueNumber 0x%x vq = %p\n", QueueNumber, vq);
 
-    element = &adaptExt->processing_srbs[QueueNumber];
     VioStorVQLock(DeviceExtension, MessageId, &LockHandle, FALSE);
+
+    element = &adaptExt->processing_srbs[QueueNumber];
+
+    ULONG_PTR id = element->next_id;
+
+    if (id == 0)
+    {
+        id++;
+    }
+
+    srbExt->id = id;
+    element->next_id = ++id;
+
     if (virtqueue_add_buf(vq, &srbExt->sg[0], srbExt->out, srbExt->in, (void *)srbExt->id, va, pa) ==
         VQ_ADD_BUFFER_SUCCESS)
     {
@@ -369,8 +397,20 @@ RhelDoUnMap(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
                  vq,
                  srbExt->vbr.out_hdr.type);
 
-    element = &adaptExt->processing_srbs[QueueNumber];
     VioStorVQLock(DeviceExtension, MessageId, &LockHandle, FALSE);
+
+    element = &adaptExt->processing_srbs[QueueNumber];
+
+    ULONG_PTR id = element->next_id;
+
+    if (id == 0)
+    {
+        id++;
+    }
+
+    srbExt->id = id;
+    element->next_id = ++id;
+
     if (virtqueue_add_buf(vq, &srbExt->sg[0], srbExt->out, srbExt->in, (void *)srbExt->id, va, pa) ==
         VQ_ADD_BUFFER_SUCCESS)
     {
@@ -463,8 +503,20 @@ RhelGetSerialNumber(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
     srbExt->sg[2].physAddr = StorPortGetPhysicalAddress(DeviceExtension, NULL, &srbExt->vbr.status, &fragLen);
     srbExt->sg[2].length = sizeof(srbExt->vbr.status);
 
-    element = &adaptExt->processing_srbs[QueueNumber];
     VioStorVQLock(DeviceExtension, MessageId, &LockHandle, FALSE);
+
+    element = &adaptExt->processing_srbs[QueueNumber];
+
+    ULONG_PTR id = element->next_id;
+
+    if (id == 0)
+    {
+        id++;
+    }
+
+    srbExt->id = id;
+    element->next_id = ++id;
+
     if (virtqueue_add_buf(vq, &srbExt->sg[0], srbExt->out, srbExt->in, (void *)srbExt->id, va, pa) ==
         VQ_ADD_BUFFER_SUCCESS)
     {