search

microsoft / DMF

Driver Module Framework
MIT License
318 stars 83 forks source link

Trying to use Dmf_IoctlHandler with Control Device Object, supported? #228

Closed nefarius closed 2 years ago

nefarius commented 2 years ago

I replaced a lot of verbose code with Dmf_IoctlHandler on my control device but I found multiple issues in doing so. While it seems to "work" the way I want, Driver Verifier has several complaints:

DMF_DmfControlDeviceInitAllocate

DMF_DmfControlDeviceInitAllocate behaves slightly differently depending on supplying a PWDFDEVICE_INIT or NULL, but both logic branches set dmfDeviceInit->FileObjectConfigHooked to TRUE, which triggers the assert in DMF_DmfDeviceInitHookFileObjectConfig because it thinks the hooks have been set already. Is this intentional? Am I using it wrong?

DMF_DmfDeviceInitHookFileObjectConfig

If I omit calling DMF_DmfDeviceInitHookFileObjectConfig my IOCTL handlers get fired but now ofc. the module won't get its file object hooks called so depending on the use case it's only partially operable. E.g. checking administrative access will not work.

I hope I could explain my setup well enough, am wondering if it's working as intended or an oversight.

Cheers

samtertzakian commented 2 years ago

Hi Benjamin I am pretty sure I have an example of using IoctlHandler with a Control Device. Let me look for it and get back to you. If I recall DMF has some specific functions for dealing with a Control Device. I will try to get all this together within 12 to 24 hours.

On Sun, Nov 6, 2022, 7:58 AM Benjamin Höglinger-Stelzer < @.***> wrote:

I replaced a lot of verbose code with Dmf_IoctlHandler on my control device but I found multiple issues in doing so. While it seems to "work" the way I want, Driver Verifier has several complaints: DMF_DmfControlDeviceInitAllocate

DMF_DmfControlDeviceInitAllocate behaves slightly differently depending on supplying a PWDFDEVICE_INIT or NULL, but both logic branches set dmfDeviceInit->FileObjectConfigHooked to TRUE, which triggers the assert in DMF_DmfDeviceInitHookFileObjectConfig because it thinks the hooks have been set already. Is this intentional? Am I using it wrong? DMF_DmfDeviceInitHookFileObjectConfig

If I omit calling DMF_DmfDeviceInitHookFileObjectConfig my IOCTL handlers get fired but now ofc. the module won't get its file object hooks called so depending on the use case it's only partially operable.

I hope I could explain my setup well enough, am wondering if it's working as intended or an oversight.

Cheers

— Reply to this email directly, view it on GitHub https://github.com/microsoft/DMF/issues/228, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHY5IWU76QE5SXAWMEHDH7LWG7IRDANCNFSM6AAAAAARYPQU7Y . You are receiving this because you are subscribed to this thread.Message ID: @.***>

samtertzakian commented 2 years ago

Ok, so I am unable to find an example of where we use IoctlHandler with a Control Device. I have plenty of examples of both independently but not together. So, let me try to make such a driver and see what happens. I will try to do it by end of the week.

DMF has this API DMF_FilterControl_DeviceCreate(). My feeling is that i will use that and try to hook up IoctlHandler that. In the meantime, I am not sure if you used that. You might look at that and see if that helps at all in the meantime in case you have time.

nefarius commented 2 years ago

I looked at that already, my main problem is that I need to register the file I/O callbacks for my use-case, it works already, but verifier complains because the asserts trigger. Obviously they only do in debug builds but the asserts are there for a reason, so I don't just want to ignore them and call it a day.

samtertzakian commented 2 years ago

Can you send the asserts you are receiving from Verifier...or and assert you receive from DMF? There is a chance I might be able to resolve the issue seeing the exact asserts your receive, especially the first one.

You definitely should not ignore the asserts especially from Verifier.

Meanwhile I will try to do what you are doing to see if I can reproduce your issue.

On Wed, Nov 9, 2022, 2:45 AM Benjamin Höglinger-Stelzer < @.***> wrote:

I looked at that already, my main problem is that I need to register the file I/O callbacks for my use-case, it works already, but verifier complains because the asserts trigger. Obviously they only do in debug builds but the asserts are there for a reason, so I don't just want to ignore them and call it a day.

— Reply to this email directly, view it on GitHub https://github.com/microsoft/DMF/issues/228#issuecomment-1308558898, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHY5IWVWRNDI27K6S4SR4E3WHN6ENANCNFSM6AAAAAARYPQU7Y . You are receiving this because you commented.Message ID: @.***>

nefarius commented 2 years ago

Here's the assert:

*** Assertion failed: DmfDeviceInit->FileObjectConfigHooked != TRUEDmfDeviceInit->FileObjectConfigHooked != 1
***   Source File: C:\Users\BenjaminHöglinger-St\Documents\DMF\Dmf\Framework\DmfDeviceInit.c, line 1058

Here is the relevant snippet from my control device creation:

pInit = WdfControlDeviceInitAllocate(
    WdfDeviceGetDriver(Device),
    // Required for unprivileged process access
    &SDDL_DEVOBJ_SYS_ALL_ADM_RWX_WORLD_RWX_RES_RWX
);

