[PATCH] Protection against stack-based memory corruption errors using SafeStack
jfb at chromium.org
Fri May 29 10:07:40 PDT 2015
In http://reviews.llvm.org/D6094#181019, @kcc wrote:
> In http://reviews.llvm.org/D6094#181007, @jfb wrote:
> > I like the technical approach overall, but the testing strategy worries me. Specifically, this is now at the IR level and I have a hard time convincing myself that the safe stack's location isn't moved to other registers, or stored on the unsafe stack or heap. This isn't just about the current implementation: I worry of breaking safe stack as LLVM gains new features (e.g. GC roots breaks it, adding similar features would silently do the same), or with different runtimes.
> > The only way I can think of convincing myself is by using binary instrumentation: keep track of safe stack ranges for each thread at runtime, and use QEMU/Pin/Valgrind/... to find any leaks. This will have false positives, but assuming there's no stack-cookie-like leaks then the lack of any warnings would mean that the safe stacks' locations aren't leaking. This tool could be run on the test suite.
> > I don't want to block checking in safe stack on such a tool, but I would be wary of using it as a security feature without having tested it.
> > Thoughts?
> Won't it be enough to have full (runnable) tests like another patch (http://reviews.llvm.org/D6096) already has?
> We can (and absolutely must!) add more of those tests in subsequent commits.
My concern is that IR may look OK in the tests, but further passes or the ISA-specific lowering could leak the safe stack's location. I'm also not sure small tests cover the entirety of what we'd want to test, especially when we consider what the optimizer can do.
Agreed the tests are useful! I'm just not sure they're sufficient.
More information about the llvm-commits