New Linux Kernel Flaw Allows Null Pointer Exploits

Trailrunner7 writes "A new flaw in the latest release of the Linux kernel gives attackers the ability to exploit NULL pointer dereferences and bypass the protections of SELinux, AppArmor and the Linux Security Module. Brad Spengler discovered the vulnerability and found a reliable way to exploit it, giving him complete control of the remote machine. This is somewhat similar to the magic that Mark Dowd performed last year to exploit Adobe Flash. Threatpost.com reports: 'The vulnerability is in the 2.6.30 release of the Linux kernel, and in a message to the Daily Dave mailing list Spengler said that he was able to exploit the flaw, which at first glance seemed unexploitable. He said that he was able to defeat the protection against exploiting NULL pointer dereferences on systems running SELinux and those running typical Linux implementations.'"

Comments

Just don't use that version

[BPPG]

It's important to note that there is almost never any "preferred" or "special" release of Linux to use. And obviously this flaw doesn't affect people that don't use any security modules.

This is not good news, but it's important news. The kernel's not likely to have a "fixed" re-release for this version, although there probably will be patches for it as well. And when in doubt, just don't upgrade. Not very many machines can take advantage of all of the cool bleeding-edge features that come with each release, anyways. Lots of older versions get "adopted" by someone who will continue to maintain that single kernel release.

Re:Just don't use that version

[Kjella]

It's important to note that there is almost never any "preferred" or "special" release of Linux to use. (...) And when in doubt, just don't upgrade. Not very many machines can take advantage of all of the cool bleeding-edge features that come with each release, anyways. Lots of older versions get "adopted" by someone who will continue to maintain that single kernel release.

As a guess pulled out of my nethers 99% use their distro's default shipping kernel, which means there's maybe a dozen kernels in widespread use with a long tail. Unless you're living on the bleeding edge that's what you want to do, otherwise you have to keep up with and patch stuff like this yourself. I'd much rather trust that than not upgrading or picking some random kernel version and hope it's adopted by someone.

Re:Just don't use that version

[mvdwege]

I'm very sorry, but you are wrong.

There is no longer an unstable/stable kernel branch difference. Essentially all new kernels are development versions. It is specifically up to the distribution vendors to pick stable kernels out of this continuous release stream.

Mart

DRM is defective by design.

[BPPG]

I think that tag is mostly reserved for DRM related news...

And I have seen news about linux DRM modules also tagged that.

Re:Double standards

[Anonymous Coward]

Thats because with Windows, no one would be able to marvel at how un-obvious the flaw is. According to The Register, the kernel actually has gaurds in place against just this type of valnerability, but the complier optimized them out during compiling. IMHO this makes this flaw a very good case study, even with security in place, you cannot really trust the compiler. (actually, this flaw apparently only occurs if security is in place... or if you use PulseAudio (in which case, you deserve it!)).

Re:Double standards

[Shinobi]

And yet comp sci trash wonder why some of us actually learn assembler, and don't blindly trust compilers and libraries.

Wait, what?

[TheRaven64]

This code looks right?

struct sock *sk = tun->sk; // initialize sk with tun->sk
...
if (!tun)
return POLLERR; // if tun is NULL return error

So, he's dereferencing tun, and then checking if tun was NULL? Looks like the compiler is performing an incorrect optimisation if it's removing the test, but it's still horribly bad style. This ought to be crashing at the sk = tun->sk line, because the structure is smaller than a page, and page 0 is mapped no-access (I assume Linux does this; it's been standard practice in most operating systems for a couple of decades to protect against NULL-pointer dereferencing). Technically, however, the C standard allows tun->sk to be a valid address, so removing the test is a semantically-invalid optimisation. In practice, it's safe for any structure smaller than a page, because the code should crash before reaching the test.

So, we have bad code in Linux and bad code in GCC, combining to make this a true GNU/Linux vulnerability.

Re:Wait, what?

[TheSunborn]

I think the compiler is correct. If tun is null, then tun->sk is undefined and the compiler can do what even optimization it want.

