11 ms·

A short tale of a read overflow

114 points by stargrave 9y ago

dsign 9y ago
Great article, bonus points for giving in passing the only explanation I have been able to understand on what a fuzzer check is and by extension how unrelated tools like QuickCheck should work.
twic 9y ago
I have a vaguely related story of a baffling segfault bug a few months ago. It was in some reference-counting code deep inside the Boost shared pointer implementation. It wasn't deterministic, and from correlating the crash log with the Boost code, i couldn't see how there could be a dodgy memory access at that point. Poking around with GDB, it seemed fine. Eventually i realised that i had compiled my application against one version of Boost, and then dynamically linked to a library i had compiled against a different version. This was all due to bad library path hygiene - my OS had one version of Boost installed, and i was trying to use another that i had installed locally, but GCC was ignoring my path options and getting seduced by the system Boost. After this, i learned about the --sysroot flag to GCC, which has helped to prevent this happening again. So yeah, protip: know what it is you're actually running. Also, never use C/C++.
- danieldk 9y ago
  It was in some reference-counting code deep inside the Boost shared pointer implementation. [...] Eventually i realised that i had compiled my application against one version of Boost, and then dynamically linked to a library i had compiled against a different version. Interesting. Boost's shared pointer implementation is header-only. Are they using shared_ptr in an API for a non-header Boost library? https://stackoverflow.com/questions/10334511/why-do-c-libraries-and-frameworks-never-use-smart-pointers/10342299#10342299 https://stackoverflow.com/questions/10334511/why-do-c-librar...
  - twic 9y ago
    The library i was dynamically linking to was not a Boost library, it was some random other thing, which used Boost and used shared_ptr liberally in its API.
wyldfire 9y ago
> You can never detect a read overflow otherwise: it will just access data outside your structure, but inside mapped memory, so the bug would be totally harmless and silent, with the exception of doing the same operations at the end of the mapped region. Unless you use one of the sanitizers while fuzzing, right? If not ASan then I would wager MSan could detect this.
- grajaganDev 9y ago
  Yes - ASan or Valgrind detect invalid reads. You could either use them during the fuzz run (and take a performance hit) or after the run on the generated corpus of test cases.
- antirez 9y ago
  Definitely, but I was using Valgrind and ASAN against the already patched version... So definitely no luck :-D I could see the issue only when it happened in the wild, and it only happened in this very odd conditions.
  - wyldfire 9y ago
    Ok, thanks for clarifying. Incidentally, something's a little funny with this article. Missing spaces between words in a few places. E.g. "replacement formalloc()"
    - antirez 9y ago
      Sorry, wrote in medium and cut and pasted in my blog. I've too much trust in web technologies.
- gumby 9y ago
  >> so the bug would be totally harmless and silent, with the exception of doing the same operations at the end of the mapped region. “Harmless” only in a sense that the read won’t crash, but likely harmful if the result of the read is input to computation.
userbinator 9y ago
But GDB is not to blame, modern compilers, with optimizations turned on, generate code that can hardly be matched back to the source code. In my experience, GDB should get some blame at least --- far too many times I've seen it fail on code of the form var = foo(); where the value of var will clearly be in the return value register after executing the function call, but GDB steadfastly refuses to display var, claming it's been optimised out and unavailable. Inspecting eax/rax clearly shows the value is available.
- imglorp 9y ago
  Sometimes turning off all the -O optimizations and making sure all the -g options are on helps with that.
  - lsiebert 9y ago
    Of course sometimes the bug disappears if you do that :-P
- gumby 9y ago
  That is more likely a bug in the compiler code that emits the debug info.
  - therein 9y ago
    Correct, GCC 4.4.7 is notoriously bad at that in my experience.
