Hacker Newsnew | past | comments | ask | show | jobs | submitlogin
This is the exploit equivalent of that guy who played the perfect game (2008) (matasano.com)
71 points by req2 on July 6, 2009 | hide | past | favorite | 33 comments


Someone makes an argument against checking malloc, talking about the overhead it adds.

Does anyone have a problem with using a malloc wrapper?

  void *Malloc(size_t size) {
    void *r = malloc(size);

    if(r == NULL)
      /* do things to quit the app */

    return r;
  }
edit: yay for " " code blocks. Hacker News++

edit again: I wasn't clear. I meant overhead in terms of lines of code it added. I'm asking if it makes sense to wrap it up in a nice upper-case call that just terminates the program if the worst happens.


People who claim that they can't afford the overhead of error checking their instructions are high. There are no realistic situation in which getting the wrong answer fast is better than the right answer one clock cycle slower, especially with the heavily pipelined architectures we have today.


If you think extra branches don't affect the performance of heavily pipelined architectures, I have some Pentium 4s I'd like to sell you. :)

Right idea, wrong proof.


My understanding is that this penalty can be mitigated by telling the compiler which way a branch is likely to go so it can optimize the generated code appropriately. I know the linux kernel uses what I think is a gcc specific extension to do this. For a malloc check, the result is almost always the same.


I'm not sure what your system's malloc looks like, but I assure you it's probably full of branches. One more isn't going to make a difference. If you are worried about branches causing pipeline stalls you aren't allocating memory.


I was responding to the part at the end where it was implied that heavily pipelined architectures aren't subject to stalls (what else does especially in italics mean?), not the idea that checking for NULL is a performance hit.


Ah I totally misread your comment. Sorry about that. Branches can definitely kill performance in heavily pipelined archs.


Pardon my (likely striking) ignorance of hardware, but I remember a discussion from way back in college about pipelines and branches. The gist of it was, you may as well guess at branches, since if you're wrong (and careful), you don't waste any more time than if you hadn't guessed. The discussion went from there into what constituted effective methods for guessing, and I remember a comment to the effect that "do what you did last time" was one of the best strategies.

I was left with the impression that hardware did this, though it's possible I misunderstood. Google seems to back me up a bit, but I admit I'm out of my depth on this topic.

Anyway, it seems to me that if my recollection were accurate, a single branch which went the same way virtually every time could have a lower cost in a heavily pipelined system than a more distrubted error checking scheme.


Anyway, it seems to me that if my recollection were accurate, a single branch which went the same way virtually every time could have a lower cost in a heavily pipelined system than a more distrubted error checking scheme.

That, in a nutshell, was what I was claiming.


No, you didn't misunderstand it and it is called branch prediction and it is done in hardware. I'm surprised no one has brought up this point.


Extra branches are only a problem if the chip guesses wrong about the direction and has to undo a lot of speculative execution. So the default, fast case should be the one where memory is successfully allocated, in which case your total penalty is one compare to null instruction.


The CPU is so much faster than memory that the time needed for checking if malloc succeeded is basically unmeasurable.

I assume that after you malloc that you'll put something in it. Copying that data to memory takes so much longer than checking if the return value == NULL that it's a joke to care.

BTW The info was interesting, but I don't care for the style of writing.


Checking your return values is pretty essential stuff. The error in this case was not checking that malloc could return null and then not to abort.

So, if you call malloc and do not check the return value (and abort if it is NULL) then you end up using the 'null' pointer. Normally this will cause your application to crash since a null pointer can not be dereferenced. But if the null pointer is used with an offset and that offset is under the control of the attacker then suddenly you can access pretty much all of memory. (of course, if the pointer would have been valid and you can combine it with arbitrary input you can STILL access all of memory but it is a lot harder to figure out where you are pointing, a null pointer literally contains '0' so all you have to do is figure out where you want to be, not where the pointer is pointing, you already know that).

The overhead of checking what malloc returns you is trivial compared to what malloc does internally so I wouldn't worry about that at all.


From my understanding, his problem was that the malloc wrapper being used not only checked but also returned -1 if it was null (I assume some other arbitrary number otherwise) which would add extra computation time.

Out of curiosity, I have always used assert to do malloc null checks. I realize it is just a macro which gives a conditional to exit but is at all hindering in terms of performance?

eg:

  void *Malloc(size_t size) {

    void *r = malloc(size);
    assert(r != null);

    return r;
  }


That's probably the same as the non-assert check EXCEPT that release builds might disable assertions and then you're in trouble.


Right. Plus as a matter of style, assertions are traditionally distinguished from error handling. Assertions are used to catch programmer errors; error handling is for when something happens that is nevertheless within the contract of the library (malloc can return NULL, fopen can return NULL, etc).


the problem is that assert is defined so that if NDEBUG is defined, it's a no-op.

I'd use an explicit check and an abort() call.


I took 15-213 at CMU. The moment I saw that code block I knew that's where you came from. One of the best clases I've taken there.

Good old csapp.c http://www.ece.cmu.edu/~ece845/sp02/csapp.c


We actually have very mixed feelings about csapp.c. Students don't seem to understand that you should quit the whole program just because one thread fails. Especially in servers (like proxy lab :))

Still, an awesome class!


"shouldn't", you mean?


yeah, my bad :)


It's my understanding that this won't work exactly as you'd expect under Linux. If you try to allocate more memory than is available, Linux can return a non-NULL pointer and hope that you don't try to use it:

http://alanreviewblog.blogspot.com/2009/02/turning-off-over-...


In the case of a browser plugin, I'd be quite annoyed if an out of memory condition in the plugin caused my whole browser to quit. I'd experience something beyond annoyance if printing caused my word processor to quit without saving. The never fail malloc wrapper makes sense for some programs, but not for others where better, context sensitive recovery is possible.


In the context of a browser plugin, "quit" could just mean to stop executing and throw up an error message. It doesn't necessarily mean to end the whole process (browser).


Throwing up an error message is likely to require allocating some of the memory you just ran out of. There's very little you can reasonably expect to do when you run out of memory. The caller needs to carefully back out, but this isn't something a wrapper can do by itself.


Strings are still statically allocated (courtesy of C compilers everywhere), right? and I guess if you have to make one function call to throw an error message, there's some room left in the stack.. and then there's the whole virtual memory thing..

but I could be reasonably wrong. :-)


Seems the best solution is to just throw an exception . . .

. . . er, crap. This is C, isn't it?


C can throw exceptions. the call is 'raise'.


Two things:

1) It's a memory allocation, scanning data structures, possibly involving some thread-locking or even a system call. A single check is cheap. If you want to keep the CPU happy the 99% of the time, put the most likely case first, so the CPU doesn't have to jump. Speculation should help mitigate the rest.

2) A good reason why operator new() throws by default.


Well, if you want to avoid the call overhead do it with an 'inline', that way there is no overhead at all.


This is a dupe and over a year old

http://news.ycombinator.com/item?id=164725


Apologies. I'm new to using searchyc and didn't expect it to miss the URL.

http://searchyc.com/submissions/http%253A%252F%252Fwww.matas...


Am I on Reddit? Seriously, what's with the headline?




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: