[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...

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/t...
==============================================================================
--- 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?...
==============================================================================
--- 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...
==============================================================================
--- 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 */