Prefast warning

NTDEV
1

I’m getting a warning from Prefast (WDK 7600.16385.1) that I don’t
understand. Probably I’m missing something obvious, but I’m just not
seeing it. Here’s the code (the request is from a DeviceIoControl call,
and the IOCTL is defined as METHOD_DIRECT_FROM_HARDWARE):

NTSTATUS vmeintWait(PDEVICE_CONTEXT dc, WDFREQUEST Request)
{
NTSTATUS status = STATUS_SUCCESS;
VMEINT_WAIT *in = NULL;
VMEINT_WAIT_RESULT *out = NULL;
size_t size = 0;

do {
// Get the input parameters
if (!NT_SUCCESS(status = WdfRequestRetrieveInputBuffer (
Request, sizeof(*in),
(PVOID *)&in, NULL)))
{
break;
}

// Get the output buffer to write to
if (!NT_SUCCESS(status = WdfRequestRetrieveOutputBuffer(
Request, sizeof(*out),
(PVOID *)&out, &size)))
{
break;
}

RtlZeroMemory(out, sizeof(VMEINT_WAIT_RESULT));
…

I get the following warning on the RtlZeroMemory:

warning 6386: (PFD)Buffer overrun while writing to ‘out’: the
writable size is ‘size’ bytes, but ‘12’ bytes may be written.

I just don’t see what the complaint is; does anyone else see what I’ve
done wrong? Or at least, what I need to do to convince Prefast that
it’s OK?

TIA,

– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

2

Michael Jones wrote:

I’m getting a warning from Prefast (WDK 7600.16385.1) that I don’t
understand. Probably I’m missing something obvious, but I’m just not
seeing it. Here’s the code (the request is from a DeviceIoControl call,
and the IOCTL is defined as METHOD_DIRECT_FROM_HARDWARE):
…
I get the following warning on the RtlZeroMemory:

warning 6386: (PFD)Buffer overrun while writing to ‘out’: the
writable size is ‘size’ bytes, but ‘12’ bytes may be written.

I just don’t see what the complaint is; does anyone else see what I’ve
done wrong? Or at least, what I need to do to convince Prefast that
it’s OK?

I’m guessing Prefast doesn’t know that WdfRequestRetrieveOutputBuffer
will fail if the buffer isn’t as large as the second parameter. Have
you tried using sizeof(VME_INT_WAIT_RESULT) instead of sizeof(*out)?

If that doesn’t do it, you could add a useless

if( size < sizeof(VMEINT_WAIT_RESULT) )
{
status = STATUS_BUFFER_OVERFLOW;
break;
}

or add a #pragma to suppress that warning.

–
Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

3

On 12/5/2013 6:53 PM, Tim Roberts wrote:

I’m guessing Prefast doesn’t know that WdfRequestRetrieveOutputBuffer
will fail if the buffer isn’t as large as the second parameter. Have
you tried using sizeof(VME_INT_WAIT_RESULT) instead of sizeof(*out)?

Yeah, that was the first thing I tried, but Prefast saw right through
that ploy.

If that doesn’t do it, you could add a useless

if( size < sizeof(VMEINT_WAIT_RESULT) )
{
status = STATUS_BUFFER_OVERFLOW;
break;
}

Hmm; I really thought that would do the trick, but no dice; I still get
the same warning.

or add a #pragma to suppress that warning.

I did not know you would use #pragma warning to disable Prefast
warnings. Fugly, but that did the trick. Thanks!

– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

4

And it’s also complaining about this:

DbgPrint(“Releasing PCI target image %Id\n”,
pcit - dc->m_PciTargetImage));

With the following message:

warning 6328: ‘__int64’ passed as parameter ‘2’ when ‘int’ is
required in call to ‘DbgPrint’

But only on 64-bit builds. I have several place likes this, where I’m
printing out the index of a pointer in an array of structures. Since
the difference is a size_t, which is 32 or 64 bits depending on the
platform, I used the %Id format specifier.

Prefast is fine with this in a 32-bit build; it only complains on the
64-bit build. It’s like it just ignores the ‘I’. Anyway, I can
certainly cast the index to an int (these are small arrays), but I
dislike casting.

– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

5

Instead of disabling the warning, it might be more clear to tell code analysis more about the situation. How about:

__analysis_assume( size >= sizeof(VMEINT_WAIT_RESULT) );

That might “read” a bit cleaner?

Peter
OSR

6

> the difference is a size_t, which is 32 or 64 bits depending on the

platform, I used the %Id format specifier.

Use %p for size_t

–
Maxim S. Shatskih
Microsoft MVP on File System And Storage
xxxxx@storagecraft.com
http://www.storagecraft.com

7

Yup. This.

Peter
OSR

8

Well, that certainly reads WAY better than #pragmas; unfortunately, it
doesn’t work. Prefast still issues the same warning. It really makes
me feel uneasy, like there’s something that I’m missing. Here’s the
message:

warning 6386: (PFD)Buffer overrun while writing to ‘out’: the writable
size is ‘size’ bytes, but ‘12’ bytes may be written.

–mkj

On 12/6/2013 9:14 AM, xxxxx@osr.com wrote:

Instead of disabling the warning, it might be more clear to tell code analysis more about the situation. How about:

__analysis_assume( size >= sizeof(VMEINT_WAIT_RESULT) );

That might “read” a bit cleaner?

Peter
OSR

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

9

Better than a cast, for sure (unless I *really* wanted the output in
decimal rather than hex for some obtuse reason).

Thanks!

On 12/6/2013 9:23 AM, xxxxx@osr.com wrote:

Yup. This.

Peter
OSR

– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

10

Indeed.

Why IS it torturing us so?

How about the incredibly annoying:

RtlZeroMemory(out,
( size <= sizeof(*out) ? size : sizeof(*out) );

and NO PreFast fudge?

Peter
OSR

11

Hmm:

DbgPrint(Releasing PCI target image %p\n",
pcit - dc->m_PciTargetImage));

warning 6066: Non-pointer passed as parameter ‘2’ when pointer is
required in call to ‘DbgPrint’

Prefast wins again! Think I’ll just use %d and cast to an int, since in
these cases I know the index is a small integer.

On 12/6/2013 9:23 AM, xxxxx@osr.com wrote:

Yup. This.

Peter
OSR

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

12

Wow. That didn’t work either–same warning. Now I’m REALLY starting to
get worried.

On 12/6/2013 9:47 AM, xxxxx@osr.com wrote:

Indeed.

Why IS it torturing us so?

How about the incredibly annoying:

RtlZeroMemory(out,
( size <= sizeof(*out) ? size : sizeof(*out) );

and NO PreFast fudge?

Peter
OSR

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

13

Well, if I do this, it shuts up:

RtlZeroMemory(out, size);

HOWEVER, if I then add this above it:

if (size > sizeof(*out))
size = sizeof(*out);

Prefast complains. It apparently knows that
WdfRequestRetrieveOutputBuffer has given us the pointer AND the size,
and “thou shalt not mess with it”.

I suppose I could just zero initialize whatever got passed in;
certainly, in my interface DLL I only ever pass a pointer to the
structure. But I dislike touching stuff I don’t need to, just in case
someone in the future passes me a pointer and messes up the size
argument. Although, zeroing the whole thing would certainly get their
attention…

On 12/6/2013 9:47 AM, xxxxx@osr.com wrote:

Indeed.

Why IS it torturing us so?

How about the incredibly annoying:

RtlZeroMemory(out,
( size <= sizeof(*out) ? size : sizeof(*out) );

and NO PreFast fudge?

Peter
OSR

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

14

OK… well. I guess I see what it’s doing.

It knows “size” is the length of “*out” – But it doesn’t know that you’ve previously indicated that the buffer at “out” must be at least sizeof(*out) or it’s not acceptable to you (and WdfRequestRetrieveOutputBuffer will return an error).

But, this does mean that your doing JUST:

RtlZeroMemory(out, size);

Should be entirely valid. At worst, it’ll zero a VALID buffer that’s larger than sizeof(VMEINT_WAIT_RESULT)… which is probably what you want in the long-run. If the user mallocs sizeof(PAGE_SIZE) instead of passing you * VMEINT_WAIT_RESULT… there’s no harm done in zeroing the entire passed-in buffer, right?

Or am I misunderstanding something?

Maybe once it’s a reasonable time on the US West Coast one of the MSFT Static Analysis people will chime in and help us out? We can dream, right?

Is this nice? With PreFast and SDV, you get to debug your code with two entirely different “languages”…

Peter
OSR

15

Why %p? Format %I is specially intended for size_t (a.k.a. uintptr_t).
But it is MS specific; the C99 standard has IIRC %z or %t.

  • pa

On 06-Dec-2013 16:48, Michael Jones wrote:

Hmm:

DbgPrint(Releasing PCI target image %p\n",
pcit - dc->m_PciTargetImage));

warning 6066: Non-pointer passed as parameter ‘2’ when pointer is
required in call to ‘DbgPrint’

Prefast wins again! Think I’ll just use %d and cast to an int, since in
these cases I know the index is a small integer.

On 12/6/2013 9:23 AM, xxxxx@osr.com wrote:
> [quote]
> Use %p for size_t
> [/quote]
>
> Yup. This.
>
> Peter
> OSR
>
>

16

I’m getting this same warning in another case, where I need to do an
RtlCopyMemory (instead of just an RtlZeroMemory). In that case, I can’t
just use ‘size’, since it may be bigger than the thing I’m copying from.

So, any help from the Redmontonians on this would be greatly
appreciated; I would hate to just disable the warning.

TIA,

–mkj

On 12/6/2013 10:23 AM, xxxxx@osr.com wrote:

OK… well. I guess I see what it’s doing.

It knows “size” is the length of “*out” – But it doesn’t know that you’ve previously indicated that the buffer at “out” must be at least sizeof(*out) or it’s not acceptable to you (and WdfRequestRetrieveOutputBuffer will return an error).

But, this does mean that your doing JUST:

RtlZeroMemory(out, size);

Should be entirely valid. At worst, it’ll zero a VALID buffer that’s larger than sizeof(VMEINT_WAIT_RESULT)… which is probably what you want in the long-run. If the user mallocs sizeof(PAGE_SIZE) instead of passing you * VMEINT_WAIT_RESULT… there’s no harm done in zeroing the entire passed-in buffer, right?

Or am I misunderstanding something?

Maybe once it’s a reasonable time on the US West Coast one of the MSFT Static Analysis people will chime in and help us out? We can dream, right?

Is this nice? With PreFast and SDV, you get to debug your code with two entirely different “languages”…

Peter
OSR

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

17

Can you test the code with the win8 kit’s version of prefast?

d

Bent from my phone

From: Michael Jonesmailto:xxxxx
Sent: ?12/?6/?2013 1:36 PM
To: Windows System Software Devs Interest Listmailto:xxxxx
Subject: Re:[ntdev] Prefast warning

I’m getting this same warning in another case, where I need to do an
RtlCopyMemory (instead of just an RtlZeroMemory). In that case, I can’t
just use ‘size’, since it may be bigger than the thing I’m copying from.

So, any help from the Redmontonians on this would be greatly
appreciated; I would hate to just disable the warning.

TIA,

–mkj

On 12/6/2013 10:23 AM, xxxxx@osr.com wrote:
>


>
> OK… well. I guess I see what it’s doing.
>
> It knows “size” is the length of “*out” – But it doesn’t know that you’ve previously indicated that the buffer at “out” must be at least sizeof(*out) or it’s not acceptable to you (and WdfRequestRetrieveOutputBuffer will return an error).
>
> But, this does mean that your doing JUST:
>
> RtlZeroMemory(out, size);
>
> Should be entirely valid. At worst, it’ll zero a VALID buffer that’s larger than sizeof(VMEINT_WAIT_RESULT)… which is probably what you want in the long-run. If the user mallocs sizeof(PAGE_SIZE) instead of passing you * VMEINT_WAIT_RESULT… there’s no harm done in zeroing the entire passed-in buffer, right?
>
> Or am I misunderstanding something?
>
> Maybe once it’s a reasonable time on the US West Coast one of the MSFT Static Analysis people will chime in and help us out? We can dream, right?
>
> Is this nice? With PreFast and SDV, you get to debug your code with two entirely different “languages”…
>
> Peter
> OSR
>

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//

—
NTDEV is sponsored by OSR

Visit the list at: http://www.osronline.com/showlists.cfm?list=ntdev

OSR is HIRING!! See http://www.osr.com/careers

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

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

18

I plan to do that, but there just isn’t time until after I’ve delivered
this version (which has to support XP and later).

I think for now I will actually leave the Prefast warning in, and just
document that it’s a false positive, rather than hiding it with a
#pragma (at least until I can try out the Win8 WDK).

–mkj

On 12/6/2013 4:46 PM, Doron Holan wrote:

Can you test the code with the win8 kit’s version of prefast?

d

Bent from my phone

From: Michael Jones mailto:xxxxx
> Sent: ý12/ý6/ý2013 1:36 PM
> To: Windows System Software Devs Interest List mailto:xxxxx
> Subject: Re:[ntdev] Prefast warning
>
> I’m getting this same warning in another case, where I need to do an
> RtlCopyMemory (instead of just an RtlZeroMemory). In that case, I can’t
> just use ‘size’, since it may be bigger than the thing I’m copying from.
>
> So, any help from the Redmontonians on this would be greatly
> appreciated; I would hate to just disable the warning.
>
> TIA,
>
> --mkj
>
> On 12/6/2013 10:23 AM, xxxxx@osr.com wrote:
>>


>>
>> OK… well. I guess I see what it’s doing.
>>
>> It knows “size” is the length of “*out” – But it doesn’t know that you’ve previously indicated that the buffer at “out” must be at least sizeof(*out) or it’s not acceptable to you (and WdfRequestRetrieveOutputBuffer will return an error).
>>
>> But, this does mean that your doing JUST:
>>
>> RtlZeroMemory(out, size);
>>
>> Should be entirely valid. At worst, it’ll zero a VALID buffer that’s larger than sizeof(VMEINT_WAIT_RESULT)… which is probably what you want in the long-run. If the user mallocs sizeof(PAGE_SIZE) instead of passing you * VMEINT_WAIT_RESULT… there’s no harm done in zeroing the entire passed-in buffer, right?
>>
>> Or am I misunderstanding something?
>>
>> Maybe once it’s a reasonable time on the US West Coast one of the MSFT Static Analysis people will chime in and help us out? We can dream, right?
>>
>> Is this nice? With PreFast and SDV, you get to debug your code with two entirely different “languages”…
>>
>> Peter
>> OSR
>>
>
> –
> – mkj
>
> //
> // Michael K. Jones
> // Stone Hill Consulting, LLC
> // http://www.stonehill.com
> //
>
> —
> NTDEV is sponsored by OSR
>
> Visit the list at: http://www.osronline.com/showlists.cfm?list=ntdev
>
> OSR is HIRING!! See http://www.osr.com/careers
>
> For our schedule of WDF, WDM, debugging and other seminars visit:
> http://www.osr.com/seminars
>
> To unsubscribe, visit the List Server section of OSR Online at
> http://www.osronline.com/page.cfm?name=ListServer

–
– mkj

//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
// </mailto:xxxxx></mailto:xxxxx>

19

On 06-Dec-2013 17:23, xxxxx@osr.com wrote:

I guess I see what it’s doing.

It knows “size” is the length of “*out” – But it doesn’t know that you’ve previously indicated that the buffer at “out” must be at least sizeof(*out) or it’s not acceptable to you (and WdfRequestRetrieveOutputBuffer will return an error).

But, this does mean that your doing JUST:

RtlZeroMemory(out, size);

Should be entirely valid.

If so, it applies “writable size” property to a pointer to a struct that
has a known constant size? This is IMHO already too much bending of the
C language. How a C structure can have “writable size”?

The warning should be at the WdfRequestRetrieveOutputBuffer call,
and say that the returned value of the pointer (&out) can be not what
caller expects. Because, it returns PVOID that can be anything.
Starting from the next statement, it should believe that the
pointer value is 100% valid.

Maybe this logic is applicable to arrays, or structures that have open
arrays as the last member, but not to “normal” structures.

Regards,
– pa

20

I spent quite a bit of time trying to get rid of this same warning and finally used the pragma to suppress it. This was with the win8 kit’s prefast (VS2012 analyze).

 

Bill Wandel

on Dec 06, 2013, Doron Holan wrote:







Can you test the code with the win8 kit’s version of prefast?

d

Bent from my phone



From: Michael Jones
Sent: ‎12/‎6/‎2013 1:36 PM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] Prefast warning



I'm getting this same warning in another case, where I need to do an
RtlCopyMemory (instead of just an RtlZeroMemory). In that case, I can't
just use 'size', since it may be bigger than the thing I'm copying from.

So, any help from the Redmontonians on this would be greatly
appreciated; I would hate to just disable the warning.

TIA,

--mkj

On 12/6/2013 10:23 AM, xxxxx@osr.com wrote:
> [quote]
> Well, if I do this, it shuts up:
>
> RtlZeroMemory(out, size);
>
> HOWEVER, if I then add this above it:
>
> if (size > sizeof(*out))
> size = sizeof(*out);
>
> Prefast complains.
> [/quote]
>
> OK... well. I guess I see what it's doing.
>
> It knows "size" is the length of "*out" -- But it doesn't know that you've previously indicated that the buffer at "out" must be at least sizeof(*out) or it's not acceptable to you (and WdfRequestRetrieveOutputBuffer will return an error).
>
> But, this does mean that your doing JUST:
>
> RtlZeroMemory(out, size);
>
> Should be entirely valid. At worst, it'll zero a VALID buffer that's larger than sizeof(VMEINT_WAIT_RESULT)... which is probably what you want in the long-run. If the user mallocs sizeof(PAGE_SIZE) instead of passing you * VMEINT_WAIT_RESULT... there's no harm done in zeroing the entire passed-in buffer, right?
>
> Or am I misunderstanding something?
>
> Maybe once it's a reasonable time on the US West Coast one of the MSFT Static Analysis people will chime in and help us out? We can dream, right?
>
> Is this nice? With PreFast and SDV, you get to debug your code with two entirely different "languages"...
>
> Peter
> OSR
>

--
-- mkj
_______________________________________________
//
// Michael K. Jones
// Stone Hill Consulting, LLC
// http://www.stonehill.com
//_______________________________________________

---
NTDEV is sponsored by OSR

Visit the list at: http://www.osronline.com/showlists.cfm?list=ntdev

OSR is HIRING!! See http://www.osr.com/careers

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

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


---
NTDEV is sponsored by OSR

Visit the list at: http://www.osronline.com/showlists.cfm?list=ntdev

OSR is HIRING!! See http://www.osr.com/careers

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

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