Strange Prefast warning about DriverObject

I ran prefast from WDK 6000 and got this message:
warning 28131: The DriverEntry routine should save a copy of the
argument DriverObject, not the pointer, since the I/O Manager frees the
buffer after DriverEntry returns

I can’t believe that this is true. The documentation states that the
RegistryPath pointer argument must not be stored because the
UNICODE_STRING gets deleted after DriverEntry returns. There is no such
statement for DriverObject. My understanding is that the DRIVER_OBJECT
is valid until after the DriverUnload call.

Any comments are welcome.

Udo

Of course, you’re right and prefast is wrong. The question is why is prefast
getting so horribly confused.

What does the “offending” code in your driver look like?

-scott


Scott Noone
Software Engineer
OSR Open Systems Resources, Inc.
http://www.osronline.com

“Udo Eberhardt” wrote in message
news:xxxxx@ntdev…
>I ran prefast from WDK 6000 and got this message:
> warning 28131: The DriverEntry routine should save a copy of the argument
> DriverObject, not the pointer, since the I/O Manager frees the buffer
> after DriverEntry returns
>
> I can’t believe that this is true. The documentation states that the
> RegistryPath pointer argument must not be stored because the
> UNICODE_STRING gets deleted after DriverEntry returns. There is no such
> statement for DriverObject. My understanding is that the DRIVER_OBJECT is
> valid until after the DriverUnload call.
>
> Any comments are welcome.
>
> Udo
>

The code is pretty simple:

// global var
DRIVER_OBJECT* gDriverObject = NULL;

NTSTATUS
DriverEntry(
IN PDRIVER_OBJECT DriverObject,
IN PUNICODE_STRING RegistryPath
)
{
gDriverObject = DriverObject; // prefast complains about this line

… more interesting stuff
}

Udo

Scott Noone wrote:

Of course, you’re right and prefast is wrong. The question is why is prefast
getting so horribly confused.

What does the “offending” code in your driver look like?

-scott

Weird, I don’t get that warning when prefasting this

-scott


Scott Noone
Software Engineer
OSR Open Systems Resources, Inc.
http://www.osronline.com

“Udo Eberhardt” wrote in message
news:xxxxx@ntdev…
> The code is pretty simple:
>
> // global var
> DRIVER_OBJECT* gDriverObject = NULL;
>
> NTSTATUS
> DriverEntry(
> IN PDRIVER_OBJECT DriverObject,
> IN PUNICODE_STRING RegistryPath
> )
> {
> gDriverObject = DriverObject; // prefast complains about this line
>
> … more interesting stuff
> }
>
>
> Udo
>
>
> Scott Noone wrote:
>> Of course, you’re right and prefast is wrong. The question is why is
>> prefast getting so horribly confused.
>>
>> What does the “offending” code in your driver look like?
>>
>> -scott
>>
>

Hmm, you are correct. There was a mistake in communication between a
colleague and me. The “offending” piece of code was from a different
driver. Sorry for this inaccuracy.

Actually the driver is written in C++ and prefast complains about the
following construct:

extern “C”
NTSTATUS
DriverEntry(
IN PDRIVER_OBJECT DriverObject,
IN PUNICODE_STRING RegistryPath
)
{
// gKnDriverObject is an instance of class KnDriverObject
NTSTATUS st = gKnDriverObject->DriverEntry(DriverObject,RegistryPath);


}

NTSTATUS
KnDriverObject::DriverEntry(
IN PDRIVER_OBJECT DriverObject,
IN PUNICODE_STRING RegistryPath
)
{
// mDriverObject is a member variable of type DRIVER_OBJECT*
mDriverObject = DriverObject; //on this line prefast reports warning
28131


}

prefast reports:
warning 28131: The DriverEntry routine should save a copy of the
argument DriverObject, not the pointer, since the I/O Manager frees the
buffer after DriverEntry returns.

It seems that prefast confuses KnDriverObject::DriverEntry and
DriverEntry, and in addition confuses DriverObject and RegistryPath
arguments.

Udo

Scott Noone wrote:

Weird, I don’t get that warning when prefasting this

-scott

PFD has hardcoded knowledge about the function named DriverEntry and the
rules that go along with it. I fwd’ed the issue to the owner of PFD,
but in the meantime, what happens if you rename the class function
DriverEntry to something else ?

d

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Udo Eberhardt
Sent: Sunday, December 10, 2006 6:17 AM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] Strange Prefast warning about DriverObject

Hmm, you are correct. There was a mistake in communication between a
colleague and me. The “offending” piece of code was from a different
driver. Sorry for this inaccuracy.

Actually the driver is written in C++ and prefast complains about the
following construct:

extern “C”
NTSTATUS
DriverEntry(
IN PDRIVER_OBJECT DriverObject,
IN PUNICODE_STRING RegistryPath
)
{
// gKnDriverObject is an instance of class KnDriverObject
NTSTATUS st =
gKnDriverObject->DriverEntry(DriverObject,RegistryPath);


}

NTSTATUS
KnDriverObject::DriverEntry(
IN PDRIVER_OBJECT DriverObject,
IN PUNICODE_STRING RegistryPath
)
{
// mDriverObject is a member variable of type DRIVER_OBJECT*
mDriverObject = DriverObject; //on this line prefast reports warning
28131


}

prefast reports:
warning 28131: The DriverEntry routine should save a copy of the
argument DriverObject, not the pointer, since the I/O Manager frees the
buffer after DriverEntry returns.

It seems that prefast confuses KnDriverObject::DriverEntry and
DriverEntry, and in addition confuses DriverObject and RegistryPath
arguments.

Udo

