[ros-diffs] [reactos] 05/17: [USBSTOR] Better validate SCSI IRPs. Patch by Vadim Galyant

[Victor Perevertkin]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=3faf5efd49586b46b2c19…

commit 3faf5efd49586b46b2c19e7b99e7771b59580125
Author:     Victor Perevertkin <victor(a)perevertkin.ru>
AuthorDate: Sun Mar 31 17:20:27 2019 +0300
Commit:     Victor Perevertkin <victor(a)perevertkin.ru>
CommitDate: Tue Jun 11 04:39:43 2019 +0300

    [USBSTOR] Better validate SCSI IRPs.
    Patch by Vadim Galyant
---
 drivers/usb/usbstor/disk.c    | 101 +++++++++++++++++++++++++++++++++---------
 drivers/usb/usbstor/usbstor.h |   1 +
 2 files changed, 81 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/usbstor/disk.c b/drivers/usb/usbstor/disk.c
index 984296214cf..b73ca91292c 100644
--- a/drivers/usb/usbstor/disk.c
+++ b/drivers/usb/usbstor/disk.c
@@ -5,6 +5,7 @@
  * COPYRIGHT:   2005-2006 James Tabor
  *              2011-2012 Michael Martin (michael.martin(a)reactos.org)
  *              2011-2013 Johannes Anderwald (johannes.anderwald(a)reactos.org)
+ *              2017 Vadim Galyant
  */
 
 #include "usbstor.h"
@@ -13,6 +14,69 @@
 #include <debug.h>
 
 
