On Sat, Aug 9, 2014 at 4:12 PM, Brent Cook <bust...@gmail.com> wrote:
> Which object is used past its lifetime? buf is local to the function, and
> secret is a static global.

That's exactly the issue: buf is local.

Conceptually, to make sure explicit_bzero() is working, we want to
write a test like this:

    int *test() {
        int buf = 42;
        explicit_bzero(&buf, sizeof(buf));
        return &buf;
    }

    void check() {
         int *p = test();
         assert(*p == 0);
    }

But this is explicitly undefined behavior in C, because the value of
&buf becomes indeterminate once test() returns and buf goes out of
scope.  Returning &buf might also affect the compiler's decision of
whether it can optimize away explicit_bzero().

As a workaround, explicit_bzero.c arranges for test() to run using
"altstack"'s memory as the stack, and we change the test() function to
be conceptually like:

    int *test() {
        int buf = 42;
        int *p = memmem(altstack, sizeof(altstack), &buf, sizeof(buf));
        explicit_bzero(&buf, sizeof(buf));
        return p;
    }

The memmem() call searches through altstack looking for a sequence of
memory that has the same contents as buf.  In practice because we've
arranged to use altstack as our stack and buf is a large unique value,
the matching address p is actually going to be &buf itself; but as far
as the compiler's concerned, it's actually an offset in altstack
instead.

Changing "memmem(altstack, sizeof(altstack), buf, sizeof(buf))" to
"memmem(buf, sizeof(buf), secret, sizeof(secret))" changes the code
back to the original, because now memmem()'s returning a pointer into
buf instead of into altstack.

> Enabling the stack protector code really does shift buf by 112 bytes on the
> signal stack, so I think its more likely working and inserting some guards
> at the head and tail of the stack signal stack.

That should be fine: explicit_bzero.c isn't assuming buf is going to
be at any particular offset within altstack, it just expects it to be
*somewhere* within it.

The issue with asan's stack checking logic is it's not aware of POSIX
signal stacks.  We've explicitly allocated the stack memory ourselves
and we're accessing it as an array of unsigned chars.  We might get
unspecified values, but we can't trigger undefined behavior.  However,
asan can't easily distinguish this case from any other
use-after-return issue, and it's a pretty rare case anyway so they're
not likely to invest resources into fixing it.

I'd say we're better off trying to mark the test functions
__attribute__((no_asan)) or something.

Reply via email to