Re: over-permissive stack_chk_guard on ARM

[Yao Qi <yao at codesourcery dot com>]

Joel Brobecker <brobecker@adacore.com> writes:

> But the implementation seems to be going further than that.
> If the location of the first ldr points to data that's not
> the address of __stack_chk_guard, then it looks at the next
> two instructions, to see if they might following another
> pattern:
>
>       /* Step 2: ldr Rd, [Rn, #immed], encoding T1.  */
>       /* Step 3: str Rd, [Rn, #immed], encoding T1.  */
>
> Looking at the code and the function description, it seems to me
> that the normal situation would be what the comment alluded to,
> and that if it was the entire story, we wouldn't have needed
> the code doing steps 2 & 3. But, looking at the email archives

Sorry, I don't understand why do you think steps 2 & 3 are not needed?
Do you mean we don't have to go to step 2 & 3 if we can't find symbol
__stack_chk_guard in step 1?

> as well as the bug report initially referenced, I can't find
> really any explanation for what prompted you to add that code.
> I would need that in order to adjust the heuristics without
> breaking your situation.

Currently, we do so in order to handle the case symbol __stack_chk_guard
is removed, as the comments said:

  /* If name of symbol doesn't start with '__stack_chk_guard', this
     instruction sequence is not for stack protector.  If symbol is
     removed, we conservatively think this sequence is for stack
     protector.  */

However, I don't recall under what circumstance symbol
'__stack_chk_guard' can be removed.  __stack_chk_guard is in .dynsym
section, so it can't be removed.  (I presume symbols in .dynsym can't be
removed, correct me if I am wrong).  If I am correct, we can restrict
the condition in step 1 that return early if the symbol name doesn't
start with '__stack_chk_guard'.  Then, step 2 & 3 is not needed, or we
can keep them as a sanity check?

-- 
Yao (éå)