| |||||||||||||
|
| |||||||||||||
Nonsense. This is trivial stuff.Nonsense. This is trivial stuff.Posted Jul 17, 2009 20:34 UTC (Fri) by corbet (editor, #1)In reply to: Nonsense. This is trivial stuff. by mikov Parent article: Linux 2.6.30 exploit posted There is an interesting aspect to this that nobody has mentioned: I have seen reviewers advising the removal of NULL pointer checks with the reasoning that, should a NULL pointer ever come about, the resulting kernel oops is just as useful as a "this pointer was null!" message. In the mean time, the overhead of the check can be avoided. The idea, clearly, is that the memory management hardware can be counted on to do the NULL check so there's no need to do it in software too. But if that address can be mapped, that reasoning clearly does not hold. I don't doubt there are other situations like this one in the kernel code; for most systems, disallowing mappings at zero seems like a reasonable step to take.
(Log in to post comments)
Removing NULL checks Posted Jul 17, 2009 21:02 UTC (Fri) by corbet (editor, #1) [Link] I had to dig a bit, but here's an example of developers being advised that some NULL checks are not needed. In retrospect, that doesn't seem like the best advice...
Removing NULL checks Posted Jul 17, 2009 21:06 UTC (Fri) by stevenb (subscriber, #11536) [Link]
Anyway, the "it appears to be a GCC bug in the end." bit in the LWN story about this is unjustified IMHO.
Removing NULL checks Posted Jul 17, 2009 22:39 UTC (Fri) by nix (subscriber, #2304) [Link]
It would be an excessive optimization, if it wasn't that GCC has a way to
turn it off... which the kernel is not using.
Removing NULL checks Posted Jul 17, 2009 22:56 UTC (Fri) by mikov (subscriber, #33179) [Link]
I don't think that disabling this option for a kernel compile is reasonable. It is not reasonable to have to check every new optimization for every new version of a compiler, assuming the optimizations themselves are valid.
What is reasonable on the other hand is to know C and to write valid C code, which doesn't break when a butterfly flaps its wings. I still can't understand why everybody is making such a big deal over such a trivial bug. It is similar to relying on the value of "(a++) + (a++)" - basic C stuff that hopefully everybody knows by now.
If this was an user app nobody would think twice about it. So, what, the kernel is an excuse for sloppy coding?? I would hope that the opposite was true...
Removing NULL checks Posted Jul 17, 2009 23:33 UTC (Fri) by stevenb (subscriber, #11536) [Link]
This optimization is not even new. It was added to GCC ten years ago, see
http://gcc.gnu.org/news/null.html
Removing NULL checks Posted Jul 17, 2009 21:44 UTC (Fri) by mikov (subscriber, #33179) [Link]
I think there are three completely orthogonal issues here, which seems to be confusing people:
1. The kernel is (mostly) written in C and uses a general purpose C compiler (even more than one, if you count Intel). So the kernel should follow the C rules (and the C standard), even if on the surface it seems that these rules don't always make sense in the kernel context. For example, under DOS dereferencing a NULL pointer would not generate any kind of error. That doesn't mean that it is a good practice to remove NULL pointer checks for all reads. It is still undefined behavior according to the C standard and any C compiler for DOS is free to apply the same (correct!) optimization.
I think what people don't realize is that such optimizations are not only valid, but necessary. While there are the few exceptional cases like this one where they seem to get in the way, they are useful in the majority of cases.
2. Whether the _tun_ pointer should ever be expected to be NULL in this point. I have no idea, but if it is supposed to have been checked earlier, it is reasonable not to check again and this is a simple logical bug.
3. Whether the kernel should trap NULL pointer accesses in hardware for kernel code. Again I don't know, but personally I prefer that it did, unless there is good reason not to. Is there one?
Still relying on this can be risky, for example:
The first page may be unmapped, but we are not accessing it.
Removing NULL checks Posted Jul 17, 2009 23:24 UTC (Fri) by nowster (subscriber, #67) [Link] This would never have happened if the kernel were written in some modern language more recent than C. I propose rewriting the kernel in INTERCAL.
Removing NULL checks Posted Jul 19, 2009 11:13 UTC (Sun) by eupator (guest, #44581) [Link]
I suspect you may underestimate the modernism of C or the classicism of INTERCAL.
Removing NULL checks Posted Jul 18, 2009 19:09 UTC (Sat) by darwish07 (guest, #49520) [Link]
You have a nice memory: I remember being advised to strip a BUG_ON line since the kernel OOPS mechanism will handle it - http://lkml.org/lkml/2008/2/13/71
I'm not sure if (or how) can this be exploited though ...
Removing NULL checks Posted Jul 20, 2009 10:03 UTC (Mon) by makomk (guest, #51493) [Link]
Hopefully, that shouldn't be exploitable. I'm pretty sure the kernel
policy is that userspace shouldn't be able to trigger BUG_ONs deliberately, so removing one in favour of just dereferencing the null pointer ought to be safe assuming there isn't a bug elsewhere. (Of course, if there is, it could turn it into a security vulnerability.)
Removing NULL checks Posted Jul 20, 2009 12:46 UTC (Mon) by spender (subscriber, #23067) [Link]
Andrew Morton is wrong, a BUG_ON is in no way equivalent to just dereferencing a null. What if I have my page mapped at NULL?
If that author adds one more call to smack_netlabel and forgets to check for sock->sk == NULL before calling it, instead of just causing an OOPS he's now potentially created a vulnerability which can result in arbitrary code execution.
-Brad
|
|
||||||||||||