[ros-diffs] [tfaber] 64950: [NTOS:IO] - Don't delete the device node for root enumerated device objects on failure. It's pointless, since IopEnumerateDevice will just recreate it, and more importantly it cause...

[tfaber]

Author: tfaber
Date: Fri Oct 24 09:39:15 2014
New Revision: 64950

URL: http://svn.reactos.org/svn/reactos?rev=64950&view=rev
Log:
[NTOS:IO]
- Don't delete the device node for root enumerated device objects on failure. 
It's pointless, since IopEnumerateDevice will just recreate it, and more 
importantly it causes a use-after-free because IopFreeDeviceNode does not unset 
the DeviceNode member of the device object extension, so IopEnumerateDevice 
will try to access the freed node
- Set the device object's DeviceNode pointer to NULL in IopFreeDeviceNode
- Use consistent pool tagging for device nodes
CORE-8671 #resolve

Modified:
    trunk/reactos/ntoskrnl/include/internal/tag.h
    trunk/reactos/ntoskrnl/io/iomgr/driver.c
    trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c

Modified: trunk/reactos/ntoskrnl/include/internal/tag.h
URL: 
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/tag.h?rev=64950&r1=64949&r2=64950&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/tag.h       [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/tag.h       [iso-8859-1] Fri Oct 24 
09:39:15 2014
@@ -80,6 +80,9 @@
 
 /* formerly located in io/mdl.c */
 #define TAG_MDL    ' LDM'
+
+/* formerly located in io/pnpmgr.c */
+#define TAG_IO_DEVNODE 'donD'
 
 /* formerly located in io/pnpnotify.c */
 #define TAG_PNP_NOTIFY  'NPnP'

Modified: trunk/reactos/ntoskrnl/io/iomgr/driver.c
URL: 
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/driver.c?rev=64950&r1=64949&r2=64950&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/driver.c    [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/driver.c    [iso-8859-1] Fri Oct 24 
09:39:15 2014
@@ -930,7 +930,6 @@
 
     if (!NT_SUCCESS(Status))
     {
-        IopFreeDeviceNode(DeviceNode);
         return Status;
     }
 
@@ -994,7 +993,6 @@
     if (!NT_SUCCESS(Status))
     {
         /* Fail */
-        IopFreeDeviceNode(DeviceNode);
         return;
     }
 
@@ -1003,7 +1001,6 @@
     if (!NT_SUCCESS(Status))
     {
         /* Fail */
-        IopFreeDeviceNode(DeviceNode);
         ObDereferenceObject(DriverObject);
         return;
     }
@@ -1013,7 +1010,6 @@
     if (!NT_SUCCESS(Status))
     {
         /* Fail */
-        IopFreeDeviceNode(DeviceNode);
         ObDereferenceObject(DriverObject);
         return;
     }
@@ -2020,7 +2016,6 @@
         {
             DPRINT1("IopInitializeDriverModule() failed (Status %lx)\n", 
Status);
             MmUnloadSystemImage(ModuleObject);
-            IopFreeDeviceNode(DeviceNode);
             return Status;
         }
 

Modified: trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c
URL: 
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c?rev=64950&r1=64949&r2=64950&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c   [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c   [iso-8859-1] Fri Oct 24 
09:39:15 2014
@@ -1024,7 +1024,7 @@
    DPRINT("ParentNode 0x%p PhysicalDeviceObject 0x%p ServiceName %wZ\n",
       ParentNode, PhysicalDeviceObject, ServiceName);
 
-   Node = (PDEVICE_NODE)ExAllocatePool(NonPagedPool, sizeof(DEVICE_NODE));
+   Node = ExAllocatePoolWithTag(NonPagedPool, sizeof(DEVICE_NODE), 
TAG_IO_DEVNODE);
    if (!Node)
    {
       return STATUS_INSUFFICIENT_RESOURCES;
@@ -1044,7 +1044,7 @@
       FullServiceName.Buffer = ExAllocatePool(PagedPool, 
FullServiceName.MaximumLength);
       if (!FullServiceName.Buffer)
       {
-          ExFreePool(Node);
+          ExFreePoolWithTag(Node, TAG_IO_DEVNODE);
           return STATUS_INSUFFICIENT_RESOURCES;
       }
 
@@ -1055,7 +1055,7 @@
       if (!NT_SUCCESS(Status))
       {
          DPRINT1("PnpRootCreateDevice() failed with status 0x%08X\n", Status);
-         ExFreePool(Node);
+         ExFreePoolWithTag(Node, TAG_IO_DEVNODE);
          return Status;
       }
 
@@ -1064,7 +1064,7 @@
       if (!NT_SUCCESS(Status))
       {
           ZwClose(InstanceHandle);
-          ExFreePool(Node);
+          ExFreePoolWithTag(Node, TAG_IO_DEVNODE);
           ExFreePool(FullServiceName.Buffer);
           return Status;
       }
@@ -1073,7 +1073,7 @@
       if (!Node->ServiceName.Buffer)
       {
           ZwClose(InstanceHandle);
-          ExFreePool(Node);
+          ExFreePoolWithTag(Node, TAG_IO_DEVNODE);
           ExFreePool(FullServiceName.Buffer);
           return Status;
       }
@@ -1122,7 +1122,7 @@
 
       if (!NT_SUCCESS(Status))
       {
-          ExFreePool(Node);
+          ExFreePoolWithTag(Node, TAG_IO_DEVNODE);
           return Status;
       }
 
@@ -1225,7 +1225,8 @@
       ExFreePool(DeviceNode->BootResources);
    }
 
-   ExFreePool(DeviceNode);
+   
((PEXTENDED_DEVOBJ_EXTENSION)DeviceNode->PhysicalDeviceObject->DeviceObjectExtension)->DeviceNode
 = NULL;
+   ExFreePoolWithTag(DeviceNode, TAG_IO_DEVNODE);
 
    return STATUS_SUCCESS;
 }
@@ -2560,7 +2561,7 @@
 
    DPRINT("IopActionInitChildServices(%p, %p)\n", DeviceNode, Context);
 
-   ParentDeviceNode = (PDEVICE_NODE)Context;
+   ParentDeviceNode = Context;
 
    /*
     * We are called for the parent too, but we don't need to do special
@@ -3545,7 +3546,7 @@
     PAGED_CODE();
 
     /* Allocate it */
-    DeviceNode = ExAllocatePoolWithTag(NonPagedPool, sizeof(DEVICE_NODE), 
'donD');
+    DeviceNode = ExAllocatePoolWithTag(NonPagedPool, sizeof(DEVICE_NODE), 
TAG_IO_DEVNODE);
     if (!DeviceNode) return DeviceNode;
 
     /* Statistics */