if (pInit == nullptr)
{
    status = STATUS_INSUFFICIENT_RESOURCES;
    TraceError(
        TRACE_SIDEBAND,
        "WdfControlDeviceInitAllocate failed with %!STATUS!",
        status
    );
    break;
}

dmfDeviceInit = DMF_DmfControlDeviceInitAllocate(pInit);

if (dmfDeviceInit == nullptr)
{
    TraceError(
        TRACE_DRIVER,
        "DMF_DmfDeviceInitAllocate failed"
    );

    status = STATUS_INSUFFICIENT_RESOURCES;
    break;
}

DMF_DmfControlDeviceInitSetClientDriverDevice(dmfDeviceInit, Device);

//
// Exclusive access OFF, allows multiple handles
// 
WdfDeviceInitSetExclusive(pInit, FALSE);

//
// Assign name to expose
// 
if (!NT_SUCCESS(status = WdfDeviceInitAssignName(pInit, &ntDeviceName)))
{
    TraceError(
        TRACE_SIDEBAND,
        "WdfDeviceInitAssignName failed with %!STATUS!",
        status
    );
    break;
}

WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, FILE_HANDLE_CONTEXT);
attributes.SynchronizationScope = WdfSynchronizationScopeNone;

//
// File object hooks and callbacks
// 

WDF_FILEOBJECT_CONFIG fileObjConfig;
WDF_FILEOBJECT_CONFIG_INIT(
    &fileObjConfig,
    Sideband_EvtWdfDeviceFileCreate,
    Sideband_EvtWdfFileClose,
    WDF_NO_EVENT_CALLBACK // No cleanup callback function
);

fileObjConfig.FileObjectClass = WdfFileObjectWdfCanUseFsContext;

/*
 * TODO: triggers Driver Verifier!
 * See: https://github.com/microsoft/DMF/issues/228
 */
//DMF_DmfDeviceInitHookFileObjectConfig(dmfDeviceInit, &fileObjConfig);

WdfDeviceInitSetFileObjectConfig(
    pInit,
    &fileObjConfig,
    &attributes
);

//
// Prepare device object creation
//

WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, CONTROL_DEVICE_CONTEXT);
attributes.EvtCleanupCallback = Sideband_DeviceContextCleanup;

if (!NT_SUCCESS(status = WdfDeviceCreate(
    &pInit,
    &attributes,
    &controlDevice
)))
{
    TraceError(
        TRACE_SIDEBAND,
        "WdfDeviceCreate failed with %!STATUS!",
        status
    );
    break;
}
samtertzakian commented 2 years ago

Ok, I see. It is not Driver Verifier that is asserting. It is just a plain assert emitted by DMF in DEBUG build... Definitely you should not ignore it. (Driver Verifier asserts are emitted by the OS/WDF.)

Two possibilities are:

  1. The assert is improperly emitted by DMF and should be adjusted to not emit for Control Device. In that case we will fix the assert to not emit in the case of Control Device.

  2. It means there is an error either in how you are using DMF or in DMF itself that causes the assert. In this case, we will let you know the error and try to clarify documentation/samples ... or fix the error in DMF if that is the issue.

The fact you tell me that everything works while the assert emits makes me feel that #1 above is the resolution which is the best case. When you said Driver Verifier I immediately assumed that the assert was legitimate and indicates a real issue. But it is not a Driver Verifier issue...just CHK build issue.

I will investigate further and let you know. I hope to have resolution one way or another by end of this week if possible.

On Wed, Nov 9, 2022, 3:09 AM Benjamin Höglinger-Stelzer < @.***> wrote:

Here's the assert:

*** Assertion failed: DmfDeviceInit->FileObjectConfigHooked != TRUEDmfDeviceInit->FileObjectConfigHooked != 1

*** Source File: C:\Users\BenjaminHöglinger-St\Documents\DMF\Dmf\Framework\DmfDeviceInit.c, line 1058

Here is the relevant snippet from my control device creation:

pInit = WdfControlDeviceInitAllocate(

WdfDeviceGetDriver(Device),

// Required for unprivileged process access

&SDDL_DEVOBJ_SYS_ALL_ADM_RWX_WORLD_RWX_RES_RWX

);

if (pInit == nullptr)

{

status = STATUS_INSUFFICIENT_RESOURCES;

TraceError(

  TRACE_SIDEBAND,

  "WdfControlDeviceInitAllocate failed with %!STATUS!",

  status

);

break;

}

dmfDeviceInit = DMF_DmfControlDeviceInitAllocate(pInit);

if (dmfDeviceInit == nullptr)

{

TraceError(

  TRACE_DRIVER,

  "DMF_DmfDeviceInitAllocate failed"

);

status = STATUS_INSUFFICIENT_RESOURCES;

break;

}

DMF_DmfControlDeviceInitSetClientDriverDevice(dmfDeviceInit, Device);

// // Exclusive access OFF, allows multiple handles // WdfDeviceInitSetExclusive(pInit, FALSE);