johnrob 9y ago
While often convenient, adding “extra credit” to an otherwise focused commit can create a lot of confusion later on. It also makes the commit harder to understand (what does that line have to do with this fix?).
aerovistae 9y ago
god that guy is good at his job
- lsiebert 9y ago
  Yep, actually the whole Redis community is great; I'm looking forward to this year's redisconf (hopefully I'll have found a job and can afford to go).
elteto 9y ago
Any recommendations on how to learn x64 assembly? I’d love to be able to throw a piece of code in godbolt and be able to understand the disassembled code.
- pkaye 9y ago
  If you just want to learn to read an disassembly, use something like the Godbolt Compiler Explorer online to generate the assembly code for some trivial functions and try to understand their meaning line by line. Get one of the Intel reference manuals on x64 architecture to help you along.

114 points by stargrave 9y ago

Great article, bonus points for giving in passing the only explanation I have been able to understand on what a fuzzer check is and by extension how unrelated tools like QuickCheck should work.

twic 9y ago

I have a vaguely related story of a baffling segfault bug a few months ago. It was in some reference-counting code deep inside the Boost shared pointer implementation. It wasn't deterministic, and from correlating the crash log with the Boost code, i couldn't see how there could be a dodgy memory access at that point. Poking around with GDB, it seemed fine. Eventually i realised that i had compiled my application against one version of Boost, and then dynamically linked to a library i had compiled against a different version. This was all due to bad library path hygiene - my OS had one version of Boost installed, and i was trying to use another that i had installed locally, but GCC was ignoring my path options and getting seduced by the system Boost. After this, i learned about the --sysroot flag to GCC, which has helped to prevent this happening again. So yeah, protip: know what it is you're actually running. Also, never use C/C++.

danieldk 9y ago
It was in some reference-counting code deep inside the Boost shared pointer implementation. [...] Eventually i realised that i had compiled my application against one version of Boost, and then dynamically linked to a library i had compiled against a different version. Interesting. Boost's shared pointer implementation is header-only. Are they using shared_ptr in an API for a non-header Boost library? https://stackoverflow.com/questions/10334511/why-do-c-libraries-and-frameworks-never-use-smart-pointers/10342299#10342299 https://stackoverflow.com/questions/10334511/why-do-c-librar...
- twic 9y ago
  The library i was dynamically linking to was not a Boost library, it was some random other thing, which used Boost and used shared_ptr liberally in its API.

wyldfire 9y ago

> You can never detect a read overflow otherwise: it will just access data outside your structure, but inside mapped memory, so the bug would be totally harmless and silent, with the exception of doing the same operations at the end of the mapped region. Unless you use one of the sanitizers while fuzzing, right? If not ASan then I would wager MSan could detect this.

grajaganDev 9y ago
Yes - ASan or Valgrind detect invalid reads. You could either use them during the fuzz run (and take a performance hit) or after the run on the generated corpus of test cases.
antirez 9y ago
Definitely, but I was using Valgrind and ASAN against the already patched version... So definitely no luck :-D I could see the issue only when it happened in the wild, and it only happened in this very odd conditions.
- wyldfire 9y ago
  Ok, thanks for clarifying. Incidentally, something's a little funny with this article. Missing spaces between words in a few places. E.g. "replacement formalloc()"
  - antirez 9y ago
    Sorry, wrote in medium and cut and pasted in my blog. I've too much trust in web technologies.
gumby 9y ago
>> so the bug would be totally harmless and silent, with the exception of doing the same operations at the end of the mapped region. “Harmless” only in a sense that the read won’t crash, but likely harmful if the result of the read is input to computation.

userbinator 9y ago

But GDB is not to blame, modern compilers, with optimizations turned on, generate code that can hardly be matched back to the source code. In my experience, GDB should get some blame at least --- far too many times I've seen it fail on code of the form var = foo(); where the value of var will clearly be in the return value register after executing the function call, but GDB steadfastly refuses to display var, claming it's been optimised out and unavailable. Inspecting eax/rax clearly shows the value is available.

imglorp 9y ago
Sometimes turning off all the -O optimizations and making sure all the -g options are on helps with that.
- lsiebert 9y ago
  Of course sometimes the bug disappears if you do that :-P
gumby 9y ago
That is more likely a bug in the compiler code that emits the debug info.
- therein 9y ago
  Correct, GCC 4.4.7 is notoriously bad at that in my experience.

johnrob 9y ago

While often convenient, adding “extra credit” to an otherwise focused commit can create a lot of confusion later on. It also makes the commit harder to understand (what does that line have to do with this fix?).

aerovistae 9y ago

god that guy is good at his job

lsiebert 9y ago
Yep, actually the whole Redis community is great; I'm looking forward to this year's redisconf (hopefully I'll have found a job and can afford to go).

elteto 9y ago

Any recommendations on how to learn x64 assembly? I’d love to be able to throw a piece of code in godbolt and be able to understand the disassembled code.

pkaye 9y ago
If you just want to learn to read an disassembly, use something like the Godbolt Compiler Explorer online to generate the assembly code for some trivial functions and try to understand their meaning line by line. Get one of the Intel reference manuals on x64 architecture to help you along.