So when the compiler see tun->sk it can assume that tun is not null, and do the optimization, because IF tun is null, then the program is invoked undefined behavier, which the compiler don't have to preserve/handle. (How do you keep the semantic of an undefined program??)

Re:Wait, what?

[pdh11]

Technically, however, the C standard allows tun->sk to be a valid address, so removing the test is a semantically-invalid optimisation.

No. Technically, if tun is null, dereferencing it in the expression tun->sk invokes undefined behaviour -- not implementation-defined behaviour. It is perfectly valid to remove the test, because no strictly conforming code could tell the difference -- the game is already over once you've dereferenced a null pointer. This is a kernel bug (and not even, as Brad Spengler appears to be claiming, a new class of kernel bug); it's not a GCC bug.

But as other posters have said, it would indeed be a good security feature for GCC to warn when it does this.

Peter

Re:Wait, what?

[TheRaven64]

No. Technically, if tun is null, dereferencing it in the expression tun->sk invokes undefined behaviour -- not implementation-defined behaviour

I've seen a lot of people claiming that, however (as someone who hacks on a C compiler) there are a few things I take issue with in your assertion.

First, NULL is a preprocessor construct, not a language construct; by the time it gets to the compiler the preprocessor has replaced it with a magic constant[1]. The standard requires that it be defined as some value that may not be dereferenced, which is typically 0 (but doesn't have to be, and isn't on some mainframes). Dereferencing NULL is invalid, however that is not what is happening here.

The value &(tun->sk) is the address of tun, plus a fixed offset. The expression &(((struct foo*)0)->bar) is valid C and will give the value of the offset of the sk field in the foo struct. A typical definition of NULL is (void*)0, and &(((struct foo*)(void*)0)->bar) will also give the value of the offset of the bar field.

In this case, it is tun->sk, not &(tun->sk) which is being loaded, however the pointer arithmetic which generates the address happens first. If tun is NULL then this is NULL + {the offset of sk}. While dereferencing NULL is explicitly not permitted, pointer arithmetic on NULL is permitted, and dereferencing any non-NULL memory address is permitted.

This is obvious from an implementation perspective. If pointer arithmetic on the NULL address were not permitted then every single pointer arithmetic expression would require a check at every intermediate stage to make sure that it did not pass through NULL. For example (a - a + a + 1) would be an invalid pointer arithmetic expression on platforms where NULL is address 0 if address computations were not permitted on NULL.

[1] GCC and C++1x both declare a new language keyword for an invalid pointer, but this is not part of standard C.

Re:Wait, what?

[johnw]

First, NULL is a preprocessor construct, not a language construct; by the time it gets to the compiler the preprocessor has replaced it with a magic constant[1].

Which must be either "0" or "(void *) 0".

