|
![](/i/fill.gif) |
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
|
![](/i/fill.gif) |