Posted Jul 17, 2009 14:46 UTC (Fri) by trasz (guest, #45786)
[Link]
Which is false, IMHO - it is readily visible that you're checking the pointer few lines below dereferencing it.
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 14:53 UTC (Fri) by spender (subscriber, #23067)
[Link]
But it would be clear from the source that the bug is not exploitable for privilege escalation (it would just cause a crash when dereferencing TUN). Say if I had a page mmaped at 0 in that case and this gcc problem didn't exist: then there would be no crash, and the function would have returned with an error (due to the !tun check). But because that check doesn't exist in the compiled code, not only does no crash occur, but I'm able to do what should be impossible from a review of the source: get arbitrary code execution in kernel context. It's all explained very well in the exploit code (80% of it is comments), if you would have read it.
-Brad
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 15:05 UTC (Fri) by trasz (guest, #45786)
[Link]
Ok, I agree. So, to sum up, it's a programming error that normally would just make the kernel crash, but due to an compiler optimization it makes the kernel exploitable.
One thing I wonder if whether the optimization is actually invalid. What does the C standard say about program behaviour after dereferencing NULL pointer? Does it say anything?
C's notion of null is not really relevant
Posted Jul 17, 2009 15:45 UTC (Fri) by xoddam (subscriber, #2322)
[Link]
It's normal, and could make sense, to succeed in dereferencing a pointer containing zero if you have readable memory mapped at address zero. Some older platforms used 'page zero' memory as cache -- it was faster (or took less object code) to reach.
So the C standard can say nothing about it. Where the C standard does mention null pointers is in saying that, when assigning (or comparing) the constant zero to a pointer, a specific value must be used that means 'null'. The semantics of this 'null' value need not be the same as the address zero (ie. it may be some special nonzero bit pattern when represented in a particular pointer of a particular type) and this allows for weird platforms where zero is a commonly used address value and there is some other special invalid address register value which can be efficiently detected or used to raise an exception.
OTOH Linux could say something about it, eg. by not permitting anyone to map anything at address zero. This might break one or two native-code emulators of ancient zero-page-using OSes but surely no-one will cry over those in 2009.
None of which is relevant to whether it's legal for gcc to remove code during optimisation that relies on common-but-not-universal behaviour. This is definitely a compiler bug.
It's possible in practice for the pointer to be zero, and for execution to continue after reading the contents of address zero, so the test should never have been removed.
C's notion of null _is_ the _only_ relevant
Posted Jul 19, 2009 0:38 UTC (Sun) by xilun (subscriber, #50638)
[Link]
"C's notion of null is not really relevant" <-- yeeeehhhaaaaaa! let's write a C compiler, but let's redefine the langage, because nothion defined by the standard are not relevant. What a joke. Create your own langage if you want. Leave C and GCC alone.
Re read C99 again. And again. And again. Maybe then you will understand. The null pointer is not just a pointer containing zero. The null pointer is something you should not dereference. Ever. Any decent C programmer knows that. It's also known for a while now that such errors are sometimes exploitable.
A pointer containing zero is not a valid pointer supported by the C langage if it happens that the representation of a null pointer is value zero, because it would then break LOT of clauses of the standard. If you want to dereference a pointer containing zero, you must do that in assembly langage. Any other way is calling for problems.
So in the name of what can this be a compiler bug when the compiler is absolutely compliant with the langage standard on this point? You are misunderstanding what C is and what it is not. The most it could be is a feature you could request, but you do not even have to because this feature already exist (there is a flag to disable the optimisation, so _this_ particular point _is_ defined at least for the translation phase when you use the flag).
If the kernel support mapping address 0 and given that Linux only support systems where the representation of NULL is 0x0 AFAIK, they should use the flag. GCC maintainers for the C language just don't have to make the default behavior defined for every undefined behavior of the standard just because you want that, even if binaries are 5x slower. C is not Java; live with it.
C's notion of null _is_ the _only_ relevant
Posted Jul 19, 2009 11:19 UTC (Sun) by nix (subscriber, #2304)
[Link]
The *attacker* mmap()ed address zero, not the kernel. I suppose there
should be an option to make NULL not all-bits-zero when inside the kernel,
but then you'd have to adjust pointers in transit from userspace and
comparisons to it would be slower and so on.
C's notion of null _is_ the _only_ relevant
Posted Jul 20, 2009 0:16 UTC (Mon) by xoddam (subscriber, #2322)
[Link]
Okay. My mistake. I re-read it.
The standard does indeed say you can't rely on successfully dereferencing null.
(So if you need to read the contents of memory at zero, you are doing something nonstandard and should explicitly tell the compiler if you want it to mean something, or use assembler as you propose).
OTOH if I as a C programmer don't *need* the contents of memory at zero (ie. I'm not writing wine or DOSEmu or equivalent) but a pointer may be invalid, I must do any necessary checking *before* dereferencing a pointer.
So I must grudgingly admit that the compiler is within its rights to make my program do something -- anything -- utterly unexpected once I've made such a stupid mistake as dereferencing null.
But in general programs that do utterly unexpected stuff in any circumstances are bad practice.
So I'd sure-as-hell like a big fat WARNING if the optimiser proposes to remove an entire if statement and thereby possibly make my OS kernel behave -- in practice, not in the meta-universe of the C standard -- in unexpected, exploitable ways.
If there's no such warning from gcc, that *is* a bug. Just IMO.
C's notion of null _is_ the _only_ relevant
Posted Jul 20, 2009 6:47 UTC (Mon) by nix (subscriber, #2304)
[Link]
Some undefined behaviour does require a diagnostic, but the universe of
undefined behaviour is unbounded, and determining if some things are
undefined rams you right into Rice's theorem and the halting problem.
Spotting null dereferences in the general case certainly is (although
warning when the compiler *already* spots a null dereference, as you
propose, is not hard: have you thought about the case of NULL dereferences
being brought into a function via inlining, though? I'd try a GCC using
this warning on a large template-heavy C++ codebase before considering it:
see how many FPs you get.)
C's notion of null _is_ the _only_ relevant
Posted Jul 21, 2009 6:22 UTC (Tue) by alankila (subscriber, #47141)
[Link]
You may be underestimating how easy it is to generate dead code. Functions which are called with statically allocated objects passed via pointers will never get a NULL and thus any if (foo == NULL) check is unnecessary. A good compiler doesn't generate object code that spends time testing conditions that can't be true, so it is reasonable that it eliminates this test. Neither can it produce a warning without driving everyone crazy, because I stipulate that this is a very common situation in defensively written code.
C's notion of null _is_ the _only_ relevant
Posted Jul 21, 2009 7:09 UTC (Tue) by xoddam (subscriber, #2322)
[Link]
Determining that code is dead is easy (and I heartily approve it) if the actual values can be computed at compile time. For the particular case you mention (all callers pass pointers which are known not to be null), you would probably need whole-program optimisation to determine it.
However, knowing that the program has already attempted to dereference a pointer is not quite the same as statically determining that the pointer is definitely non-NULL.
I submit that removing such a test when some possible sources of the value are not visible to the compiler is an excessive optimisation and warrants a warning.
People write defensive code for a reason.
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 15:48 UTC (Fri) by clugstj (subscriber, #4020)
[Link]
"C" says that once you dereference a null pointer, all bets are off. UNIX says that you get a SIG_SEGV signal that can't be restarted. So, the GCC optimization is for usermode code only.
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 16:09 UTC (Fri) by bluebirch (guest, #58264)
[Link]
I don't understand why the optimisation should be wrong for kernel code. It doesn't violate the "all bets are off".
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 17:11 UTC (Fri) by clugstj (subscriber, #4020)
[Link]
True, but since the kernel developers expect a certain thing to happen when a null pointer is dereferenced, they were caught unaware of the effect of the optimization. (my guess)
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 21:01 UTC (Fri) by stevenb (guest, #11536)
[Link]
Then kernel folks should use -fno-delete-null-pointer-checks.
Linux 2.6.30 exploit posted
Posted Jul 21, 2009 7:23 UTC (Tue) by xoddam (subscriber, #2322)
[Link]
As I understand it, that option would also prevent the optimisation in the case where the pointer's value can be determined not to be NULL at compile time, eg. by having already passed such a test in all callers.
But it's *fine* to remove a test and its consequent if the result can be determined by the compiler. It's not that we don't want the compiler to optimise, it's that this particular optimisation removes a *necessary* check whose result was *not* known at compile time.
(YES, the pointer was dereferenced already so the program's behaviour is -- if the test is true -- already UNDEFINED and it's no-one's fault but the programmer's if he's eaten by a sea serpent. But the dereference does not actually prove that the pointer is not null, so IMO the compiler is not justified in removing this test without a warning).
I'm not asking the compiler to warn every time it removes dead code. I'm asking for the compiler not to assume that a pointer value is valid, and thus that tests for its validity are dead code, once it has been dereferenced.
Coders are human, not gods. Compilers are not gods either, merely tools for coders.
Linux 2.6.30 exploit posted
Posted Jul 17, 2009 16:05 UTC (Fri) by forthy (guest, #1525)
[Link]
The behavior of dereferencing a null pointer (or any other invalid
pointer) is undefined (Page 79 of the ISO/IEC 9899:1999 draft). This
doesn't mean "crash" - undefined means undefined, you only can know if you
define it (and then the compiler has to ensure that the implementation-
The dereferencing worked, so it can't be the null pointer. This is the
same rubbish argument that (x+n >= x)=true, because x+n didn't fail, and
overflows are undefined by the C99 standard (but not by the execution
model used on GCC's targets, which all use two's complement circular
number spaces!).