m68k: fix clobbering a5 in setjmp() [BZ #24202]

  setjmp() uses C code to store current registers into jmp_buf
environment. -fstack-protector-all places canary into setjmp()
prologue and clobbers 'a5' before it gets saved.

The change inhibits stack canary injection to avoid clobber.

	[BZ #24202]
	* sysdeps/m68k/setjmp.c (*setjmp): Use
	inhibit_stack_protector.

CC: James Le Cuirot <chewi@gentoo.org>
CC: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 ChangeLog             | 6 ++++++
 sysdeps/m68k/setjmp.c | 1 +
 2 files changed, 7 insertions(+)
  

On 10/02/2019 21:29, Sergei Trofimovich wrote:
> setjmp() uses C code to store current registers into jmp_buf
> environment. -fstack-protector-all places canary into setjmp()
> prologue and clobbers 'a5' before it gets saved.
> 
> The change inhibits stack canary injection to avoid clobber.
> 
> 	[BZ #24202]
> 	* sysdeps/m68k/setjmp.c (*setjmp): Use
> 	inhibit_stack_protector.

LGTM. I am not seeing the stack smash issue with example provided
in BZ#24202 in my environment (gcc 6.2.1, Aranym2015Jan on 3.16.0-4-m68k),
however the fix shows the expected printed value.

> 
> CC: James Le Cuirot <chewi@gentoo.org>
> CC: Andreas Schwab <schwab@linux-m68k.org>
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
>  ChangeLog             | 6 ++++++
>  sysdeps/m68k/setjmp.c | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index c143073ca7..c1e8dd9c3a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-02-10  Sergei Trofimovich  <slyfox@gentoo.org>
> +
> +	[BZ #24202]
> +	* sysdeps/m68k/setjmp.c (*setjmp): Use
> +	inhibit_stack_protector.
> +
>  2019-02-06  Joseph Myers  <joseph@codesourcery.com>
>  
>  	* elf/dl-load.h (_dl_postprocess_loadcmd): Use __always_inline
> diff --git a/sysdeps/m68k/setjmp.c b/sysdeps/m68k/setjmp.c
> index 39ab7178a0..62bd281119 100644
> --- a/sysdeps/m68k/setjmp.c
> +++ b/sysdeps/m68k/setjmp.c
> @@ -19,6 +19,7 @@
>  
>  /* Save the current program position in ENV and return 0.  */
>  int
> +inhibit_stack_protector
>  #if defined BSD_SETJMP
>  # undef setjmp
>  # define savemask 1
>
  

On 10 Feb 2019, Sergei Trofimovich uttered the following:

> setjmp() uses C code to store current registers into jmp_buf
> environment. -fstack-protector-all places canary into setjmp()
> prologue and clobbers 'a5' before it gets saved.
>
> The change inhibits stack canary injection to avoid clobber.

LGTM. I tried to identify everything that depended on clobbering no regs
and inhibit stack-protection in all of them, but clearly failed to spot
several setjmp implementations. Sorry!
  

* Sergei Trofimovich:

> setjmp() uses C code to store current registers into jmp_buf
> environment. -fstack-protector-all places canary into setjmp()
> prologue and clobbers 'a5' before it gets saved.
>
> The change inhibits stack canary injection to avoid clobber.
>
> 	[BZ #24202]
> 	* sysdeps/m68k/setjmp.c (*setjmp): Use
> 	inhibit_stack_protector.

The code is still invalid.  The C compiler can still clobber any
register it wants.  So this does not actually fix the bug.  But I think
it's an incremental improvement.

Thanks,
Florian
  

On 24 Feb 2019, Florian Weimer uttered the following:

>> setjmp() uses C code to store current registers into jmp_buf
>> environment. -fstack-protector-all places canary into setjmp()
>> prologue and clobbers 'a5' before it gets saved.
>>
>> The change inhibits stack canary injection to avoid clobber.
>>
>> 	[BZ #24202]
>> 	* sysdeps/m68k/setjmp.c (*setjmp): Use
>> 	inhibit_stack_protector.
>
> The code is still invalid.  The C compiler can still clobber any
> register it wants.  So this does not actually fix the bug.  But I think
> it's an incremental improvement.

I'd say it's in the same state as pthread_cond_wait() et al were in on
i386 before 7a25d6a84df9fea56963569ceccaaf7c2a88f161: works with the
compiler as it is now, by sheer good fortune.

To me this feels like code that can't really be written in C safely, but
it's so arch-dependent that having it depend on the compiler not
emitting anything else in the prologue is probably not *too* likely to
break. But we should probably add the reproducer for this to glibc's
'make check' to stop it regressing again.
  

On 2/11/19 4:59 AM, Sergei Trofimovich wrote:
> setjmp() uses C code to store current registers into jmp_buf
> environment. -fstack-protector-all places canary into setjmp()
> prologue and clobbers 'a5' before it gets saved.
> 
> The change inhibits stack canary injection to avoid clobber.
> 

I've dropped the Signed-off-by and pushed this since it had approvals 
but wasn't committed, probably due to oversight.

Thanks,
Siddhesh