Re: [patch] for PR 28281: enable -fstack-protector for the x86_64 linux kernel

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Jul 06, 2006 at 11:33:52AM +0200, Arjan van de Ven wrote:
> Hi,
> 
> sadly another buffer overflow has been found in the linux kernel
> (http://www.heise.de/newsticker/meldung/75117 and
> http://bugzilla.kernel.org/show_bug.cgi?id=2966 ).
> I'd really like to be able to use -fstack-protector for the kernel, and
> have a kernel patch series lined up for that. However, due to the
> different segment registers used for TLS between userspace and kernel
> space, a small mod to gcc is needed to actually make this work.
> (The reason for the different segment registers is performance: by using
> different registers, a system call doesn't need to trash the userspace
> segment register, which would be expensive)
> 
> Below is the patch to fix this, including a testcase
> 
> 2006-07-06  Arjan van de Ven <arjan@linux.intel.com>
> 	* config/i386/i386.md: add conditonal for kernel side
> 	stack-protector

The ChangeLog entry is bad, should be:

2006-07-06  Arjan van de Ven <arjan@linux.intel.com>

	* config/i386/i386.md (stack_tls_protect_set_di,
	stack_tls_protect_test_di): Use %gs rather than %fs for
	-mcmodel=kernel.

	* gcc.target/i386/stack-prot-kernel.c: New test.

The test should go into gcc/testsuite/gcc.target/i386/ rather than
gcc.dg (as it is i?86/x86_64 specific) and should start with:
/* { dg-do compile { target lp64 } } */
/* { dg-options "-O2 -fstack-protector-all -mcmodel=kernel" } */

Other than that it looks good to me, but somebody else has to approve it.

> --- gcc/testsuite/gcc.dg/stack-prot-kernel.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/stack-prot-kernel.c	(revision 0)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target x86_64-*-linux* } } */
> +/* { dg-options "-O2 -fstack-protector-all -mcmodel=kernel" } */
> +
> +void test1 (int x)
> +{
> +  char p[40];
> +  int i;
> +  for (i=0; i<40; i++)
> +	p[i] = x;
> +}
> +
> +/* { dg-final { scan-assembler-not "%gs" } } */
> --- gcc/config/i386/i386.md	(revision 115220)
> +++ gcc/config/i386/i386.md	(working copy)
> @@ -20393,7 +20393,12 @@
>     (set (match_scratch:DI 2 "=&r") (const_int 0))
>     (clobber (reg:CC FLAGS_REG))]
>    "TARGET_64BIT"
> -  "mov{q}\t{%%fs:%P1, %2|%2, QWORD PTR %%fs:%P1}\;mov{q}\t{%2, %0|%0, %2}\;xor{l}\t%k2, %k2"
> +  {  
> +     if (ix86_cmodel != CM_KERNEL)
> +        return "mov{q}\t{%%fs:%P1, %2|%2, QWORD PTR %%fs:%P1}\;mov{q}\t{%2, %0|%0, %2}\;xor{l}\t%k2, %k2";
> +     else 
> +        return "mov{q}\t{%%gs:%P1, %2|%2, QWORD PTR %%gs:%P1}\;mov{q}\t{%2, %0|%0, %2}\;xor{l}\t%k2, %k2";
> +  }
>    [(set_attr "type" "multi")])
>  
>  (define_expand "stack_protect_test"
> @@ -20461,7 +20466,12 @@
>  		    UNSPEC_SP_TLS_TEST))
>     (clobber (match_scratch:DI 3 "=r"))]
>    "TARGET_64BIT"
> -  "mov{q}\t{%1, %3|%3, %1}\;xor{q}\t{%%fs:%P2, %3|%3, QWORD PTR %%fs:%P2}"
> +  {
> +     if (ix86_cmodel != CM_KERNEL)
> +        return "mov{q}\t{%1, %3|%3, %1}\;xor{q}\t{%%fs:%P2, %3|%3, QWORD PTR %%fs:%P2}";
> +     else
> +        return "mov{q}\t{%1, %3|%3, %1}\;xor{q}\t{%%gs:%P2, %3|%3, QWORD PTR %%gs:%P2}";
> +  }
>    [(set_attr "type" "multi")])
>  
>  (include "sse.md")

	Jakub