// // Assign name to expose // if (!NT_SUCCESS(status = WdfDeviceInitAssignName(pInit, &ntDeviceName)))

{

TraceError(

  TRACE_SIDEBAND,

  "WdfDeviceInitAssignName failed with %!STATUS!",

  status

);

break;

}

WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, FILE_HANDLE_CONTEXT);

attributes.SynchronizationScope = WdfSynchronizationScopeNone;

// // File object hooks and callbacks //

WDF_FILEOBJECT_CONFIG fileObjConfig; WDF_FILEOBJECT_CONFIG_INIT(

&fileObjConfig,

Sideband_EvtWdfDeviceFileCreate,

Sideband_EvtWdfFileClose,

WDF_NO_EVENT_CALLBACK // No cleanup callback function

);

fileObjConfig.FileObjectClass = WdfFileObjectWdfCanUseFsContext;

/*

WdfDeviceInitSetFileObjectConfig(

pInit,

&fileObjConfig,

&attributes

);

// // Prepare device object creation //

WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, CONTROL_DEVICE_CONTEXT);

attributes.EvtCleanupCallback = Sideband_DeviceContextCleanup;

if (!NT_SUCCESS(status = WdfDeviceCreate(

&pInit,

&attributes,

&controlDevice

)))

{

TraceError(

  TRACE_SIDEBAND,

  "WdfDeviceCreate failed with %!STATUS!",

  status

);

break;

}

— Reply to this email directly, view it on GitHub https://github.com/microsoft/DMF/issues/228#issuecomment-1308588731, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHY5IWRANV5G7WEZBW6FYEDWHOA6NANCNFSM6AAAAAARYPQU7Y . You are receiving this because you commented.Message ID: @.***>

nefarius commented 2 years ago

Ok, I see. It is not Driver Verifier that is asserting. It is just a plain assert emitted by DMF in DEBUG build... Definitely you should not ignore it. (Driver Verifier asserts are emitted by the OS/WDF.)

Correct, pardon the confusion, I debugged multiple issues in the same session, so I got the terms mixed up a bit.

samtertzakian commented 2 years ago

Hi Benjamin,

I have reproduced the issue. To clarify, this issue is not directly related with DMF_IoctlHandler. DMF_IoctlHandler is supported for Control Device objects.

The problem you are having is simply the call to DMF_DmfDeviceInitHookFileObjectConfig(). (You don't need to call this function to use DMF_IoctlHandler. You are calling it because your driver needs use `WdfDeviceInitSetFileObjectConfig() to set the WDF_FILEOBJECT Create/Close callbacks and set the specific attribute. Because of that you have to call DMF_DmfDeviceInitHookFileObjectConfig().

I have reproduced the issue using a private driver and I am trying to determine what to do about it.

I will try to update this issue later today after I discuss with my colleagues. Thank you for patience and especially for your feedback.

samtertzakian commented 2 years ago

Hi Benjamin,

OK, here is the proposed fix for the issue you found:

https://github.com/microsoft/DMF/pull/229

Can you try it and verify it works for you?

We still need to do some validation on our end. There was actually another issue the PR corrects where the FileClose callback would not be called. I have updated the NonPnp1 sample to verify that all the WDFFILEOBJECT callbacks are called. I have tried running multiple instances of the application as well as abnormal termination, etc. It all looks good to me.

You always give great feedback and we all here appreciate it.

I will close this issue once you verify you are happy with it. We basically use option one above to allow the call to proceed without assert for Control Devices.

Thank you

nefarius commented 2 years ago

Great! Will test midweek when I'm back at this particular project.

samtertzakian commented 2 years ago

This fix has been merged into master, Release v1.1.130.

https://github.com/microsoft/DMF/releases/tag/v1.1.130

Once you verify it, we can close this issue.

nefarius commented 2 years ago

Sorry for the delay, it's been a surprisingly busy week so far!

samtertzakian commented 2 years ago

No need to apologize. I merged that branch into master so I could merge some other updates. Let me know once you verify and we can close this issue.

Thank you again for your feedback.

On Thu, Nov 17, 2022, 1:48 PM Benjamin Höglinger-Stelzer < @.***> wrote:

Sorry for the delay, it's been a surprisingly busy week so far!

— Reply to this email directly, view it on GitHub https://github.com/microsoft/DMF/issues/228#issuecomment-1319251728, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHY5IWWF5HC7US4KV2DLFDLWI2R3RANCNFSM6AAAAAARYPQU7Y . You are receiving this because you commented.Message ID: @.***>

nefarius commented 2 years ago

Just came around to test on my end, everything looks good now, thank you!

samtertzakian commented 2 years ago

Great to hear. Thank you again for your feedback and patience.

On Sun, Nov 20, 2022, 10:26 AM Benjamin Höglinger-Stelzer < @.***> wrote:

Just came around to test on my end, everything looks good now, thank you!

— Reply to this email directly, view it on GitHub https://github.com/microsoft/DMF/issues/228#issuecomment-1321207689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHY5IWSNC4BEB7EF5EFBOZ3WJJUMHANCNFSM6AAAAAARYPQU7Y . You are receiving this because you commented.Message ID: @.***>