Nonsense. This is trivial stuff. [LWN.net]

Nonsense. This is trivial stuff.

Posted Jul 18, 2009 0:54 UTC (Sat) by bojan (subscriber, #14302)
In reply to: Nonsense. This is trivial stuff. by mikov
Parent article: Linux 2.6.30 exploit posted

Yeah, that was what I thought exactly when I saw it:

struct sock *sk = tun->sk;  // initialize sk with tun->sk

if (!tun)
    return POLLERR;  // if tun is NULL return error

It is _way_ to late to check once tun has already been used. The rest is just other problems that may exist already in the kernel.

PS. I am not the author of that article, despite having the same first name.

(Log in to post comments)

Nonsense. This is trivial stuff.

Posted Jul 18, 2009 1:18 UTC (Sat) by spender (subscriber, #23067) [Link]

Way too late to prevent a trivial oops which only terminates the process causing the oops, with no side-effects on system stability? Yes.
Way too late to prevent arbitrary code execution if -fno-delete-null-pointer-checks was used? No.

Without gcc's optimization, the code would have been unexploitable for arbitrary code execution. That's why it's being discussed.

-Brad

Nonsense. This is trivial stuff.

Posted Jul 18, 2009 9:21 UTC (Sat) by Ross (subscriber, #4065) [Link]

You are saying that the optimization is what "changes" this.

But it doesn't even have to be an optimization -- the compiler can assume a pointer is valid anywhere in the execution as long as it has the same value and that value is, was, or is going to be dereferenced. And there is no "standard" behavior without said optimization unless you assume a lot about the way the compiler works (basically it would be assuming the compiler doesn't track which pointers are dereferenced). The program is telling something to the compiler which the compiler is likely to use, just like a compiler will almost always assume local variables keep the same value between assignments (unless their address is passed or they are marked volatile). You wouldn't complain about a compiler "skipping" reads of a stack slot. Dereferencing a pointer is shouting to the compiler: "hey! this pointer is good (obviously not NULL) because I'm using it!".

Nonsense. This is trivial stuff.

Posted Jul 18, 2009 10:05 UTC (Sat) by bojan (subscriber, #14302) [Link]

Yeah, I get that. I'm referring to the code itself, which on its face is trivially wrong. In other words, the check whether tun==NULL should be _before_ sk=tun->sk, not after it.

Nonsense. This is trivial stuff.

Posted Jul 20, 2009 8:07 UTC (Mon) by bojan (subscriber, #14302) [Link]

After reading some of the C spec (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1336.pdf), section 6.3.2.3 has this:

> An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant.57) If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function.

So, my guess is that GCC folks simply put two and two together and got four. In other words, if tun was zero, it would not be pointing to any valid object. Therefore, the fact that the programmer is using it must mean that the programmer _knows_ that it will be pointing to a valid object (i.e. is non-zero). Therefore, the check for it being zero later on is superfluous and can be removed. Ergo, no bug in GCC.