Re: [PATCH] [RFC] rs6000: -mreadonly-in-sdata (PR82411)

[Kees Cook via gcc-patches]

On Tue, Feb 27, 2018 at 2:01 PM, Segher Boessenkool
 wrote:
> This adds a new option -mreadonly-in-sdata (on by default) that
> controls whether readonly data can be put in sdata.  (For EABI this
> does nothing, readonly data is put in sdata2 as usual).

Cool! Thanks for working on this.

> Kees, could you try this out with your use case?  Add the flag
> -mno-readonly-in-sdata in your build scripts.  The patch is against
> GCC trunk.

I'm struggling to create a ppc cross compiler, otherwise I would be
happy to test this. :)

If you're able to build a ppc kernel and remove the "static" from
mm/rodata_test_data.c's rodata_test_data and see the rodata_test_data
variable not in .sdata, that should be sufficient. The case reported
was here:
https://lkml.org/lkml/2017/9/21/156

I'll continue trying to solve my cross build issue...

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] [RFC] rs6000: -mreadonly-in-sdata (PR82411)

[Segher Boessenkool]

On Wed, Feb 28, 2018 at 01:46:33PM -0800, Kees Cook wrote:
> On Tue, Feb 27, 2018 at 2:01 PM, Segher Boessenkool
>  wrote:
> > This adds a new option -mreadonly-in-sdata (on by default) that
> > controls whether readonly data can be put in sdata.  (For EABI this
> > does nothing, readonly data is put in sdata2 as usual).
> 
> Cool! Thanks for working on this.
> 
> > Kees, could you try this out with your use case?  Add the flag
> > -mno-readonly-in-sdata in your build scripts.  The patch is against
> > GCC trunk.
> 
> I'm struggling to create a ppc cross compiler, otherwise I would be
> happy to test this. :)

For compiling kernels you can use


> If you're able to build a ppc kernel and remove the "static" from
> mm/rodata_test_data.c's rodata_test_data and see the rodata_test_data
> variable not in .sdata, that should be sufficient. The case reported
> was here:
> https://lkml.org/lkml/2017/9/21/156
> 
> I'll continue trying to solve my cross build issue...

I'm trying to figure out how to get that file to build at all :-)


Segher

Re: [PATCH] [RFC] rs6000: -mreadonly-in-sdata (PR82411)

[Kees Cook via gcc-patches]

On Wed, Feb 28, 2018 at 3:23 PM, Segher Boessenkool
 wrote:
> On Wed, Feb 28, 2018 at 01:46:33PM -0800, Kees Cook wrote:
>> On Tue, Feb 27, 2018 at 2:01 PM, Segher Boessenkool
>>  wrote:
>> > This adds a new option -mreadonly-in-sdata (on by default) that
>> > controls whether readonly data can be put in sdata.  (For EABI this
>> > does nothing, readonly data is put in sdata2 as usual).
>>
>> Cool! Thanks for working on this.
>>
>> > Kees, could you try this out with your use case?  Add the flag
>> > -mno-readonly-in-sdata in your build scripts.  The patch is against
>> > GCC trunk.
>>
>> I'm struggling to create a ppc cross compiler, otherwise I would be
>> happy to test this. :)
>
> For compiling kernels you can use
> 

Hah, yes, I had just asked around privately about cross building tools
and was aimed there only to discover you're the author. ;)

>> If you're able to build a ppc kernel and remove the "static" from
>> mm/rodata_test_data.c's rodata_test_data and see the rodata_test_data
>> variable not in .sdata, that should be sufficient. The case reported
>> was here:
>> https://lkml.org/lkml/2017/9/21/156
>>
>> I'll continue trying to solve my cross build issue...
>
> I'm trying to figure out how to get that file to build at all :-)

This works for me with my distro cross compiler:

$ git diff
diff --git a/mm/rodata_test.c b/mm/rodata_test.c
index d908c8769b48..6bb4deb12e78 100644
--- a/mm/rodata_test.c
+++ b/mm/rodata_test.c
@@ -14,7 +14,7 @@
 #include 
 #include 

-static const int rodata_test_data = 0xC3;
+const int rodata_test_data = 0xC3;

 void rodata_test(void)
 {

$ CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc make wii_defconfig
$ for i in CONFIG_STRICT_KERNEL_RWX \
   CONFIG_DEBUG_KERNEL \
   CONFIG_DEBUG_RODATA_TEST ; do
CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc ./scripts/config -e $i
done

$ CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc make -j...
...
$ objdump -t mm/rodata_test.o | grep rodata_test_data
 g O .sdata 0004 rodata_test_data


-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] [RFC] rs6000: -mreadonly-in-sdata (PR82411)

[Segher Boessenkool]

On Wed, Feb 28, 2018 at 04:15:23PM -0800, Kees Cook wrote:
> On Wed, Feb 28, 2018 at 3:23 PM, Segher Boessenkool
>  wrote:
> > I'm trying to figure out how to get that file to build at all :-)

> $ CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc make wii_defconfig
> $ for i in CONFIG_STRICT_KERNEL_RWX \
>CONFIG_DEBUG_KERNEL \
>CONFIG_DEBUG_RODATA_TEST ; do
> CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc ./scripts/config -e $i
> done

