[Mesa-dev] [PATCH 0/2] Simple Klocwork patches

[Ian Romanick]

On 02/07/2016 02:07 PM, Matt Turner wrote:
> On Sun, Feb 7, 2016 at 1:37 PM, Juha-Pekka Heikkilä
> <juhapekka.heikkila at gmail.com> wrote:
>> Hi Iago,
>>
>> I know there are lot of places where there is malloc unchecked still
>> -- and then there is ralloc which is a story of its own. Reason why I
>> think checking these would be remotely useful in windows only (or
>> other way around, not under linux kernel) is on Windows one can get
>> the null pointer from malloc. On Androids I think memory over
>> committing has always been enabled and on Linux I suspect I belong to
>> the minority who like to set ulimits for memory.
>>
>> I agree checking these mostly is quite useless but there are those
>> corners where it may suddenly become valuable. When process is running
>> and everything has settled it will be weird if hit any of these checks
>> but any code which is run when process is starting I notice is the
>> place where things will fail if they fail. This is of course just my
>> opinion about the value of these checks but I really dislike
>> possibility of segfault when it is coming from a library.
>>
>> I didn't quickly notice where _mesa_error() get more heap. Stack it of
>> course needs but when I did stress test these _mesa_error() did still
>> work. Cannot promise my test was 100% correct though, I think it was
>> over year ago when I was playing with it.
> 
> There's no guarantee that fprintf() doesn't call malloc. In fact, glibc's does.
> 
> Adding these checks is really useless.

Maybe.  What Klocwork tries to detect isn't that your program might
crash because of a failed allocation.  It's trying to detect that malloc
returns NULL, then, perhaps through a series of offset operations, you
reference some other valid memory and get a weird result that could be
used for an exploit.  It's super paranoid and overly conservative in
these estimations.

This

    *(char *)NULL = 'x';

will crash.  This

    ((int *)NULL)[some_value] = 42;

is much less predictable... especially if some_value comes directly or
indirectly from user input.  Since the dereference may occur far, far
away from the allocation, it's halting-problem hard for a static
analysis tool to determine it cannot happen.

That said, I think the way we're currently adding this checks is pretty
much rubbish.  There's no testing, and, as a result, an unfortunate
percentage of the added tests have had bugs or one sort or another.

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>