+static
+BOOLEAN
+IsRequestValid(PIRP Irp)
+{
+    ULONG TransferLength;
+    PIO_STACK_LOCATION IoStack;
+    PSCSI_REQUEST_BLOCK Srb;
+
+    IoStack = IoGetCurrentIrpStackLocation(Irp);
+    Srb = IoStack->Parameters.Scsi.Srb;
+
+    if (Srb->SrbFlags & (SRB_FLAGS_DATA_IN | SRB_FLAGS_DATA_OUT))
+    {
+        if ((Srb->SrbFlags & SRB_FLAGS_UNSPECIFIED_DIRECTION) ==
SRB_FLAGS_UNSPECIFIED_DIRECTION)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. Srb->SrbFlags - %X\n",
Srb->SrbFlags);
+            return FALSE;
+        }
+
+        TransferLength = Srb->DataTransferLength;
+
+        if (Irp->MdlAddress == NULL)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. Irp->MdlAddress ==
NULL\n");
+            return FALSE;
+        }
+
+        if (TransferLength == 0)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. TransferLength == 0\n");
+            return FALSE;
+        }
+
+        if (TransferLength > USBSTOR_DEFAULT_MAX_TRANSFER_LENGTH)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. TransferLength >
0x10000\n");
+            return FALSE;
+        }
+    }
+    else
+    {
+        if (Srb->DataTransferLength)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. Srb->DataTransferLength !=
0\n");
+            return FALSE;
+        }
+
+        if (Srb->DataBuffer)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. Srb->DataBuffer !=
NULL\n");
+            return FALSE;
+        }
+
+        if (Irp->MdlAddress)
+        {
+            DPRINT1("IsRequestValid: Invalid Srb. Irp->MdlAddress !=
NULL\n");
+            return FALSE;
+        }
+    }
+
+    return TRUE;
+}
+
 NTSTATUS
 USBSTOR_HandleInternalDeviceControl(
     IN PDEVICE_OBJECT DeviceObject,
@@ -35,28 +99,23 @@ USBSTOR_HandleInternalDeviceControl(
         {
             DPRINT("SRB_FUNCTION_EXECUTE_SCSI\n");
 
-            // check if request is valid
-            if (Request->SrbFlags & (SRB_FLAGS_DATA_IN | SRB_FLAGS_DATA_OUT))
+            if (!IsRequestValid(Irp))
             {
-                // data is transferred with this irp
-                if ((Request->SrbFlags & (SRB_FLAGS_DATA_IN | SRB_FLAGS_DATA_OUT))
== (SRB_FLAGS_DATA_IN | SRB_FLAGS_DATA_OUT) ||
-                    Request->DataTransferLength == 0 ||
-                    Irp->MdlAddress == NULL)
-                {
-                    Status = STATUS_INVALID_PARAMETER;
-                    break;
-                }
+                Status = STATUS_INVALID_PARAMETER;
+                break;
             }
-            else
+
+            if (Request->Cdb[0] == SCSIOP_MODE_SENSE)
             {
-                // sense buffer request
-                if (Request->DataTransferLength || Request->DataBuffer ||
Irp->MdlAddress)
-                {
-                    Status = STATUS_INVALID_PARAMETER;
-                    break;
-                }
+                DPRINT("USBSTOR_Scsi: SRB_FUNCTION_EXECUTE_SCSI - FIXME
SCSIOP_MODE_SENSE\n");
+                // FIXME Get from registry WriteProtect for StorageDevicePolicies;
+                //
L"\\Registry\\Machine\\System\\CurrentControlSet\\Control\\StorageDevicePolicies"
+                // QueryTable[0].Name = L"WriteProtect"
             }
 
+            IoMarkIrpPending(Irp);
+            Request->SrbStatus = SRB_STATUS_PENDING;
+
             // add the request
             if (!USBSTOR_QueueAddIrp(PDODeviceExtension->LowerDeviceObject, Irp))
             {
@@ -337,8 +396,8 @@ USBSTOR_HandleQueryProperty(
         // fill out descriptor
         AdapterDescriptor->Version = sizeof(STORAGE_ADAPTER_DESCRIPTOR);
         AdapterDescriptor->Size = sizeof(STORAGE_ADAPTER_DESCRIPTOR);
-        AdapterDescriptor->MaximumTransferLength = MAXULONG; //FIXME compute some sane
value
-        AdapterDescriptor->MaximumPhysicalPages = 25; //FIXME compute some sane value
+        AdapterDescriptor->MaximumTransferLength =
USBSTOR_DEFAULT_MAX_TRANSFER_LENGTH;
+        AdapterDescriptor->MaximumPhysicalPages = USBSTOR_DEFAULT_MAX_TRANSFER_LENGTH
/ PAGE_SIZE + 1; // See CORE-10515 and CORE-10755
         AdapterDescriptor->AlignmentMask = 0;
         AdapterDescriptor->AdapterUsesPio = FALSE;
         AdapterDescriptor->AdapterScansDown = FALSE;
@@ -406,8 +465,8 @@ USBSTOR_HandleDeviceControl(
 
             if (Capabilities)
             {
-                Capabilities->MaximumTransferLength = MAXULONG;
-                Capabilities->MaximumPhysicalPages = 25;
+                Capabilities->MaximumTransferLength =
USBSTOR_DEFAULT_MAX_TRANSFER_LENGTH;
+                Capabilities->MaximumPhysicalPages =
USBSTOR_DEFAULT_MAX_TRANSFER_LENGTH / PAGE_SIZE + 1; // See CORE-10515 and CORE-10755
                 Capabilities->SupportedAsynchronousEvents = 0;
                 Capabilities->AlignmentMask = 0;
                 Capabilities->TaggedQueuing = FALSE;
diff --git a/drivers/usb/usbstor/usbstor.h b/drivers/usb/usbstor/usbstor.h
index 1ac3e33a55e..6d23a567853 100644
--- a/drivers/usb/usbstor/usbstor.h
+++ b/drivers/usb/usbstor/usbstor.h
@@ -9,6 +9,7 @@
 
 #define USB_STOR_TAG 'sbsu'
 #define USB_MAXCHILDREN              (16)
+#define USBSTOR_DEFAULT_MAX_TRANSFER_LENGTH 0x10000
 
 #define HTONS(n) (((((unsigned short)(n) & 0xFF)) << 8) | (((unsigned short)(n)
& 0xFF00) >> 8))
 #define NTOHS(n) (((((unsigned short)(n) & 0xFF)) << 8) | (((unsigned short)(n)
& 0xFF00) >> 8))