The standard requires that it be defined as some value that may not be dereferenced, which is typically 0 (but doesn't have to be

Not true - the standard requires NULL to be defined as one of the two values given above.

and isn't on some mainframes

There are indeed some platforms where a null pointer is not an all-bits-zero value, but this is achieved by compiler magic behind the scenes. It is still created by assigning the constant value 0 to a pointer, and can be checked for by comparing a pointer with a constant 0.

Re:Wait, what?

[vslashg]

&(((struct foo*)(void*)0)->bar) will also give the value of the offset of the bar field.

You're speaking with a voice of authority, which is dangerous because of how incorrect in general your post is.

Others have already pointed out that you are wrong about NULL. Here's precisely what the spec says about the argument to &:

The operand of the unary & operator shall be either a function designator, the result of a
[] or unary * operator, or an lvalue that designates an object that is not a bit-field and is
not declared with the register storage-class specifier.

(((struct foo*)(void*)0)->bar) in particular is none of those things, and your expression is not legal C.

Some apparent dereferences of null pointers are allowed. For instance:

void *a = 0;
void *b = &(*a);

The above is legal not because dereferencing a null pointer is legal, but rather because of an explicit exception to the rule carved out in section 6.5.3.2 of the spec, which says that in this case, the & and * cancel, and "the result is as if both were omitted".

Your expression is neither safe nor portable. If you do need to check the offset of a field in a structure, use the standard library offsetof() macro -- that's what it's for.

Re:Wait, what?

[Athanasius]

This ought to be crashing at the sk = tun->sk line, because the structure is smaller than a page, and page 0 is mapped no-access (I assume Linux does this; it's been standard practice in most operating systems for a couple of decades to protect against NULL-pointer dereferencing).

If you actually read the exploit code (see: http://grsecurity.net/~spender/cheddar_bay.tgz) the thing that really enables this exploit is one of two ways to map page zero. One of these seems to be a flaw with SELinux (either with the default settings and/or how the default config commonly ships) or using personality(2) to select a personality that explicitly allows this.

From the exploit for the personality case:

int main(void)
{
        int ret;
        struct stat fstat;
 
        ret = personality(PER_SVR4);
 
        if (ret == -1) {
                fprintf(stderr, "Unable to set personality!\n");
                return 0;
        }

Note you do need some setuid root program even with this (from my reading of the exploit code).

In the SELinux case "it just works" without needing the setuid program it seems.

Re:Wait, what?

[MSG]

Which must be either "0" or "(void *) 0". ...
There are indeed some platforms where a null pointer is not an all-bits-zero value, but this is achieved by compiler magic behind the scenes. It is still created by assigning the constant value 0 to a pointer, and can be checked for by comparing a pointer with a constant 0.

What you've said is technically true, but doesn't contradict or clarify the post to which you replied in any way, so I'm not sure what your point is.

As you point out, a NULL pointer is a pointer which is represented by "(void *) 0" in the C language. However, where you may be confused is that "(void *) 0 != (int) 0". At least, not always. The compiler is responsible for determining if any "0" is used in a pointer context and casting it to the appropriate value, which may not be the same as numeric "0". So, while it's always possible to check for a NULL pointer by comparing a pointer to 0 in code, the machine may use a different value for NULL pointers. When you check "if(p)", the binary code that is produced will be comparing the value of "p" to the NULL address which is appropriate for the machine on which it is running.

The C FAQ has more information.

Re:Wait, what?

[Anonymous Coward]

In this case, it is tun->sk, not &(tun->sk) which is being loaded, however the pointer arithmetic which generates the address happens first. If tun is NULL then this is NULL + {the offset of sk}. While dereferencing NULL is explicitly not permitted, pointer arithmetic on NULL is permitted, and dereferencing any non-NULL memory address is permitted.

Raven, I've seen you make the same comment a few times in this story. Please stop pushing this nonsense.

The language standard calls * and -> operations "dereferencing". The way it works is that tun->sk dereferences the whole struct, then hands you the sk field from it.

When you implement this in your compiler you do an address computation first then load only the field because you don't want to load the whole struct when you don't need to, but that's an implementation detail. The compiler is required to act as if the pointer tun were being dereferenced.

It would be a major missed optimization bug if the compiler didn't eliminate the later if (!tun) operation. This is a case where the input code is simply wrong.

CFLAGS

[Epsillon]

CFLAGS+= -fno-delete-null-pointer-checks

Job done (should work with Gentoo, buggered if I know how to do this in other distros, DYOR), even with -O2/-O3. This is an optimisation/code conflict. The code itself is perfectly valid, so if your CFLAGS are -O -pipe you have nothing to worry about. GCC's info pages show what is enabled at various optimisation levels. -fdelete-null-pointer-checks is enabled at -O2. Of course, this only applies when you compile your own kernel. If vendors are supplying kernels compiled with -O2 without checking what it does to the code then it is obvious who is to blame.

Re:CFLAGS

[Tony Hoyle]

No. That doesn't fix the problem. All it does is stop the broken optimisation (why the *hell* did someone at gcc think such a thing should be default anyway?)

You need an -ferror-on-bogus-null-pointer-checks parameter so that the code can be fixed.

It's an easy error to make. It's the compilers job to warn you.. in this case not only did it fail to throw a warning it also made the problem worse by 'optimising' it.

Re:Double standards

[infolation]

This language is called Pedantry. A pedant pedantically peddles english into pedanticism.

Re:Serious bug in gcc?

[Tony Hoyle]

gcc is definitely doing the wrong thing here.

Given the code:
a = foo->bar
if(foo) something()

gcc is doing precisely the wrong thing - optimising out the if on the theory that the app would have crashed if it was null.

What it *should* do is throw a warning (even an error, given the clear intent of the code) pointing out that the variable is dereferensed before it is tested.

This kind of error being missed by gcc is going to affect a *lot* of code - it's really not that uncommon a coding error, and is easy to do.

Re:I always disable those

[140Mandak262Jamuna]

They create vulnerabilities by allowing remote code to overload error handlers and thus pwn your system?

Actually, it's already been fixed

[inode_buddha]

Actually, it's already been fixed as of 2.6.31-rc3. Interestingly enough, the code by itself was fine until gcc tries to re-assign the pointer value upon compiling. Steven J. Vaughn-Nichols had a decent write-up about it in Computerworld.

Re:Actually, it's already been fixed

[inode_buddha]

Submissions and patches to the kernel are independently tested and verified at least twice before being signed off and committed, usually by upstream developers (more experienced). This is the normal process. The only thing different in this case is that a vulnerability was exposed, hence it is in the news.

Re:Just like Linux

[Tony Hoyle]

Unless they're going to add a proper warning for the condition to gcc 'today' it won't, really.

Sure there are enough developers to go over the kernel to make sure such errors haven't been missed elsewhere, but all it takes is one to miss it and it's still there. Then there's all the other software compiled by gcc..

I'm not entirely sure how it can lead to an exploit (short of remapping page zero, which requires root privileges so doesn't really count) but since it has it's going to need a proper fix.

Re:Serious bug in gcc?

[Bananenrepublik]

They were writing nonsense. GCC makes use of the fact that in the C language any pointer that was dereferenced can't be NULL (this is made explicit in the standard). People use C as a high-level assembly where these assumptions don't hold. This is why code that doesn't assume this breaks. This issue came up a few months ago on the GCC lists, where an embedded developer pointed out that he regularly maps memory to the address 0x0, thereby running into issues with this assumption in the optimizers. The GCC developers introduced a command-line flag which tells the computer to not make that assumption, therefore allowing the compiler to be used even in environments where NULL pointers can be valid.

Now, the exploit uses this feature of the compiler (or the C language, if you will) to get the kernel into an unspecified state (which is then exploited) -- the NULL pointer check will be "correctly" optimized away. But in order to do this it first has to make sure that the pointer dereference preceding the NULL pointer check doesn't trap. This needs some mucking around with SELinux, namely one has to map memory to 0x0.

This is a beautiful exploit, which nicely demonstrates how complex interplay between parts can show unforeseen consequences. Linux fixes this by using the aforementioned new compiler option to not have the NULL pointer check optimized away.

Re:Double standards

[mortonda]

This is arguably more of an issue in the compiler than in the kernel,

Not completely... from the SANS Storm Center, the code was as follows:

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

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

The error was that the compiler optimized away the if statement, assuming that tun had already been initialized. The check should have been placed before the sock variable referenced it. Not entirely obvious maybe, but then again, it should have been checked before the assignment.

Re:Double standards

[gilgongo]

For such a piece of shit company, they sure do have a lot more marketshare than the computing godOS known as Linux.

Microsoft's current market share has nothing to do with quality, and everything to do with monopoly. It doesn't matter whether their product is any good or not, because not only do the vast majority of computer users not even know what Windows is, they wouldn't have the first clue what an alternative to Windows or MS Office would be like.

Time to learn about basic economic theory I think.

Re:Double standards

[alnjmshntr]

Right... Because Microsoft are really losing sleep over the negative comments posted on slashdot, so they have assembled a crack team of slashdotters to game the moderation system in their favour.

You have to be kidding me.

Re:Serious bug in gcc?

[TheRaven64]

Except that his explanation is wrong. Dereferencing NULL is illegal, but pointer arithmetic on NULL is legal (and, even if it were illegal, would be practically impossible for a compiler to check). This statement is not dereferencing NULL, it is (potentially) performing pointer arithmetic on NULL (adding the offset of the sk field to it) and then dereferencing the result. This may or may not be valid, depending on how NULL is defined (it doesn't have to be 0 in C, although it usually is), what the offset of sk is, and what the memory layout of the target platform is.

On most modern platforms, NULL is defined as (void*)0 and the entire bottom page of memory is mapped as no-access. On some embedded systems, however, the bottom few hundred bytes are used for I/O and you get the addresses of these by adding a value to 0. On these systems it is perfectly valid (and correct) C to define a structure which has the layout of the attached devices and then cast 0 to a pointer to this structure and use that for I/O.

Re:Double standards

[Shinobi]

I never said anything about writing everything in it. But many of us with proficiency in it tend to check what the compiler actually outputs, because we know that the compiler is not smarter than the human who wrote it is. (A behaviour further reinforced by the two smelly piles of fecal matter that are MSVC and GCC). This is also why many of us don't blindly trust optimizations to the compiler either, and always double-check. A disassembler is also useful for tearing through critical pieces of code to see if the compiler has built it in the way you intended.

I've removed quite a few obscure but potentially very nasty bugs in my software by doing that. Then again, I'm a freelancer, I live by my reputation for solid, fault-free code.

Re:Double standards

[Shinobi]

Sure. My last entire project has been specifically about that. Been working on a piece of software to go onto an embedded device with deterministic behaviour, with the hardware specs being 32kiBiByte RAM, no cache, 8MHz processor.

Most people I am forced to work with who have a comp sci degree are unable to work under such conditions. On the other hand, EE's and comp.eng graduates tend to be very nice to work with on such projects.

Re:Serious bug in gcc?

[johnw]

That is: It don't have to be all 0 bits. It just need to be distinct from any valid pointer,

Correct - apart from the "just" bit.

It doesn't need to be all 0 bits.
It does need to be distinct from any valid pointer.

*and*

void *p = 0;

must generate a null pointer, and:

p == 0

must come out true if p is a null pointer. The internal implementation need not be all zeroes, but it does need to look rather like it to source code.

I really don't see how this is a compiler problem?

[gbutler69]

To me, the "if (!tun)" check should/must be before the de-reference; otherwise, it is meaningless! However, the compiler should print a warning in this case, not just optimize it away.

Re:Linus, you Rookie !!

[luca]

Ok, I know I shouldn't be feeding the troll, but read the article: the kernel source itself is perfectly fine, is the compiler that optimizes the check away.

no exact code snippet found in Linux

[pikine]

I tried to google code search for "tun->sk" and Linux doesn't contain that snippet of code. Since SANS claimed that drivers/net/tun.c is at fault, I looked at that source file and didn't find any instances where "if (!...) return ...;" is performed after NULL dereference.

I think the only fascinating bit of the story is that the SElinux extension allows you to map a page at memory address 0 (the NULL page), making NULL dereferencing valid. I also found out about that a while ago, but I didn't know it has anything to do with SElinux. By the way, mapping the NULL page also works on Mac OS X.

However, mapping NULL page is typically NOT exploitable. A correct program will simply reject access to NULL pointer, giving it a special semantic regardless whether the memory page itself is valid or not.

Re:Double standards

[QuoteMstr]

No. You are wrong.

The code is grabbing the value of the sk field of the tun struct, not its address. Did you misread the code, or do you not actually know C? Or are you perhaps just on the sauce?

You're claiming the code reads struct sock **sk = &tun->sk when in reality, it reads struct sock* sk = tun->sk, which is completely different.

Re:Serious bug in gcc?

[QuoteMstr]

Of course NULL is part of the C language, you blathering idiot, and it always has been. The level of ignorance here astounds me. Don't post about things you don't understand.

Quoting from C89: (not C99, C89, the one that's older than dirt.)

4.1.5 Common definitions The following types and macros are defined in the standard header . Some are also defined in other headers, as noted in their respective sections.... NULL which expands to an implementation-defined null pointer constant ... A.6.3.13 Library functions * The null pointer constant to which the macro NULL expands ($4.1.5).

NULL wasn't even "added" in C89: NULL appears in the oldest, cruftiest UNIX code you can imagine. (That link is the original cat command from 1979.)

Interesting

[improfane]

Guys, I'm trying to decide what to post:

[ ] Downplay how serious flaw is
[ ] Compare to Window's track record
[x] Make a meta-reference to Slashdot psychology
[ ] Post work-around that doesn't fix problem
[ ] Say that flaw is a feature
[ ] bash Windows
  [ ] Claim that not all Windows software is bad
[ ] Claim that the more popular gets, Linux will be targeted more
[ ] Pretend I understand the problem ...or we could RFA

Re:Serious bug in gcc?

[marcansoft]

Sure it does - GCC knows at compile time that if the if() condition were true, we're already in the "undefined behavior" realm and all bets are off. So it gets rid of it. The code is broken: it's not the compiler's job to compile for the maximum defensiveness of the resulting machine code, otherwise we'd all be using bounds-checking compilers. If the compiler realizes that a certain runtime value will lead to undefined results (because the programmer chose to do so), it is free to break the execution as much as it wants in that case for code that runs afterwards. Essentially, undefined behavior is a contract signed by the programmer that says "I certify that this will never happen", which is why the compiler chose to perform this optimization.

Even though the real bug is clearly in the code, moving on to the realm of what's desirable from a compiler, I think it's clear that this behavior can make some problems worse (to the compiler, problems are binary - if there's a problem all bets are off - but not to us). This is fine in the name of optimization, but I think in this particular instance either a) kernel developers should opt to turn this optimization off, or b) (better) make GCC warn when this kind of optimization happens, because it's quite likely a bug.

In effect, the code is a form of broken defensive programming (you check after the fact whether you've screwed up). It's wrong, but we still wouldn't want the compiler to silently remove the check. So I think the ideal solution (besides fixing the code) is to add a warning to the compiler. NULL pointer dereferences are a bug in the vast majority of cases, and checking for a NULL pointer after dereferencing it (in such a way that the compiler recognizes it and is about to remove the check) is at best redundant and more likely a bug.

There's still the issue of the page 0 fuckery. If someone can make page 0 accesses not crash the kernel then that's also a bug - there are good reason why we want NULL and neal-NULL pointer accesses to always crash.

Re:Double standards

[dna_(c)(tm)(r)]

Oh please, it's a response to

If this had been Windows, the article would have been tagged defectivebydesign.

You're not supposed to read the article, but at least the post you're criticizing.

Re:Double standards

[kestasjk]

So you can disassemble compiled code, way to go.. Have fun disassembling a huge binary that's far too large to economically analyze in assembly.

What's that? You don't fully disassemble and analyze large binaries but only critical paths or small binaries? How unique and sought-after your services must be. I'm sure analysis of compiled kernels is the best way to tackle this bug..

code found in Linux 2.6.30

[pikine]

Oh, found the code on lxr. It looks like Linux kernels up to 2.6.29.6 are NOT affected, and this is a vulnerability introduced in 2.6.30 due to a fairly significant rewrite of tun.c. Linux 2.6.30 was released in Jun 9, 2009, just a month ago. Funny the tun.c rewrite was not mentioned in the set of changes for 2.6.30.

I think this example actually shows a forte of Linux as open source. New vulnerability is found very quickly after "new" code is released.

the set of changes for 2.6.30

[pikine]

For some reason I didn't link this correctly. The set of changes for 2.6.30 is found http://kernelnewbies.org/Linux_2_6_30.

Re:Linus, you Rookie !!

[gnasher719]

Ok, I know I shouldn't be feeding the troll, but read the article: the kernel source itself is perfectly fine, is the compiler that optimizes the check away.

Absolutely not. The code itself has a severe bug: If tun is a null pointer then it invokes undefined behaviour. Undefined behaviour means anything can happen. Anything can happen means a severe bug, especially in kernel code. The optimizing compiler just turned C source code that was buggy, but not obviously enough for the programmer, into assembler code that would have been obviously buggy to anyone. Most definitely not the fault of the compiler.

Re:Double standards

[NormalVisual]

And frankly, it's not particularly difficult to work under those constraints. There are a lot of us that were writing code 25-30 years ago, when that level of hardware represented a state-of-the-art microcomputer, except for the 8 MHz CPU - most CPUs then ran only a fraction of that speed. I still marvel at the fact that the tiny BlackBerry that I hold in my hand is *in every way* a faster and more capable computer than what I worked on back in the Dark Ages.

Assembly of any sort isn't that difficult once you get some experience with it, and with the proper macros and defines set up, it can actually be fairly quick to code in. Some chips are easier than others (the 68K was *awesome* to code for), but it just requires some attention to detail and a good understanding of how the machine works.

Re:Double standards

[BitZtream]

Yes, but the rest of us have written about 1000 times more code than you because we didn't spend our time checking a ton of assembly because we presume the compiler is flawed.

There are times when this sort of checking is acceptable if not required. The kernel is a good place to do it.

You aren't going to do this for KDE or Gnome however.

Re:Serious bug in gcc?

[Rockoon]

In effect, the code is a form of broken defensive programming (you check after the fact whether you've screwed up). It's wrong, but we still wouldn't want the compiler to silently remove the check. So I think the ideal solution (besides fixing the code) is to add a warning to the compiler. NULL pointer dereferences are a bug in the vast majority of cases, and checking for a NULL pointer after dereferencing it (in such a way that the compiler recognizes it and is about to remove the check) is at best redundant and more likely a bug.

My problem with this sort of thinking is when you throw in macros and templates and whatnot, there can end up being hundreds, thousands, even millions of "redundant" tests againt NULL specified by the expanded source. Now, I suspect that simply adding this warning to GCC and then compiling some large project would generate so many such warnings that the only reasonable choice would be to then disable that warning. The warning would then have no value, and if so then that certainly doesnt address the "problem."

As far as the other stuff.. my point was that the arguement that the compiler should never optimize away such if() statements is flawed. I was responding to someone who did in fact make such a claim. There are certainly cases where the pointer absolutely cannot be NULL (or absolutely must be) .. ex, the pointer was just assigned, or its nested within another test for null.

Re:Just like Linux

[Jimithing DMB]

Funny enough a few months back I made a very similar error if not the exact same error while coding on the bootloader for Darwin/x86. Except in my case it wasn't exactly a true error because in the bootloader I know that a page zero dereference isn't going to fault the machine but will instead just read something out of the IVT.

So as I recall it seemed perfectly reasonable to go ahead and initialize a local variable with the contents of something in the zero page and then check for null and end the function. But GCC had other ideas. It assumed that because I had dereferenced the pointer a few lines above that the pointer must not be NULL so it just stripped my NULL check out completely. Had it warned about this like "warning: pointer is dereferenced before checking for NULL, removing NULL check" then that would have been great. But there was no warning so I wound up sitting in GDB (via VMware debug stub) stepping through the code then looking at the disassembly until I realized that.. oops.. the compiler assumed that this code would never be reached because in user-land it would have segfaulted 4 lines ago if the pointer was indeed NULL.

Obviously the fix is simple. Declare the variable but don't initialize it at that time. Do the null check and return if null. Then initialize the variable. If using C99 or C++ then you can actually defer the local variable declaration until after you've done the NULL check which IMO is preferable. It may be that the guy wrote it as C99 (where you can do this) then went oops, the compiler won't accept that in older C and simply moved the declaration and initialization statement up to the top of the function instead of splitting the declaration from the initialization. My recollection of how I managed to introduce this bug myself is shady but as I recall it was something like that.

Re:Double standards

[kdemetter]

i compiled my kernel using that flag , and now it boots Windows instead.

Re:Linus, you Rookie !!

[lilo_booter]

Umm - no - the *code* does the undefined behaviour and *then* checks if the undefined behaviour could happen. But, heck, mistakes happen - it was identified and fixed. Not much of a story really.