Scott Noone wrote:

Weird, I don’t get that warning when prefasting this

-scott


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

To unsubscribe, visit the List Server section of OSR Online at
http://www.osronline.com/page.cfm?name=ListServer

The PFD had this to say.

  1. look for the following informational warning, you should have 2 of them in the log. that is the first clue.

1: The Drivers module has inferred that the current function is a DRIVER_INITIALIZE function: This is informational only. No problem has been detected.
FUNCTION: DriverEntry (17)

1: The Drivers module has inferred that the current function is a DRIVER_INITIALIZE function: This is informational only. No problem has been detected.
FUNCTION: KnDriverObject::DriverEntry (30)

  1. The bug in PFD is that it using the C simple name vs the fully qualified class name. The developer has filed a bug to track this issue and potentially fix it in the future.

  2. The reason it is flagging DriverObject for a deep copy is since you are calling a C++ function, the “this” parameter is the first parameter and DriverObject is the 2nd, and PFD explicitly looks for the usage of the 2nd parameter (RegistryPath) to make sure you make a deep copy, so the “this” parameter is throwing PFD off.

Simple solution: rename the function to something else and PFD won’t get lost

d

Hi Doron,

thanks for your help in this matter. I tried your advice. Here are the
results:

  • The PFD log contains the two informational messages exactly as you
    wrote. PFD says ‘The Drivers module has inferred that the current
    function is a DRIVER_INITIALIZE function…’ for both functions, the
    real extern “C” DriverEntry and the member function
    KnDriverObject::DriverEntry.

  • I renamed KnDriverObject::DriverEntry to something else, e.g.
    KnDriverObject::Driver_Entry_Handler. PFD still reports these two
    informational messages, the second one applies to
    KnDriverObject::Driver_Entry_Handler now. PFD still reports the strange
    warning about DriverObject. So PFD’s assumption seems not to be based on
    the function’s name.

  • I removed the call
    gKnDriverObject->DriverEntry(DriverObject,RegistryPath);
    from the DriverEntry function. PFD still reports the same
    DRIVER_INITIALIZE message and still reports the warning on DriverObject.
    So PFD’s assumption seems also not to be based on the chain of calls.

  • I removed the RegistryPath parameter from the
    KnDriverObject::DriverEntry function. So the functions signature now is:
    NTSTATUS
    KnDriverObject::DriverEntry(
    IN PDRIVER_OBJECT DriverObject
    );
    With this, PFD does not report the “DRIVER_INITIALIZE function”
    statement for KnDriverObject::DriverEntry and does also not report the
    strange warning on the DriverObject argument.
    The same thing happens when I keep DriverObject and RegistryPath but add
    a dummy parameter to KnDriverObject::DriverEntry so that the function
    has 3 arguments.

My conclusion: PFD seems to assume that a given function is a
DRIVER_INITIALIZE function if it has exactly two parameters of type
PDRIVER_OBJECT and PUNICODE_STRING. PFD seems not to take the function’s
name into account.

Your explanation that the implicit “this” parameter causes PFD to
confuse DriverObject and RegistryPath sounds plausible to me.

Udo

xxxxx@Microsoft.com wrote:

The PFD had this to say. 1) look for the following informational
warning, you should have 2 of them in the log. that is the first
clue.

1: The Drivers module has inferred that the current function is a
DRIVER_INITIALIZE function: This is informational only. No problem
has been detected. FUNCTION: DriverEntry (17)

1: The Drivers module has inferred that the current function is a
DRIVER_INITIALIZE function: This is informational only. No problem
has been detected. FUNCTION: KnDriverObject::DriverEntry (30)

  1. The bug in PFD is that it using the C simple name vs the fully
    qualified class name. The developer has filed a bug to track this
    issue and potentially fix it in the future.

  2. The reason it is flagging DriverObject for a deep copy is since
    you are calling a C++ function, the “this” parameter is the first
    parameter and DriverObject is the 2nd, and PFD explicitly looks for
    the usage of the 2nd parameter (RegistryPath) to make sure you make a
    deep copy, so the “this” parameter is throwing PFD off.

Simple solution: rename the function to something else and PFD won’t
get lost

d

Since it appears to be inferring based on the parameter list, the simple
fix should be to swap the order of the parameters in your class member,
which should silence the warning entirely.

Phil

Philip D. Barila
Seagate Technology LLC
(720) 684-1842

From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Udo Eberhardt

Sent: Wednesday, December 13, 2006 2:39 PM
To: “Windows System Software Devs Interest List”
Subject: Re:[ntdev] Strange Prefast warning about DriverObject

Hi Doron,

[snip]

My conclusion: PFD seems to assume that a given function is a
DRIVER_INITIALIZE function if it has exactly two parameters of type
PDRIVER_OBJECT and PUNICODE_STRING. PFD seems not to take the function’s
name into account.

[snip]

I spoke with the prefast developer and he confirmed that the detection for DriverEntry is based off of signature, not name. He will see if can detect that the function is a thiscall type of call and ignore it. Like Phil suggested, he also suggested swapping the parameters or putting a #pragma warning(supress or disable) if you really want to keep it the same.

d

Thanks Doron, thanks Phil. Swapping the parameters is a good idea to
work around this problem. My first idea was to add a dummy parameter,
but this is less elegant.

Thanks for great support.

Udo

xxxxx@Microsoft.com wrote:

I spoke with the prefast developer and he confirmed that the detection for DriverEntry is based off of signature, not name. He will see if can detect that the function is a thiscall type of call and ignore it. Like Phil suggested, he also suggested swapping the parameters or putting a #pragma warning(supress or disable) if you really want to keep it the same.

d