On 05/03/2018 10:55 AM, Segher Boessenkool wrote:
> Hi!
>
> On Wed, May 02, 2018 at 07:57:55AM +0100, Thomas Preudhomme wrote:
>> As mentionned in the ticket this was my first thought but this means
>> making the pattern aware of all the possible way the address could be
>> access (PIC Vs non-PIC, Arm Vs Thumb-2 Vs Thumb-1) to decide how many
>> scratch registers are needed. I'd rather reuse the existing pattern as
>> much as possible to make sure they are well tested. Ideally I wanted a
>> way to mark a REG RTX so that it is never spilled and such that the
>> mark is propagated when the register is moved to another register or
>> propagated. But that is a bigger change so decided it should be an
>> improvement for later but needed another solution right now.
>
> How would that work, esp. for pseudos? If too many regs have such a
> mark then the compiler will have to sorry() or similar, not a good
> thing at all.
>
>> By the way about making sure the address is not left in a register, I
>> have a question regarding the current stack_protect_set and
>> stack_protect_check pattern and their requirements to have register
>> cleared afterwards: why is that necessary? Currently not all registers
>> are cleared and the guard is available in the canari before it is
>> overwritten anyway so I don't see how clearing the register adds any
>> extra security. What sort of attack is it protecting against?
>
> From md.texi:
>
> @item @samp{stack_protect_set}
> This pattern, if defined, moves a @code{ptr_mode} value from the memory
> in operand 1 to the memory in operand 0 without leaving the value in
> a register afterward. This is to avoid leaking the value some place
> that an attacker might use to rewrite the stack guard slot after
> having clobbered it.
>
> (etc.)
>
> Having the canary in a global variable makes it a lot easier for exploit
> code to access it then if it is e.g. in TLS data. Actually leaking a
> pointer to it would make it extra easy...
Yup. And at least one guard (not the stack guard) has that properly.
It's stuffed into static storage. Not good.
Though I must admit it was awful convenient when tracking down a bug in
how the guard was used.
jeff
