 |
|
|
|
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Doctor John wrote:
> At a guess that would be:
> http://www.theregister.co.uk/2009/07/17/linux_kernel_exploit/
Yes, but since that wasn't the point of my question and people might
reasonably assume I'm bashing on Linux if I posted a link to it, I thought
I'd omit it. The question was why the compiler felt the need for *that*
optimization in the first place. When would it ever be a good idea?
--
Darren New, San Diego CA, USA (PST)
"We'd like you to back-port all the changes in 2.0
back to version 1.0."
"We've done that already. We call it 2.0."
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Warp wrote:
> Darren New <dne### [at] san rrcom> wrote:
>> Someone recently found a bug whereby a piece of C code read
>
>> xyz->pdq = some value;
>> ....
>> if (!xyz) return (error code);
>> ...
>> do_something_dangerous();
>
>
>> Someone mapped a page into 0, invoked the bit of the kernel that had the
>> code, and did something dangerous in spite of xyz being null. It turns out
>> GCC looks at the test for NULL in the second line, decides that since it has
>> already been dereferenced in the first line it can't be NULL (or a segfault
>> would have thrown), and eliminates that test.
>
> Why couldn't the compiler assume that if nothing modifies xyz between
> the assignment and the test, that it doesn't get modified? It seems to me
> like a rather valid assumption.
There's no assumption xyz didn't get modified. The assumption is that since
you referenced it in the first line, the second line will never fail because
the first line would have dumped core. I.e., if the first line had been
int pdq = *xyz;
if (!xyz) do_something();
then the compiler would omit the code to invoke do_something(), as well as
the test. No assignment to xyz, but just a reference.
I'm trying to figure out what possible benefit such an optimization could
have. I.e., this code is obviously *wrong* - why would you bother to include
optimizations to improve the performance of erroneous code?
> (Btw, AFAIK the C standard specifially says that a null pointer cannot
> contain a valid value. Thus the compiler can assume that it can't contain
> a valid value.)
Yep.
--
Darren New, San Diego CA, USA (PST)
"We'd like you to back-port all the changes in 2.0
back to version 1.0."
"We've done that already. We call it 2.0."
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
> (Btw, AFAIK the C standard specifially says that a null pointer cannot
> contain a valid value. Thus the compiler can assume that it can't contain
> a valid value.)
The problem is that in C a null pointer is represented
by 0, but 0 is a valid memory address. So when you
have a valid pointer to address 0, then the optimizer
thinks you are checking for null, not for address 0.
It does seem bad to me to have a so much bloat
in the control switches for the GCC optimizer.
Take a look...
http://gcc.gnu.org/onlinedocs/gcc-4.4.0/gcc/Optimize-Options.html#Optimize-Options
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Tim Attwood wrote:
> The problem is that in C a null pointer is represented
> by 0,
Depends on the architecture, really, but in most cases, yes.
> but 0 is a valid memory address. So when you
> have a valid pointer to address 0, then the optimizer
> thinks you are checking for null, not for address 0.
The exploit was a bug in the kernel that dereferenced a pointer before
checking for null, and the compiler silently optimized out the later check
for null. If you can get the first dereference to work (by mapping some
valid memory to the address associated with the null pointer value) then you
skip over code people thought they wrote into their program and which the
compiler removed.
> It does seem bad to me to have a so much bloat
> in the control switches for the GCC optimizer.
Tell me about it. Just wait till you have the fun of cross-compiling the
compiler. :-)
--
Darren New, San Diego CA, USA (PST)
"We'd like you to back-port all the changes in 2.0
back to version 1.0."
"We've done that already. We call it 2.0."
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Let's say I wrote a macro that does something that happens to be useful to
me, along these lines:
#define DO_USEFUL_STUFF( OBJECT ) if ( OBJECT ) OBJECT->usefulMethod();
Now, I go and use this macro in different places. In some places, the object
passed is likely to be NULL. In others, it's not. I would be glad that the
compiler is optimizing out the unnecessary checks for me, while still
letting me benefit from the general usefulness of my macro.
Of course this is a contrived example, but it's probably not too far from a
real use case.
- Slime
[ http://www.slimeland.com/ ]
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Tim Attwood <tim### [at] anti-spamcomcastnet> wrote:
> > (Btw, AFAIK the C standard specifially says that a null pointer cannot
> > contain a valid value. Thus the compiler can assume that it can't contain
> > a valid value.)
> The problem is that in C a null pointer is represented
> by 0, but 0 is a valid memory address. So when you
> have a valid pointer to address 0, then the optimizer
> thinks you are checking for null, not for address 0.
Well, dereferencing a null pointer is undefined behavior, so from the point
of view of the standard the compiler can do whatever it wants. Thus gcc is
not doing anything wrong here (because after a null pointer dereference there
is no "wrong" to be done).
If someone is to be blamed is the designers of the kernel if they decided
that a null pointer points to a valid address, against the C standard, and
then implemented the kernel in C (using a standard-conforming C compiler).
--
- Warp
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Darren New <dne### [at] sanrrcom> wrote:
> The exploit was a bug in the kernel that dereferenced a pointer before
> checking for null, and the compiler silently optimized out the later check
> for null. If you can get the first dereference to work (by mapping some
> valid memory to the address associated with the null pointer value) then you
> skip over code people thought they wrote into their program and which the
> compiler removed.
I'd rather say it was a flaw in the kernel, forgetting to already check for null
*before* dereferencing.
In such a "mission-critical" piece of software such as a kernel, *all* that
comes in from untrusted sources should *always* be checked thoroughly before
doing *anything* with it. "Oh, if this pointer happens to be NULL, the software
will core dump anyway" is a *very* questionable mode of operation.
Code dereferencing a possibly-NULL pointer *is* bogus - it's nothing you can
simply blame on the compiler. And mission-critical code should not *never* be
bogus.
There's even some reason to argue that if the programmer happily dereferences a
pointer without checking it for NULL, why shouldn't the compiler assume that
other provisions have been made that it cannot be NULL in the first place?
I'm also surprised that this flaw wasn't found and fixed earlier. There's tools
like PC-lint (taken as an example here because I'm a bit familiar with it) that
do a pretty good job at complaining when you're trying to dereference a
potentially-NULL pointer. I mean, you'd expect that *someone* runs *something*
like PC-lint on code as widely-used as a Linux kernel?!
Well, maybe that's a problem with free-software projects: PC-lint costs money.
Companies developing commercial software can afford to buy a few licenses. The
individual parties in a free-software project possibly can't, or refuse to pay
anything for their tools, or refuse to be the only ones paying for something
that benefits all.
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
clipka <nomail@nomail> wrote:
> Code dereferencing a possibly-NULL pointer *is* bogus - it's nothing you can
> simply blame on the compiler. And mission-critical code should not *never* be
> bogus.
I didn't get the impression that they were dereferencing a null pointer
deliberately in order to get an exception, or that it was a mistake. (The
only "mistake" was not realizing what kind of optimization gcc would do.)
--
- Warp
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
clipka wrote:
> I'd rather say it was a flaw in the kernel, forgetting to already check for null
> *before* dereferencing.
Yes, exactly. But I'm trying to figure out why you would add optimizations
specifically to improve the performance of code you are optimizing *because*
you know it is flawed.
Note that this isn't just a general dead-code-elimination. This is
specifically checking for "test for null after already dereferencing", as
there's an explicit compiler chip to turn that off.
> In such a "mission-critical" piece of software such as a kernel, *all* that
> comes in from untrusted sources should *always* be checked thoroughly before
> doing *anything* with it.
Right. I think the comments on the report of it that I read implied that
the assignment was added later, and the problem is that in C, the
declarations all had to be at the top.
So someone wrote
blah * xyz;
if (!xyz) return failure;
... use xyz ...
Someone else came along and added a declaration
blah * xyz;
another pdq = xyz->something;
if (!xyz) ....
Instead, they should have said
blah * xyz;
another pdq;
if (!xyz) ...
pdq = xyz->something;
> "Oh, if this pointer happens to be NULL, the software
> will core dump anyway" is a *very* questionable mode of operation.
Exactly.
> Code dereferencing a possibly-NULL pointer *is* bogus - it's nothing you can
> simply blame on the compiler. And mission-critical code should not *never* be
> bogus.
Agreed. The problem is that the compiler optimized out good code based on
bogus code. My only question is whether I'm missing something here, because
such an optimization seems really a bad idea.
> There's even some reason to argue that if the programmer happily dereferences a
> pointer without checking it for NULL, why shouldn't the compiler assume that
> other provisions have been made that it cannot be NULL in the first place?
That's exactly what the compiler did. It looked, saw the dereference of the
pointer, then the check that the pointer isn't null, and optimized out the
check for the pointer being null. But the fact that the programmer said to
check for NULL at a point where all code paths have already dereferenced the
pointer would seem to be at least a warning, not an "oh good, here's an
optimization I can apply."
The only reason I can think of for that would be code generated from some
other language, at which point I'd think you'd want a compiler flag to turn
*on* that optimization, with it being off by default.
> I'm also surprised that this flaw wasn't found and fixed earlier.
I'm surprised and dismayed when the kernel gets code checked in that's
syntactically invalid, then released, then when people complain about it
they say "Oh, I didn't know how that code builds" and someone else says "try
changing the ownership of the source file to be root", and then a few weeks
later they have a patch on some website somewhere unassociated with the
actual original project that makes it compile.
I'm beyond being surprised at how bad the free stuff can be if you're
actually a programmer. </rant>
--
Darren New, San Diego CA, USA (PST)
"We'd like you to back-port all the changes in 2.0
back to version 1.0."
"We've done that already. We call it 2.0."
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Slime wrote:
> Of course this is a contrived example, but it's probably not too far from a
> real use case.
Ah, that makes sense. Thank you for clarifying that.
It *is* code generation. Just from the same language you're compiling. That
didn't occur to me. :-)
--
Darren New, San Diego CA, USA (PST)
"We'd like you to back-port all the changes in 2.0
back to version 1.0."
"We've done that already. We call it 2.0."
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
|
|
