If you pass invalid parameters, then all bets are off


Alun Williams pointed out that if you pass invalid parameters to DeferWindowPos, it does not destroy the HDWP. Well, yeah, because if you pass invalid parameters, then all bets are off.

Different functions perform different degrees of parameter validation; the degree to which this is done is typically guided by security concerns. Information that crosses security boundaries must be fully-validated, whereas a call to an in-process function has very little in the way of security obligations with respect to invalid parameters, since a bad caller could just mess with the in-process function directly; no need to try to "trick" it with invalid parameters.

In practice, most functions that perform parameter validation go something like this:

SomeFunction(...)
{
 if (any parameter is invalid) {
   signal invalid parameter error in an appropriate manner
 } else {
   actually do something
 }
}

(In some cases, the validation code is not even written by a human being. Instead, there's a script that parses the header files and autogenerates the validation code.)

If there is an invalid parameter, the entire operation is typically abandoned. Because, after all, how can you expect a function even to get off the ground when it doesn't have all its parameters? I mean, how can the DeferWindowPos destroy the HDWP when it fails to validate its parameters, if the invalid parameter might be the HDWP?

Regardless of the degree to which parameter validation occurs, if you pass invalid parameters, then (generally speaking) there are no guarantees. Passing valid parameters is part of the basic ground rules for programming. If you break your end of the deal, then the function is under no obligation to hold up its end.

Comments (32)
  1. unnamed commenter says:

    But what if the parameters were valid? Quote: All the parameters are valid for the function but I sometimes get a failure" (emphasis mine)

  2. mikeb says:

    > All the parameters are valid for the function but I sometimes get a failure" <<

    This is where being able to at least understand some basic x86 assembly comes in handy.  It would be a fairly simple matter in this case to step into the DeferWindowPos() API and determine which parameter is causing the ERROR_INVALID_PARAMETER result.

    I’d be willing to bet one of the parameters is actually invalid.

  3. N. Velope says:

      I thought this issue was decided the other way in the move from windows 3.0 to 3.1 when it was concluded that users hated delayed UAE errors from a random application (that had its memory erroneously locked or something) and would rather have the performance hit of Windows checking parameters and raising an error (usually halting the calling program) and refusing to perform the function and pointing a finger at the actual application causing the error.

      This wasn’t because of security concerns, but because users couldn’t tell which program had initially caused the error eg:  program A would lock or write to memory needed by program B, then program B would crash when it tried to use it even though it did nothing wrong.  Users would then phone the company that made program B to complain and the company could not replicate the error.

      Of course, that was win16 which is way more open to one application hurting another than 32 bit windows, but I thought microsoft had made a general speed vs: accountabilty decision to do lots of parameter checking.  

  4. alex.r. says:

    N. Velope:

    Doing parameter validation is impossible in the general case for non type-safe languages. I doubt that Microsoft made the decision to start doing impossible stuff with the release of Windows 3.1.

  5. N. Velope says:

      Just to clarify – the Windows API functions check parameters when they are called and raise an error if the parameters are bad.  The language calling them is irrelevant.  The only relevant language is the one the API was written in.

      Parameter checking can include seeing if the number is in an expected range or uses an undefined number not on its list of options for doing something to a window or if a memory location is not readable/writable by the caller.  The API function can raise an error or return a value that indicates that the function did not do what was asked.

  6. lonesome coder says:

    Or fail silently. I’ve been using wxWidgets again lately (used it before back when it was wxWindows) and 2 days into it I already had code like

    DoSomething ();

    DoSomething (); //it works the 2nd time

  7. pm2 says:

    Well I was a Windows PM too, but I disagree with the other PM.  I think the system would be more stable if all processes did what MTS did — log a call stack and terminate the process.  then the dev would fix their bugs…

  8. chrismcb says:

    I can’t believe Raymond had to say this.

  9. Nick Lamb says:

    Every C programmer knows that double free() is illegal.

    The GNU C library includes, by default on most systems a set of double free checks. Most programming errors that result in free() being called on the same allocation twice are caught and reported.

    However this reporting uses the system log feature if it is enabled, and the system logging uses functions which themselves allocate memory.

    If the program which experienced the error is threaded then the allocator uses a lock to ensure mutual exclusion. Since this lock is non-recursive, and is already taken when the double free() detection occurs, the allocations from within the logging code will cause deadlock.

    Now, is that deadlock a reasonable example of "all bets are off" due to the invalid parameter passed to free() which set this chain of events off ? Or is it a bug that ought to be fixed ? We shall see, in some future version of GNU libc.

  10. Dean Harding says:

    Nick: I don’t see how it could be an example of "all bets are off". Why do people have to take that statement so literally? Just because "all bets are off" doesn’t mean it’s reasonable to expect the function to format your hard drive, or make the monitor explode — people still have reasonable expectations about how a system will fail.

    Just so people know, "all bets are off" means the function does not have to meet it’s side of the contract if you do not meet your side of the contract. That’s all.

    By the way, to pm (#1) just saying you’re an "ex MSFT windows PM" doesn’t give what you say any extra weight. Especially when you clearly don’t even understand the circumstances of what’s being spoken of.

  11. Pax says:

    Dean: Just because "all bets are off" doesn’t mean it’s reasonable to expect the function to format your hard drive, or make the monitor explode — people still have reasonable expectations about how a system will fail.

    I liken this to the "undefined" marker in standards docs – it means exactly that – if you do this, monkeys might fly out of your butt or the whole world might explode.

    In your first example, say you were meant to call the format-hard-disk function with a letter indicating the drive: e.g., ‘c’ (0x63), ‘d’ (0x64) and you passed in CTRL-D (0x04) thinking that would format drive D.

    The ideal program would return with an error stating "parameter out of range", but let’s say this were the old DOS days when memory was scarce and they stated clearly that all bets were off unless you passed in a value between 0x63 and 0x69 – maybe they’ll just increase your parameter until it’s valid and go ahead anyway (because that code takes up less room than the error string).

    Your second point I’ve seen in the early X setup programs for Linux – if you got your horizontal or vertical refresh rates wrong, they did indded warn you that your monitor might explode.  I think that’s improved quite a bit since then :-).

    I’m not sure it’s wise to talk about reasonable behaviour in failure scenarios.  Reasonable would mean program will fully recover to some people.  To some coders, reasonable may mean "you stuffed up the parameters, YOU fix the problem".

    Pax

  12. Entity says:

    We have a slight problem.

    One of the problems today is not in regards to error checking or parameter checking. But when to correctly use such techniques in the design of the application. For example, placing such parameter checking inside core routines of the program would lead the size and complexity to completely go out of control.

    Most people will be saying "bah you should know this". But how often do I see the basic principles or techniques miss used and the confusion when and how to use is amazing high. This is not helped by the fact that different languages use the same terminology and names to refer to completely different things! Eg.. Java  Exception handeling for error handeling. Where Exception in C++ are exceptional and should not be used for error handeling. Or modern C++ exceptions to be used for error handeling.

    I see that most people are confused about should windows API act as a filter module for all possible incorrect parameters. Then other people will be saying that windows API should act very strictly against incorrectly parameters.

    Raymond did say before hand there has been a shift in the windows API from being correct and now leaning more toward a defensive nature of the API to do no evil, even when the application is at fault. Mainly because when an application fails the user blames Microsoft.

  13. nksingh says:

    An API which fails noisily and immediately on invalid parameters is a godsend for skilled developers since it allows them to narrow down bugs far more quickly than APIs which allow the program to continue with corrupt state until things get so bad that the program crashes for other reasons with no obvious smoking gun.  

  14. pm says:

    What a terrible article. I am ex MSFT windows PM and I am shocked to read something like that

    I agree that calling with invalid params is bad. But the OS should try to be rugged against dumb calls. Leaking handles becuase somebody passed in a bad param is bad.

    I agree that you cannot deal with every piece of garbage that gets passed in, but saying that you should not even try is totally wrong and that the programmer deserves what he gets – reminder – its the *user* that will pay the consequences when he finds that the app crashes after 10 hours of use.

  15. JM says:

    The problem might be that the documentation isn’t helping.

    "If a call to DeferWindowPos fails, the application should abandon the window-positioning operation and not call EndDeferWindowPos."

    Note that it does not say "if a call to DeferWindowPos() fails for any other reason than passing an invalid parameter". Nor does the MSDN at any other point mention "if a Win32 function fails because of invalid parameters, all bets are off and you might as well terminate your process because the functions do not make any guarantees about what they’ll do, if anything, so even safe cleanup is impossible". And good for us that that’s not the general approach, too.

    What the ideal arrangement *would* be is that an application must have a call to EndDeferWindowPos() at the end of every sequence of BeginDeferWindowPos()/DeferWindowPos() calls *no matter what*. It’s alright for the call to EndDeferWindowPos() to fail, as long as it also frees the HDWP while returning failure.

    The problem here is that EndDeferWindowPos() puts itself in a bind by conflating the deallocation of an auxiliary structure with its actual function (moving multiple windows simultaneously). What should happen is something like this:

    AllocateHdwp();

    try {

     [DeferWindowPos();]*

     EndDeferWindowPos();

    } finally {

     DeallocateHdwp();

    }

    Regardless of any failure of DeferWindowPos() and EndDeferWindowPos(), this sequence will not leak. We would have this situation if we call AllocateHdwp "BeginDeferWindowPos" and DeallocateHdwp "EndDeferWindowPos", with the proviso that EndDeferWindowPos *may not fail to deallocate the HDWP if it is valid*. Clearly a separate function would make more sense, since "do nothing if your arguments are invalid" is actually a very useful failure mode.

    The position that functions are free to do *anything* if you pass them invalid arguments, while convenient, is also the absolute least amount of effort you can make towards robustness. It’s certainly true that it’s not principally wrong, as ultimately the application is responsible for its own correctness, but black and white are such harsh colors.

  16. alex.r. says:

    N.Velope:

    The language is relevant. For example, in C seeing ‘if a memory location is not readable/writable by the caller’ would not be of much use. I can easily generate a valid memory address. I can’t invent valid data structures as easily.

    pm:

    > Leaking handles becuase somebody passed in a bad param is bad.

    I don’t see the point really — this thing could never stop.

    If someone calls CloseHandle() with an invalid handle, should the API try to find which handle the programmer really meant to close in order to avoid a leak? Because the *user* will end up with less memory?

    Choosing to stop when no security risk is involved seems like a pretty good choice to me; it also avoids the setting the expectation that the platform will fix whatever problem you have in your own code which makes for a better ecosystem.

  17. Ulric says:

    Seriously, what are you guys doing passing invalid parameters to DeferWindowPos?  What the *heck* are you doing that you don’t know if your HWND are valid or not?  Perhaps programming in C is not for you. :P

    The guy who asked the question said: I’ll call SetWindowPos instead, I’m affraid DeferWindowPos will leak the HDWM

    I say: if you may be working with invalid window handles, you have a bigger problem than leaks.  SetWindowPos will not fix it.

  18. Dean Harding says:

    Pax: You’re also taking things too literally. Of course a "FormatDrive" function might end up formatting the "wrong" drive if you pass it an invalid parameter. If you pass "D" when you meant to pass "C" then it’s still "invalid" from your point of view, but not from the function’s point of view.

    What I’m saying is that it’s *not* reasonable to expect DeferWindowPos to format your hard drive if you pass an invalid parameter. In just the same way that it’s also *not* reasonable to expect free() to deadlock your process if you pass an invalid parameter.

  19. I am relatively new of your blog (a month or so). And going through the various posts here I have come to know of various coding horrors committed by developers.

    Too bad that you have to reiterate the ground rules 100 times.

    One of the interns in my company wrote:

    while (fp != EOF)

    {

      fread(buf, 1, n, fp);

      // …

    }

    and said, "since fread advances the file pointer by the amount it reads from the file, therefore after sometime the file pointer ‘fp’ will reach end-of-file and therefore fp == EOF after sometime."

    I was speechless. Either people don’t know how to read documentation, or are lazy enough to do so, or just plain disinterested in programming.

  20. David Walker says:

    @N. velope: "if a memory location is not readable/writable by the caller"

    I thought you couldn’t tell if a memory location was readable or writable by the caller.  I know that using IsBadReadPtr and IsBadWritePtr is a bad idea.

  21. pm says:

    killing the process is fine, leaking resources is not. This allows the app to ship and punish the user

  22. Dean Harding says:

    pm: you still don’t seem to get it. The application will leak a handle, but Windows is not leaking anything.

  23. Yuhong Bao says:

    "I thought this issue was decided the other way in the move from windows 3.0 to 3.1 when it was concluded that users hated delayed UAE errors from a random application (that had its memory erroneously locked or something) and would rather have the performance hit of Windows checking parameters and raising an error (usually halting the calling program) and refusing to perform the function and pointing a finger at the actual application causing the error."

    That is why, for example, IsBad*Ptr was invented.

  24. Dean Harding says:

    "Applications leaking handles is a potential cause of this."

    That’s definitely quite possible. What’s your point?

  25. Triangle says:

    10:02 PM by Pax

    Your second point I’ve seen in the early X setup programs for Linux – if you got your horizontal or vertical refresh rates wrong, they did indded warn you that your monitor might explode.  I think that’s improved quite a bit since then :-).

    That’s just plain poorly designed software. There is no reason that the person entering choosing the refresh rate might get momentarily distracted and enter in garbage.

    6:25 PM by Dean Harding

    pm: you still don’t seem to get it. The application will leak a handle, but Windows is not leaking anything.

    Have you ever heard stories of windows systems that seems to get slower the longer they stays up? Applications leaking handles is a potential cause of this.

    2:17 AM by Yuhong Bao

    That is why, for example, IsBad*Ptr was invented.

    Yes, but unfortunately due to the oversight of whoever designed those functions, they return their results by directly trying to access said memory location, instead of doing the correct thing (VirtualQuery). And thanks to the oversight of someone else at microsoft, instead of fixing it, it was rationalized that such a function, even if it was corrected, wouldn’t be safe.

    So IsBad*Ptr is out of the picture for now.

  26. Hardee says:

    Is it no wonder that debugging Win32 failures is so much harder than modern API frameworks?

    This ‘undefined behavior’ for predictable errors in the caller takes from the old C standard library approach: "hey, you should know what you’re doing and read the API docs completely, and all background literature, before calling this API."  Yeah, sounds great, because all programmers are Einsteins and no one ever makes mistakes.

    Hence the culture of hardened devs making statements like the above about how ‘if you have an invalid handle then this is the least of your problems’. Perfectly true, obvious and with a slight air of superiority (despite the fact that the person saying has undoubtedly had to learn this lesson the hard way himself). The question is how you know you have a bad handle in the first place and how you can track it down, sir.

    Win332 is incredibly successful and can be learned, but it’s hilarious to see people pretending that these kinds of design decisions still make sense, especially when the designers of those APIs have often recanted and moved on to better things.

  27. I decided to stay on the Design by Contract side for just a little bit. Recently, Raymond Chen posted

  28. I decided to stay on the Design by Contract side for just a little bit. Recently, Raymond Chen posted

  29. Triangle says:

    Dean Harding: What exactly is the difference between Windows leaking a handle and applications leaking handles if the end result is the same ?

  30. Yuhong Bao says:

    My point was that the Is*BadPtr was designed in the Windows 3.1 days, where it did not have the race condition it has today.

  31. Dean Harding says:

    Triangle: The discussion is about what Windows can do about an application that leaks handles. The end-result of applications leaking handles is irrelevent (because we already know happens). If you can figure out a way to stop applications leaking handles, then I’m sure MS research would love to hear from you.

  32. Steven Edwards says:

    Any way you could make that script public? I a lot of developers would find it of use.

Comments are closed.

Skip to main content