[Toybox] imgtec patch: Fix static linkage of toybox binary.

[Evgenii Stepanov]

Now, if we want to really preserve this check logic under safestack,
we will have to do something safestack-specific. There is no way to
keep pretending that there is a single, continuous stack region and
still get realistic results.

1. Use __builtin_frame_pointer and __builtin___get_unsafe_stack_ptr().
They are supported whenever safestack is supported and can be
protected with simple preprocessor guards.
2. Rely on safestack semantics to know which of the two stacks a
variable gets allocated on. This is embedding some knowledge about
safestack implementation (not just the ABI) into the application, but
it relies on the fundamental security promise of safestack and very
unlikely to change. For example, this line in my original patch:
  intptr_t volatile stackaddr = (intptr_t)&which;
leaks the address of "which" into a volatile location. Such variables
are guaranteed to be allocated on the "unsafe" stack.

(2) does not seem to have any advantage over (1). Would (1) be acceptable?

On Mon, May 9, 2016 at 2:23 PM, Evgenii Stepanov <eugenis at google.com> wrote:
> Does https://android-review.googlesource.com/#/c/223875/ look OK?
> It disables the stack depth check if toybox is configured with no recursion.
>
>
> On Sat, May 7, 2016 at 11:30 AM, enh <enh at google.com> wrote:
>> On Fri, May 6, 2016 at 10:58 PM, Rob Landley <rob at landley.net> wrote:
>>> On 05/07/2016 12:16 AM, Evgenii Stepanov wrote:
>>>> Sorry, I did not look at the problem hard enough.
>>>> The real issue is interaction of this code with safestack
>>>> (http://clang.llvm.org/docs/SafeStack.html), which splits the stack in
>>>> 2 disjoint memory regions. If the two variables are allocated on
>>>> different stacks, the comparison result is truly undefined.
>>>>
>>>> I don't really understand what this code is tying to do. Is it
>>>> catching unlimited stack growth? Why does the comment speak about
>>>> heap?
>>>
>>> It's a heuristic that enables an optimization. You could select
>>> CONFIG_TOYBOX_NORECURSE to disable this optimization.
>>
>> i thought we were still using that, but it looks like i undid my
>> setting of that option here:
>> https://googleplex-android.git.corp.google.com/platform/external/toybox/+/a729fc8373ade0ced1cd0dd5ad43ef6a61a5cd24
>>
>> https://android-review.googlesource.com/223514 (unsubmitted) turns it
>> back on, since that might be easier than trying to come up with a
>> heuristic that works well in a SafeStack world. as long as we're still
>> using mksh i don't think it matters much anyway.
>>
>>> The help text of that option describes it a little: when one toybox
>>> command calls another, it can either recurse into the new command's
>>> main() function, or call the actual execve() to relaunch the toybox
>>> binary with a fresh environment. Recursing is much faster, but has the
>>> downside that if you do enough in a row you tend to accumulate debris
>>> (open filehandles and unfreed mallocs and such from being halfway
>>> through another program). (Plus if you do it _forever_, you'll actualy
>>> run out of stack.) So it checks how much stack we've used as a simple
>>> heuristic to see whether we should recurse or should exec.
>>>
>>> This heuristic has not been particularly tuned, that's one of my toysh
>>> todo items. (toysh is likely to be the heaviest user.)
>>>
>>> Rob
>>
>>
>>
>> --
>> Elliott Hughes - http://who/enh - http://jessies.org/~enh/
>> Android native code/tools questions? Mail me/drop by/add me as a reviewer.