I use ppc6xx_defconfig normally; I had to disable CONFIG_HIBERNATION
as well.

Without the new flag, rodata_test_data ends up in .sdata in the object
file and in .data in vmlinux.

Adding

===
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ccd2556..31b9613 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -143,6 +143,8 @@ CFLAGS-$(CONFIG_PPC64)  += $(call cc-option,-mcmodel=med
 CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
 CFLAGS-$(CONFIG_PPC32) := -ffixed-r2 $(MULTIPLEWORD)
 
+CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
+
 ifeq ($(CONFIG_PPC_BOOK3S_64),y)
 CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,-mtune=power4)
 CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power4
===

it ends up in .rodata in both.

It would be nice to have a more comprehensive test, but this will do; I'll
commit it tomorrow (will resend, with doc and changelog and all that).

Cheers,


Segher

Re: [PATCH] [RFC] rs6000: -mreadonly-in-sdata (PR82411)

[Kees Cook via gcc-patches]

On Wed, Feb 28, 2018 at 4:39 PM, Segher Boessenkool
 wrote:
> On Wed, Feb 28, 2018 at 04:15:23PM -0800, Kees Cook wrote:
>> On Wed, Feb 28, 2018 at 3:23 PM, Segher Boessenkool
>>  wrote:
>> > I'm trying to figure out how to get that file to build at all :-)
>
>> $ CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc make wii_defconfig
>> $ for i in CONFIG_STRICT_KERNEL_RWX \
>>CONFIG_DEBUG_KERNEL \
>>CONFIG_DEBUG_RODATA_TEST ; do
>> CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc ./scripts/config -e $i
>> done
>
> I use ppc6xx_defconfig normally; I had to disable CONFIG_HIBERNATION
> as well.
>
> Without the new flag, rodata_test_data ends up in .sdata in the object
> file and in .data in vmlinux.
>
> Adding
>
> ===
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index ccd2556..31b9613 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -143,6 +143,8 @@ CFLAGS-$(CONFIG_PPC64)  += $(call 
> cc-option,-mcmodel=med
>  CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
>  CFLAGS-$(CONFIG_PPC32) := -ffixed-r2 $(MULTIPLEWORD)
>
> +CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
> +
>  ifeq ($(CONFIG_PPC_BOOK3S_64),y)
>  CFLAGS-$(CONFIG_GENERIC_CPU) += $(call cc-option,-mtune=power7,-mtune=power4)
>  CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power4
> ===
>
> it ends up in .rodata in both.

Excellent! That's perfect. :)

> It would be nice to have a more comprehensive test, but this will do; I'll
> commit it tomorrow (will resend, with doc and changelog and all that).

Thank you! Do you want to send the Makefile fix upstream too, or
should I route that?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] [RFC] rs6000: -mreadonly-in-sdata (PR82411)

[Segher Boessenkool]

On Wed, Feb 28, 2018 at 04:43:56PM -0800, Kees Cook wrote:
> On Wed, Feb 28, 2018 at 4:39 PM, Segher Boessenkool
>  wrote:
> > On Wed, Feb 28, 2018 at 04:15:23PM -0800, Kees Cook wrote:
> >> On Wed, Feb 28, 2018 at 3:23 PM, Segher Boessenkool
> >>  wrote:
> >> > I'm trying to figure out how to get that file to build at all :-)
> >
> >> $ CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc make wii_defconfig
> >> $ for i in CONFIG_STRICT_KERNEL_RWX \
> >>CONFIG_DEBUG_KERNEL \
> >>CONFIG_DEBUG_RODATA_TEST ; do
> >> CROSS_COMPILE=powerpc-linux-gnu- ARCH=powerpc ./scripts/config -e $i
> >> done
> >
> > I use ppc6xx_defconfig normally; I had to disable CONFIG_HIBERNATION
> > as well.
> >
> > Without the new flag, rodata_test_data ends up in .sdata in the object
> > file and in .data in vmlinux.
> >
> > Adding
> >
> > ===
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index ccd2556..31b9613 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -143,6 +143,8 @@ CFLAGS-$(CONFIG_PPC64)  += $(call 
> > cc-option,-mcmodel=med
> >  CFLAGS-$(CONFIG_PPC64) += $(call 
> > cc-option,-mno-pointers-to-nested-functions)
> >  CFLAGS-$(CONFIG_PPC32) := -ffixed-r2 $(MULTIPLEWORD)
> >
> > +CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
> > +
> >  ifeq ($(CONFIG_PPC_BOOK3S_64),y)
> >  CFLAGS-$(CONFIG_GENERIC_CPU) += $(call 
> > cc-option,-mtune=power7,-mtune=power4)
> >  CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=power4
> > ===
> >
> > it ends up in .rodata in both.
> 
> Excellent! That's perfect. :)
> 
> > It would be nice to have a more comprehensive test, but this will do; I'll
> > commit it tomorrow (will resend, with doc and changelog and all that).
> 
> Thank you! Do you want to send the Makefile fix upstream too, or
> should I route that?

If you could?  Have my

Signed-off-by: Segher Boessenkool 

if you want.

It'll be a while before all users have a compiler with the new flag, but
I'll backport it to GCC 7 and GCC 6 (the open release branches) to speed
that up a bit.


Segher