| |||||||||||||
Linux 2.6.30 exploit postedLinux 2.6.30 exploit postedPosted Jul 17, 2009 14:28 UTC (Fri) by Ajaxelitus (subscriber, #56754)Parent article: Linux 2.6.30 exploit posted
gcc makes the slightly reasonable assumption that once a pointer has been dereferenced, the successful dereference proves that it is non-NULL, so any checks for NULL afterwards is superfluous.
(Log in to post comments)
Linux 2.6.30 exploit posted Posted Jul 17, 2009 14:30 UTC (Fri) by Ajaxelitus (subscriber, #56754) [Link]
It would have been better for gcc to print a 'test for NULL after use' warning than for it to silently optimize-away the code.
It's not the worst offender Posted Jul 17, 2009 16:15 UTC (Fri) by khim (subscriber, #9252) [Link] It would have been better for gcc to print a 'test for NULL after use' warning than for it to silently optimize-away the code. If you think GCC is bad then think again. RVCT will happily optimize away things like "if (!this) { ... }" - because standard gurantees that this is never NULL!
Linux 2.6.30 exploit posted Posted Jul 18, 2009 1:38 UTC (Sat) by kjp (subscriber, #39639) [Link]
Agreed. I can't believe -Wall wouldn't turn something like that on.
Linux 2.6.30 exploit posted Posted Jul 20, 2009 8:55 UTC (Mon) by tialaramex (subscriber, #21167) [Link]
One of Raymond Chen's rules applies: Features do not exist by default.
If you (or the kernel developer responsible for this goof, or anyone else) want GCC to emit a diagnostic for this scenario then they need to write code to detect the scenario (which may be very tricky depending on how spread through GCC the different aspects of it are) and write an informative warning message.
If nobody has yet done this, there is no warning included in -Wall.
(Insert generic complaint about how when I was a lad we had to write our own compilers, and it were up hill both ways)
Linux 2.6.30 exploit posted Posted Jul 20, 2009 10:58 UTC (Mon) by muntyan (subscriber, #58894) [Link]
> Features do not exist by default.
Or they get removed. GCC folks break warnings by "optimizations".
Linux 2.6.30 exploit posted Posted Jul 20, 2009 19:17 UTC (Mon) by stevenb (guest, #11536) [Link]
Ah, so fact-based, the parent. Examples?
Linux 2.6.30 exploit posted Posted Jul 21, 2009 5:14 UTC (Tue) by muntyan (subscriber, #58894) [Link]
Linux 2.6.30 exploit posted Posted Jul 21, 2009 6:08 UTC (Tue) by stevenb (guest, #11536) [Link]
Great example. It totally justifies your quotes around "optimization" and intentionally breaks warnings.
Not.
Did you actually read the bug? Getting warnings right is hard. See http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings#pro...
Linux 2.6.30 exploit posted Posted Jul 21, 2009 9:43 UTC (Tue) by muntyan (subscriber, #58894) [Link]
Did *you* read the bug report? Some quotes: "this is a regression", "It has never worked on the tree-ssa branch", "The 4.x compilers does not warn when using unset variables. The 3.x compilers did warn on this: ...". Gcc-3 produces better warnings, while gcc-4 gives you code which crashes 2% faster.
Linux 2.6.30 exploit posted Posted Jul 30, 2009 13:59 UTC (Thu) by lysse (guest, #3190) [Link]
Apparently you both read the bug report and came away with different meanings of it. Now stop waving your willies at each other and have a nice cup of tea.
Linux 2.6.30 exploit posted Posted Jul 17, 2009 14:36 UTC (Fri) by epa (subscriber, #39769) [Link]
So why is this assumption true for userspace code but unsound when running in the kernel?
Linux 2.6.30 exploit posted Posted Jul 17, 2009 14:44 UTC (Fri) by trasz (guest, #45786) [Link]
Userspace code would get SEGV signal due to null pointer dereference and probably exit. Pretty much any kernel other than Linux would panic and restart. In Linux there is this strange "Oops" mechanism inspired probably by Windows 95, which makes the kernel try to continue. I've got an impression that it would make the code exit somehow instead of continuing past the null check that got optimized away, though.
Linux 2.6.30 exploit posted Posted Jul 17, 2009 15:11 UTC (Fri) by hegel (guest, #49501) [Link]
Oops mechanism doesn't try to continue either - it kills the current process most of the time (See http://www.faqs.org/docs/Linux-HOWTO/Kernel-HOWTO.html#AE...). The exploit does mmap(NULL, MAP_FIXED) - that's why it doesn't crash..
Linux 2.6.30 exploit posted Posted Jul 18, 2009 9:11 UTC (Sat) by Ross (subscriber, #4065) [Link]
Yes that's the usual response. But the compiler can apply the same logic in userspace and there are ways to make NULL dereferences not crash (installation of a SEGV signal handler, mapping something to page zero).
In any case, it's not like this would be a good idea for userspace code. If you care enough to check if a pointer is NULL, it should really be done before dereferencing it, otherwise it is too late to stop any undefined behavior. Even if you can be sure an implementation will just crash the program, what's the point of adding code afterwards which isn't reachable?
undefined behaviour Posted Jul 17, 2009 15:06 UTC (Fri) by bluebirch (guest, #58264) [Link]
Gcc is making the reasonable assumption that the code doesn't rely on undefined behaviour. This has previously produced security bugs. E.g. when gcc started removing test like: x + n >= x (n>0) which where checking for integer overflows.
undefined behaviour Posted Jul 17, 2009 15:55 UTC (Fri) by forthy (guest, #1525) [Link] The question is "is that reasonable"? And what is "undefined behavior"? The GCC maintainer (the language talibans) argue that whatever the C99 standard defines not accurately (and that's pretty much all of the it ;-) is left open "undefined". I suppose that if x+n >= x holds true, than x+n really ought to be greater than x - which is not true when compiled by GCC. Same for dereferencing the NULL pointer: This is not defined in C, but being not defined does not say "it breaks". When it may not break, optimizing the test away is simply wrong. If GCC's optimizer would work a bit differently, it would reorder the access and the test - because the test causes the function to return without using the accessed field (this value is dead, and dead loads can be optimized away) - and then by the funny logic the test is both vital and can be optimized away ;-). Not. But I've basically given up on the GCC maintainer to do reasonable things, anyway. There interpretation of standards is just upside down: A standard is a compromise between various implementers and users, so that different qualities of implementations can claim they are "standard". It's like the POSIX discussion we had some times ago here (ext4 problems) where POSIX allowed a file system to loose data and leave the filesystem in a limbo state. This is not a good implementation if it does so. Standard writers can drive towards good implementations while still allowing bad ones: Use the word "should" instead of "shall". Like "a file system should preserve a consistent state in case of a crash" - this means "best effort", and the amount of reasonable effort is left to the implementer. A C compiler should wrap around numbers on a two's complement system, not trap, not crash. The code should honor the execution model of the underlying machine (which e.g. can dereference null pointers). And making a language defined only in vague terms is not a good idea - because writing programs in an underspecified language won't work. The "good" practice of implementing such an underspecified language is to define the terms properly and then stick to them - and hope the practice catches on, so that the next round of standardization effort can encode them with "shall" instead of "should".
undefined behaviour Posted Jul 17, 2009 16:29 UTC (Fri) by bluebirch (guest, #58264) [Link]
I think it is safe to say that the gcc maintainers are very closer to the spirit of the C standard:
"The reason some behavior has been left undefined is to allow compilers for a wide variety of instruction set architectures to generate more efficient executable code for well-defined behavior, which was deemed important for C's primary role as a systems implementation language;" (from Wikipedia[1])
Relying on undefined behaviour means the programmer needs to fully understand the (common) implementation. In any case the code is easier to understand if it doesn't rely on this magic. E.g.:
INT_MAX - x >= n // instead of x + n > x
[1] http://en.wikipedia.org/wiki/C_(programming_language)#Undefined_behavior
undefined behaviour Posted Jul 17, 2009 19:18 UTC (Fri) by ehabkost (guest, #46058) [Link]
The point is that gcc itself is relying on undefined behavior, not the program. If dereferencing a NULL pointer is undefined, gcc must NOT assume that the program will crash anyway and the check for NULL can be optimized out.
undefined behaviour Posted Jul 17, 2009 19:38 UTC (Fri) by foom (subscriber, #14868) [Link] Well, that's a wrong point. When the C standard says something is undefined, that means the *compiler* can do whatever it wants. GCC would be within its rights to automatically exec NetHack whenever you dereference a null pointer. :) But instead it chooses to assume that the program will crash, and optimizes the rest of the program accordingly. Which is also a perfectly valid option. "Undefined" is for the benefit of the compiler, not a constraint upon the compiler.
Actually no, the idea is slightly different... Posted Jul 18, 2009 0:57 UTC (Sat) by khim (subscriber, #9252) [Link] GCC would be within its rights to automatically exec NetHack whenever you dereference a null pointer. :) But instead it chooses to assume that the program will crash, and optimizes the rest of the program accordingly. No, no, no. Nothing of the sort. Idea is different and it's somewhat
simpler: That's why it's so hard to undertand for the outsider the discussion
which goes in cyrcles when GCC developers talk with normal users: GCC developer really don't care what happens to the program with undefined behavior. Not one jot. What happens - will happens. ICE, crash, whatever. The programmer must ensue his (or her) program does not contain undefined constructs - then and only then it's time to complain. Note: not all behaviors come from C standard. Some come from other standards, some come from descussions from in mailing lists (for example if you go with C standard it becomes impossible to write multithreaded programs so there are some additiona guarantees invented by GCC developers). But if you agree that something is "undefined behavior" then the resolution WONTFIX comes automatically.
Actually no, the idea is slightly different... Posted Jul 18, 2009 7:03 UTC (Sat) by ABCD (subscriber, #53650) [Link]
I agree with most of your post, except that I think the GCC devs do care if you get an ICE: in that case, there should have been either an informative error message, or some kind of output. As I understand it, in an ideal world (where GCC doesn't have any bugs, per the GCC devs' definition of a GCC bug) an ICE is never the proper response to any input, no matter how undefined.
Actually no, the idea is slightly different... Posted Jul 18, 2009 10:58 UTC (Sat) by nix (subscriber, #2304) [Link]
Undefined behaviour only and precisely means 'behaviour which is not
defined by the language standard'. It is a matter of QoI, tastefulness and portability matter whether the GCC devs choose to define this behaviour. There are many things that ISO C considers undefined that GCC has defined: we call them language extensions. But if GCC also doesn't define what happens, well, it's not tested, anything goes.
(Undefined behaviour isn't a magic word. It means *exactly what it says*.)
Actually no, the idea is slightly different... Posted Jul 19, 2009 0:12 UTC (Sun) by xilun (subscriber, #50638) [Link]
The compiler does not introduce security bugs. The program simply contains bugs, that triggers undefined behaviors, and undefined behaviors can very often be exploited. This is as simple as this.
Do you run all of your programs in production continuously under Valgrind? Because the way you (incorrectly) intrepret the spirit of the standard, I think you should, for your security. Memory is cheap and CPU are fast anyway.
Actually no, the idea is slightly different... Posted Jul 19, 2009 0:36 UTC (Sun) by dlang (✭ supporter ✭, #313) [Link]
what I would consider reasonable 'undefined' behavior for a compiler to do when a null pointer is used would be to put whatever garbage that it wants in the resulting variable. making _use_ of the resulting data is where you would run into grief (because the data is essentially random). another reasonable thing to do would be to do whatever you would do if the pointer was pointing at an address that didn't exist in the system.
deciding to overwrite the hard drive, run nethack, etc may technically qualifies as undefined, but is defiantly not reasonable.
deciding to 'optimize away' a check immediately afterwards that checks if the pointer is null (prior to the resulting variable being used) is not reasonable.
Actually no, the idea is slightly different... Posted Jul 19, 2009 1:22 UTC (Sun) by xilun (subscriber, #50638) [Link]
I'm NOT calling for the compiler to intentionally generate malicious code (that would be stupid, even if still conforming). I'm saying that it does NOT have to generate extra checks by default, nor refrain from optimising out useless ones, because this would not be in the spirit of C.
"deciding to 'optimize away' a check immediately afterwards that checks if the pointer is null (prior to the resulting variable being used) is not reasonable"
In the name of what?
You could abritrarily disallow a lot of optimisations with such edict.
Everybody that actually have read the C standard know that it perfectly allow such optimisation. See 6.3.2.3. See 6.5.3.2 (note)
Refraining from making optimisation just because it could make buggy program buggier is not reasonable. Just fix the buggy program. Or in some limited cases, explicitely use the flag that the GCC maintainers kindly provide you, so that even if your program is not strictly conforming, it remains conforming given this particular compiler and compilation option.
undefined behaviour Posted Jul 17, 2009 19:43 UTC (Fri) by bluebirch (guest, #58264) [Link]
Gcc isn't assuming the program will crash with (this) undefined behaviour. Gcc is assuming that the program doesn't get into undefined behaviour. And if it does, that is a bug in the program.
The correct behaviour isÂ…, well, undefined. So opening a security hole is no less correct (by definition) than crashing or causing "demons to fly out of your nose".
undefined behaviour Posted Jul 17, 2009 20:58 UTC (Fri) by stevenb (guest, #11536) [Link]
That is what you say.
When GCC makes that the default behavior, there will be others bashing GCC for not optimizing away an obvious unnecessary null-pointer check.
Whatever GCC does, there will always be folks around here and everywhere else who disagree with it. GCC bashing is just the favorite hobby of the entire FOSS developer community, it seems. Just sad...
undefined behaviour Posted Jul 18, 2009 9:04 UTC (Sat) by Ross (subscriber, #4065) [Link]
That doesn't quite make sense. First, compilers are able to do whatever they like when a program does something undefined -- so in that sense they certainly rely on the ability to make the program do whatever they like when the behavior is undefined (otherwise the behavior would be specified in the standard and it would no longer be undefined). But where the logic in your statement fails is that it is presumably the program which has a need to stick to defined behavior, to well, act in a useful and predictable manner. Any program relying on some specific thing to happen or not happen when it triggers undefined behavior is going to be unhappy with the results. Even if the code is tested and the programmer can see that the compiler does "the right thing", there is no guarantee it will happen that way every time, or that other compilers (or updates to the same compiler) will produce the same effect.
So your other question is who defines what is defined. The C standard defines what things trigger undefined behavior and it is the obligation of the program to avoid them. It's like a contract between programs and the compiler. In a few places specific compilers may define behavior in some of those cases as extensions, but there is no such extension here. Any program which fails to avoid dereferencing of NULL pointers and cares about the subsequent execution of the program is buggy, and blaming the compiler for the problem is not reasonable.
undefined behaviour Posted Jul 19, 2009 0:01 UTC (Sun) by xilun (subscriber, #50638) [Link]
If a compiler vendor wants to go beyond a standard, defining undefined (by the standard) behaviors, he can. If he does not want, he does not have to, no matter how much you insult him.
If a programer wants to write non portable code, by using such vendor specific extensions, he can. Such constructs indeed exists between Linux and GCC, but not in this case.
If no such extension exists and some code triggers an undefined behavior, then is just a bug in this code. It can be because of a false assumption of the programmer about what the standard says (so the programmer thinks a program is portable and correct on some aspect while it is indeed not) or because of a bug (seems to be the case here). In NO way this is a bug of the compiler. In NO way this is an indication the compiler is bad. There are more people than you suspect that value compilers that produces fast binaries, thanks of optimisations completely allowed by the standard.
As long as the compiler does not pretend to support undefined behaviors it indeed does not support, this is reasonable. This is indeed the very way to use standardized technology : do not rely on non standard features unless you are concious this is non portable and sure the way you use it is supported by the implementation.
And about NULL pointer dereferencing, the unreasonable behavior is indeed to think it can be dereferenced under special circonstances. There is no good reason to do that. The very purpose of the NULL pointer is to designate that something does NOT point to a valid object. ("If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compater unequal to a pointer to any object or function".) The standard even says that NULL is an _invalid_ value for dereferencing a pointer.
As for "A C compiler should wrap around numbers on a two's complement system", this is not in the standard, and this is indeed NOT what a lot of people think it should do when the alternative is an optimisation. This clearly can't be portable as C is not limited to 2 complement computers, and this is not needed even on 2 complement computers as you can cast to (unsigned) and then back to (int) if you really do want 2-complement behavior on such computers, with the additionnal advantage that such construct triggers future readers of the code into thinking there can be an overflow there the original programmer has thought about.
|
|||||||||||||