Re: [PATCH v2 0/4] MR413: ntoskrnl: Enumerate child devices on a separate thread.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntoskrnl.exe/pnp.c:
>  switch (type)
>  {
>      case BusRelations:
> -
>        handle_bus_relations( device_object );
> -
>        EnterCriticalSection( &invalidated_devices_cs );
> -
>        invalidated_devices = realloc( invalidated_devices,
> -
>                (invalidated_devices_count + 1) * sizeof(*invalidated_devices) );
> -
>        invalidated_devices[invalidated_devices_count++] = device_object;

Should we use `ObReferenceObject(device_object)` here?  Recent tests seem to fail with intermittent `Got 1 remove events.`, and I suspect reference management could be the culprit.

[PATCH v2 0/4] MR413: ntoskrnl: Enumerate child devices on a separate thread.

[Zebediah Figura]

On 7/12/22 12:23, Jinoh Kang (@iamahuman) wrote:
> Jinoh Kang (@iamahuman) commented about dlls/ntoskrnl.exe/pnp.c:
> >   switch (type)
> >   {
> >       case BusRelations:
> > -
> >        handle_bus_relations( device_object );
> > -
> >        EnterCriticalSection( &invalidated_devices_cs );
> > -
> >        invalidated_devices = realloc( invalidated_devices,
> > -
> >                (invalidated_devices_count + 1) * sizeof(*invalidated_devices) );
> > -
> >        invalidated_devices[invalidated_devices_count++] = device_object;
>
> Should we use `ObReferenceObject(device_object)` here?  Recent tests seem to fail with intermittent `Got 1 remove events.`, and I suspect reference management could be the culprit.

No, the test failure happens because IRP_MN_REMOVE_DEVICE is supposed to 
only be sent when all user handles have closed (which can't happen if 
we're still inside the ioctl handler), but we don't currently respect that.

We don't need to do any refcounting here, because the thread itself is 
the one destroying PnP devices, and it can't destroy a parent device 
without destroying its children first.