[PATCH 1/2] ntoskrnl.exe: Track drivers created with IoCreateDriver

[Sebastian Lackner]

On 26.02.2016 21:22, Aric Stewart wrote:
> Signed-off-by: Aric Stewart <aric at codeweavers.com>
> ---
>  dlls/ntoskrnl.exe/ntoskrnl.c | 64 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 48 insertions(+), 16 deletions(-)
> 
> 
> 
> 0002-ntoskrnl.exe-Track-drivers-created-with-IoCreateDriver.txt
> 
> 
> diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
> index 3bee2bf..49c8cab 100644
> --- a/dlls/ntoskrnl.exe/ntoskrnl.c
> +++ b/dlls/ntoskrnl.exe/ntoskrnl.c
> @@ -72,6 +72,16 @@ static DWORD request_thread;
>  static DWORD client_tid;
>  static DWORD client_pid;
>  
> +typedef struct _created_driver {
> +    struct list entry;
> +
> +    UNICODE_STRING driver_name;
> +    DRIVER_OBJECT *driver_obj;
> +    DRIVER_EXTENSION *driver_extension;

Isn't it possible to add the DRIVER_OBJECT and DRIVER_EXTENSION struct directly
(without pointer), to avoid multiple memory allocations?

I would also suggest to use a more meaningful name than "created_driver", and
you also don't really need the typedef. Something like "struct wine_driver"
would make more sense for example, but probably you have a better idea. ;)

> +} created_driver;
> +
> +struct list created_drivers = LIST_INIT(created_drivers);
> +
>  #ifdef __i386__
>  #define DEFINE_FASTCALL1_ENTRYPOINT( name ) \
>      __ASM_STDCALL_FUNC( name, 4, \
> @@ -818,36 +828,47 @@ PIRP WINAPI IoBuildSynchronousFsdRequest(ULONG majorfunc, PDEVICE_OBJECT device,
>   */
>  NTSTATUS WINAPI IoCreateDriver( UNICODE_STRING *name, PDRIVER_INITIALIZE init )
>  {
> -    DRIVER_OBJECT *driver;
> -    DRIVER_EXTENSION *extension;
> +    created_driver *driver;
>      NTSTATUS status;
>  
>      TRACE("(%s, %p)\n", debugstr_us(name), init);
>  
>      if (!(driver = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY,
> -                                    sizeof(*driver) + sizeof(*extension) )))
> +                                    sizeof(*driver) )))
> +        return STATUS_NO_MEMORY;
> +
> +    if (!(driver->driver_obj = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY,
> +                                    sizeof(*driver->driver_obj) + sizeof(*driver->driver_extension) )))
> +    {
> +        RtlFreeHeap( GetProcessHeap(), 0, driver );
>          return STATUS_NO_MEMORY;
> +    }
>  
> -    if ((status = RtlDuplicateUnicodeString( 1, name, &driver->DriverName )))
> +    if ((status = RtlDuplicateUnicodeString( 1, name, &driver->driver_name )))
>      {
> +        RtlFreeHeap( GetProcessHeap(), 0, driver->driver_obj );
>          RtlFreeHeap( GetProcessHeap(), 0, driver );
>          return status;
>      }
>  
> -    extension = (DRIVER_EXTENSION *)(driver + 1);
> -    driver->Size            = sizeof(*driver);
> -    driver->DriverInit      = init;
> -    driver->DriverExtension = extension;
> -    extension->DriverObject   = driver;
> -    extension->ServiceKeyName = driver->DriverName;
> +    driver->driver_extension = (DRIVER_EXTENSION *)((BYTE*)driver->driver_obj + sizeof(*driver->driver_obj));
> +    driver->driver_obj->DriverName      = driver->driver_name;
> +    driver->driver_obj->Size            = sizeof(*driver->driver_obj);
> +    driver->driver_obj->DriverInit      = init;
> +    driver->driver_obj->DriverExtension = driver->driver_extension;
> +    driver->driver_extension->DriverObject   = driver->driver_obj;
> +    driver->driver_extension->ServiceKeyName = driver->driver_name;
>  
> -    status = driver->DriverInit( driver, name );
> +    status = driver->driver_obj->DriverInit( driver->driver_obj, name );
>  
>      if (status)
>      {
> -        RtlFreeUnicodeString( &driver->DriverName );
> +        RtlFreeUnicodeString( &driver->driver_name );
> +        RtlFreeHeap( GetProcessHeap(), 0, driver->driver_obj );
>          RtlFreeHeap( GetProcessHeap(), 0, driver );
>      }
> +
> +    list_add_tail( &created_drivers, &driver->entry );

Such a global list will need locking.

>      return status;
>  }
>  
> @@ -855,12 +876,23 @@ NTSTATUS WINAPI IoCreateDriver( UNICODE_STRING *name, PDRIVER_INITIALIZE init )
>  /***********************************************************************
>   *           IoDeleteDriver   (NTOSKRNL.EXE.@)
>   */
> -void WINAPI IoDeleteDriver( DRIVER_OBJECT *driver )
> +void WINAPI IoDeleteDriver( DRIVER_OBJECT *driver_object )
>  {
> -    TRACE("(%p)\n", driver);
> +    created_driver *driver, *next;
> +
> +    TRACE("(%p)\n", driver_object);
> +

By putting the driver_object in the same struct, you could use CONTAINING_RECORD here.
Also, a lock will be required while unlinking the driver from the list.

> +    LIST_FOR_EACH_ENTRY_SAFE(driver, next, &created_drivers, created_driver, entry)
> +    {
> +        if (driver->driver_obj == driver_object)
> +        {
> +            list_remove( &driver->entry );
> +            RtlFreeUnicodeString( &driver->driver_name);
> +            break;
> +        }
> +    }
>  
> -    RtlFreeUnicodeString( &driver->DriverName );
> -    RtlFreeHeap( GetProcessHeap(), 0, driver );
> +    RtlFreeHeap( GetProcessHeap(), 0, driver_object );
>  }
>  
>  
> 
> 
>