linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
@ 2015-12-07 22:35 Kees Cook
  2015-12-08  7:47 ` Ard Biesheuvel
  0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2015-12-07 22:35 UTC (permalink / raw)
  To: linux-arm-kernel

When rodata is large enough that it crosses a section boundary after the
kernel text, mark the rest NX. This is as close to full NX of rodata as
we can get without splitting page tables or doing section alignment via
CONFIG_DEBUG_ALIGN_RODATA.

When the config is:

 CONFIG_DEBUG_RODATA=y
 # CONFIG_DEBUG_ALIGN_RODATA is not set

Before:

---[ Kernel Mapping ]---
0x80000000-0x80100000           1M     RW NX SHD
0x80100000-0x80a00000           9M     ro x  SHD
0x80a00000-0xa0000000         502M     RW NX SHD

After:

---[ Kernel Mapping ]---
0x80000000-0x80100000           1M     RW NX SHD
0x80100000-0x80700000           6M     ro x  SHD
0x80700000-0x80a00000           3M     ro NX SHD
0x80a00000-0xa0000000         502M     RW NX SHD

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
- static declaration, ard
---
 arch/arm/kernel/vmlinux.lds.S | 9 +++++++--
 arch/arm/mm/init.c            | 7 ++++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index a6e395c53a48..9c249c71fda1 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -8,9 +8,7 @@
 #include <asm/thread_info.h>
 #include <asm/memory.h>
 #include <asm/page.h>
-#ifdef CONFIG_DEBUG_RODATA
 #include <asm/pgtable.h>
-#endif
 
 #define PROC_INFO							\
 	. = ALIGN(4);							\
@@ -337,6 +335,13 @@ SECTIONS
 }
 
 /*
+ * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
+ * be the first section-aligned location after __start_rodata. Otherwise,
+ * it will be equal to __start_rodata.
+ */
+__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
+
+/*
  * These must never be empty
  * If you have to comment these two assert statements out, your
  * binutils is too old (for other reasons as well)
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 321d3683dc7c..6b16f6cf4843 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -579,6 +579,9 @@ struct section_perm {
 	pmdval_t clear;
 };
 
+/* First section-aligned location@or after __start_rodata. */
+extern char __start_rodata_section_aligned[];
+
 static struct section_perm nx_perms[] = {
 	/* Make pages tables, etc before _stext RW (set NX). */
 	{
@@ -596,16 +599,14 @@ static struct section_perm nx_perms[] = {
 		.mask	= ~PMD_SECT_XN,
 		.prot	= PMD_SECT_XN,
 	},
-#ifdef CONFIG_DEBUG_ALIGN_RODATA
 	/* Make rodata NX (set RO in ro_perms below). */
 	{
 		.name	= "rodata NX",
-		.start  = (unsigned long)__start_rodata,
+		.start  = (unsigned long)__start_rodata_section_aligned,
 		.end    = (unsigned long)__init_begin,
 		.mask   = ~PMD_SECT_XN,
 		.prot   = PMD_SECT_XN,
 	},
-#endif
 };
 
 static struct section_perm ro_perms[] = {
-- 
1.9.1


-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2015-12-07 22:35 [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX Kees Cook
@ 2015-12-08  7:47 ` Ard Biesheuvel
  2015-12-08 18:38   ` Kees Cook
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2015-12-08  7:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
> When rodata is large enough that it crosses a section boundary after the
> kernel text, mark the rest NX. This is as close to full NX of rodata as
> we can get without splitting page tables or doing section alignment via
> CONFIG_DEBUG_ALIGN_RODATA.
>
> When the config is:
>
>  CONFIG_DEBUG_RODATA=y
>  # CONFIG_DEBUG_ALIGN_RODATA is not set
>
> Before:
>
> ---[ Kernel Mapping ]---
> 0x80000000-0x80100000           1M     RW NX SHD
> 0x80100000-0x80a00000           9M     ro x  SHD
> 0x80a00000-0xa0000000         502M     RW NX SHD
>
> After:
>
> ---[ Kernel Mapping ]---
> 0x80000000-0x80100000           1M     RW NX SHD
> 0x80100000-0x80700000           6M     ro x  SHD
> 0x80700000-0x80a00000           3M     ro NX SHD
> 0x80a00000-0xa0000000         502M     RW NX SHD
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> v2:
> - static declaration, ard
> ---
>  arch/arm/kernel/vmlinux.lds.S | 9 +++++++--
>  arch/arm/mm/init.c            | 7 ++++---
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index a6e395c53a48..9c249c71fda1 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -8,9 +8,7 @@
>  #include <asm/thread_info.h>
>  #include <asm/memory.h>
>  #include <asm/page.h>
> -#ifdef CONFIG_DEBUG_RODATA
>  #include <asm/pgtable.h>
> -#endif
>
>  #define PROC_INFO                                                      \
>         . = ALIGN(4);                                                   \
> @@ -337,6 +335,13 @@ SECTIONS
>  }
>
>  /*
> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
> + * be the first section-aligned location after __start_rodata. Otherwise,
> + * it will be equal to __start_rodata.
> + */
> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
> +
> +/*
>   * These must never be empty
>   * If you have to comment these two assert statements out, your
>   * binutils is too old (for other reasons as well)
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 321d3683dc7c..6b16f6cf4843 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -579,6 +579,9 @@ struct section_perm {
>         pmdval_t clear;
>  };
>
> +/* First section-aligned location at or after __start_rodata. */
> +extern char __start_rodata_section_aligned[];
> +
>  static struct section_perm nx_perms[] = {
>         /* Make pages tables, etc before _stext RW (set NX). */
>         {
> @@ -596,16 +599,14 @@ static struct section_perm nx_perms[] = {
>                 .mask   = ~PMD_SECT_XN,
>                 .prot   = PMD_SECT_XN,
>         },
> -#ifdef CONFIG_DEBUG_ALIGN_RODATA
>         /* Make rodata NX (set RO in ro_perms below). */
>         {
>                 .name   = "rodata NX",
> -               .start  = (unsigned long)__start_rodata,
> +               .start  = (unsigned long)__start_rodata_section_aligned,
>                 .end    = (unsigned long)__init_begin,

What happens if start > end ?

>                 .mask   = ~PMD_SECT_XN,
>                 .prot   = PMD_SECT_XN,
>         },
> -#endif
>  };
>
>  static struct section_perm ro_perms[] = {
> --
> 1.9.1
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2015-12-08  7:47 ` Ard Biesheuvel
@ 2015-12-08 18:38   ` Kees Cook
  2016-01-22 21:19     ` Kees Cook
  0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2015-12-08 18:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 7, 2015 at 11:47 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
>> When rodata is large enough that it crosses a section boundary after the
>> kernel text, mark the rest NX. This is as close to full NX of rodata as
>> we can get without splitting page tables or doing section alignment via
>> CONFIG_DEBUG_ALIGN_RODATA.
>>
>> When the config is:
>>
>>  CONFIG_DEBUG_RODATA=y
>>  # CONFIG_DEBUG_ALIGN_RODATA is not set
>>
>> Before:
>>
>> ---[ Kernel Mapping ]---
>> 0x80000000-0x80100000           1M     RW NX SHD
>> 0x80100000-0x80a00000           9M     ro x  SHD
>> 0x80a00000-0xa0000000         502M     RW NX SHD
>>
>> After:
>>
>> ---[ Kernel Mapping ]---
>> 0x80000000-0x80100000           1M     RW NX SHD
>> 0x80100000-0x80700000           6M     ro x  SHD
>> 0x80700000-0x80a00000           3M     ro NX SHD
>> 0x80a00000-0xa0000000         502M     RW NX SHD
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> v2:
>> - static declaration, ard
>> ---
>>  arch/arm/kernel/vmlinux.lds.S | 9 +++++++--
>>  arch/arm/mm/init.c            | 7 ++++---
>>  2 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>> index a6e395c53a48..9c249c71fda1 100644
>> --- a/arch/arm/kernel/vmlinux.lds.S
>> +++ b/arch/arm/kernel/vmlinux.lds.S
>> @@ -8,9 +8,7 @@
>>  #include <asm/thread_info.h>
>>  #include <asm/memory.h>
>>  #include <asm/page.h>
>> -#ifdef CONFIG_DEBUG_RODATA
>>  #include <asm/pgtable.h>
>> -#endif
>>
>>  #define PROC_INFO                                                      \
>>         . = ALIGN(4);                                                   \
>> @@ -337,6 +335,13 @@ SECTIONS
>>  }
>>
>>  /*
>> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
>> + * be the first section-aligned location after __start_rodata. Otherwise,
>> + * it will be equal to __start_rodata.
>> + */
>> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
>> +
>> +/*
>>   * These must never be empty
>>   * If you have to comment these two assert statements out, your
>>   * binutils is too old (for other reasons as well)
>> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
>> index 321d3683dc7c..6b16f6cf4843 100644
>> --- a/arch/arm/mm/init.c
>> +++ b/arch/arm/mm/init.c
>> @@ -579,6 +579,9 @@ struct section_perm {
>>         pmdval_t clear;
>>  };
>>
>> +/* First section-aligned location at or after __start_rodata. */
>> +extern char __start_rodata_section_aligned[];
>> +
>>  static struct section_perm nx_perms[] = {
>>         /* Make pages tables, etc before _stext RW (set NX). */
>>         {
>> @@ -596,16 +599,14 @@ static struct section_perm nx_perms[] = {
>>                 .mask   = ~PMD_SECT_XN,
>>                 .prot   = PMD_SECT_XN,
>>         },
>> -#ifdef CONFIG_DEBUG_ALIGN_RODATA
>>         /* Make rodata NX (set RO in ro_perms below). */
>>         {
>>                 .name   = "rodata NX",
>> -               .start  = (unsigned long)__start_rodata,
>> +               .start  = (unsigned long)__start_rodata_section_aligned,
>>                 .end    = (unsigned long)__init_begin,
>
> What happens if start > end ?

It isn't possible, since rodata will be after text and before
init_begin, so it must either less than or equal to init_begin. The
equal case is quite possible, and that's fine, since both cases are
(correctly) silently ignored by set_section_perms:

                for (addr = perms[i].start; addr < perms[i].end; addr
+= SECTION_SIZE)
                    section_update(....

-Kees

>
>>                 .mask   = ~PMD_SECT_XN,
>>                 .prot   = PMD_SECT_XN,
>>         },
>> -#endif
>>  };
>>
>>  static struct section_perm ro_perms[] = {
>> --
>> 1.9.1
>>
>>
>> --
>> Kees Cook
>> Chrome OS & Brillo Security



-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2015-12-08 18:38   ` Kees Cook
@ 2016-01-22 21:19     ` Kees Cook
  2016-01-22 22:08       ` Ard Biesheuvel
  2016-02-11 18:00       ` Russell King - ARM Linux
  0 siblings, 2 replies; 8+ messages in thread
From: Kees Cook @ 2016-01-22 21:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 8, 2015 at 10:38 AM, Kees Cook <keescook@chromium.org> wrote:
> On Mon, Dec 7, 2015 at 11:47 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
>>> When rodata is large enough that it crosses a section boundary after the
>>> kernel text, mark the rest NX. This is as close to full NX of rodata as
>>> we can get without splitting page tables or doing section alignment via
>>> CONFIG_DEBUG_ALIGN_RODATA.
>>>
>>> When the config is:
>>>
>>>  CONFIG_DEBUG_RODATA=y
>>>  # CONFIG_DEBUG_ALIGN_RODATA is not set
>>>
>>> Before:
>>>
>>> ---[ Kernel Mapping ]---
>>> 0x80000000-0x80100000           1M     RW NX SHD
>>> 0x80100000-0x80a00000           9M     ro x  SHD
>>> 0x80a00000-0xa0000000         502M     RW NX SHD
>>>
>>> After:
>>>
>>> ---[ Kernel Mapping ]---
>>> 0x80000000-0x80100000           1M     RW NX SHD
>>> 0x80100000-0x80700000           6M     ro x  SHD
>>> 0x80700000-0x80a00000           3M     ro NX SHD
>>> 0x80a00000-0xa0000000         502M     RW NX SHD
>>>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>> v2:
>>> - static declaration, ard
>>> ---
>>>  arch/arm/kernel/vmlinux.lds.S | 9 +++++++--
>>>  arch/arm/mm/init.c            | 7 ++++---
>>>  2 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>>> index a6e395c53a48..9c249c71fda1 100644
>>> --- a/arch/arm/kernel/vmlinux.lds.S
>>> +++ b/arch/arm/kernel/vmlinux.lds.S
>>> @@ -8,9 +8,7 @@
>>>  #include <asm/thread_info.h>
>>>  #include <asm/memory.h>
>>>  #include <asm/page.h>
>>> -#ifdef CONFIG_DEBUG_RODATA
>>>  #include <asm/pgtable.h>
>>> -#endif
>>>
>>>  #define PROC_INFO                                                      \
>>>         . = ALIGN(4);                                                   \
>>> @@ -337,6 +335,13 @@ SECTIONS
>>>  }
>>>
>>>  /*
>>> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
>>> + * be the first section-aligned location after __start_rodata. Otherwise,
>>> + * it will be equal to __start_rodata.
>>> + */
>>> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
>>> +
>>> +/*
>>>   * These must never be empty
>>>   * If you have to comment these two assert statements out, your
>>>   * binutils is too old (for other reasons as well)
>>> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
>>> index 321d3683dc7c..6b16f6cf4843 100644
>>> --- a/arch/arm/mm/init.c
>>> +++ b/arch/arm/mm/init.c
>>> @@ -579,6 +579,9 @@ struct section_perm {
>>>         pmdval_t clear;
>>>  };
>>>
>>> +/* First section-aligned location at or after __start_rodata. */
>>> +extern char __start_rodata_section_aligned[];
>>> +
>>>  static struct section_perm nx_perms[] = {
>>>         /* Make pages tables, etc before _stext RW (set NX). */
>>>         {
>>> @@ -596,16 +599,14 @@ static struct section_perm nx_perms[] = {
>>>                 .mask   = ~PMD_SECT_XN,
>>>                 .prot   = PMD_SECT_XN,
>>>         },
>>> -#ifdef CONFIG_DEBUG_ALIGN_RODATA
>>>         /* Make rodata NX (set RO in ro_perms below). */
>>>         {
>>>                 .name   = "rodata NX",
>>> -               .start  = (unsigned long)__start_rodata,
>>> +               .start  = (unsigned long)__start_rodata_section_aligned,
>>>                 .end    = (unsigned long)__init_begin,
>>
>> What happens if start > end ?
>
> It isn't possible, since rodata will be after text and before
> init_begin, so it must either less than or equal to init_begin. The
> equal case is quite possible, and that's fine, since both cases are
> (correctly) silently ignored by set_section_perms:
>
>                 for (addr = perms[i].start; addr < perms[i].end; addr
> += SECTION_SIZE)
>                     section_update(....
>
> -Kees

Hi Ard,

Is this v2 ok? I'd like to have your Ack before sending it to the patch tracker.

Thanks!

-Kees

>
>>
>>>                 .mask   = ~PMD_SECT_XN,
>>>                 .prot   = PMD_SECT_XN,
>>>         },
>>> -#endif
>>>  };
>>>
>>>  static struct section_perm ro_perms[] = {
>>> --
>>> 1.9.1
>>>
>>>
>>> --
>>> Kees Cook
>>> Chrome OS & Brillo Security
>
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security



-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2016-01-22 21:19     ` Kees Cook
@ 2016-01-22 22:08       ` Ard Biesheuvel
  2016-02-11 18:00       ` Russell King - ARM Linux
  1 sibling, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2016-01-22 22:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 22 January 2016 at 22:19, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Dec 8, 2015 at 10:38 AM, Kees Cook <keescook@chromium.org> wrote:
>> On Mon, Dec 7, 2015 at 11:47 PM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
>>>> When rodata is large enough that it crosses a section boundary after the
>>>> kernel text, mark the rest NX. This is as close to full NX of rodata as
>>>> we can get without splitting page tables or doing section alignment via
>>>> CONFIG_DEBUG_ALIGN_RODATA.
>>>>
>>>> When the config is:
>>>>
>>>>  CONFIG_DEBUG_RODATA=y
>>>>  # CONFIG_DEBUG_ALIGN_RODATA is not set
>>>>
>>>> Before:
>>>>
>>>> ---[ Kernel Mapping ]---
>>>> 0x80000000-0x80100000           1M     RW NX SHD
>>>> 0x80100000-0x80a00000           9M     ro x  SHD
>>>> 0x80a00000-0xa0000000         502M     RW NX SHD
>>>>
>>>> After:
>>>>
>>>> ---[ Kernel Mapping ]---
>>>> 0x80000000-0x80100000           1M     RW NX SHD
>>>> 0x80100000-0x80700000           6M     ro x  SHD
>>>> 0x80700000-0x80a00000           3M     ro NX SHD
>>>> 0x80a00000-0xa0000000         502M     RW NX SHD
>>>>
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>> ---
>>>> v2:
>>>> - static declaration, ard
>>>> ---
>>>>  arch/arm/kernel/vmlinux.lds.S | 9 +++++++--
>>>>  arch/arm/mm/init.c            | 7 ++++---
>>>>  2 files changed, 11 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>>>> index a6e395c53a48..9c249c71fda1 100644
>>>> --- a/arch/arm/kernel/vmlinux.lds.S
>>>> +++ b/arch/arm/kernel/vmlinux.lds.S
>>>> @@ -8,9 +8,7 @@
>>>>  #include <asm/thread_info.h>
>>>>  #include <asm/memory.h>
>>>>  #include <asm/page.h>
>>>> -#ifdef CONFIG_DEBUG_RODATA
>>>>  #include <asm/pgtable.h>
>>>> -#endif
>>>>
>>>>  #define PROC_INFO                                                      \
>>>>         . = ALIGN(4);                                                   \
>>>> @@ -337,6 +335,13 @@ SECTIONS
>>>>  }
>>>>
>>>>  /*
>>>> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
>>>> + * be the first section-aligned location after __start_rodata. Otherwise,
>>>> + * it will be equal to __start_rodata.
>>>> + */
>>>> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
>>>> +
>>>> +/*
>>>>   * These must never be empty
>>>>   * If you have to comment these two assert statements out, your
>>>>   * binutils is too old (for other reasons as well)
>>>> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
>>>> index 321d3683dc7c..6b16f6cf4843 100644
>>>> --- a/arch/arm/mm/init.c
>>>> +++ b/arch/arm/mm/init.c
>>>> @@ -579,6 +579,9 @@ struct section_perm {
>>>>         pmdval_t clear;
>>>>  };
>>>>
>>>> +/* First section-aligned location at or after __start_rodata. */
>>>> +extern char __start_rodata_section_aligned[];
>>>> +
>>>>  static struct section_perm nx_perms[] = {
>>>>         /* Make pages tables, etc before _stext RW (set NX). */
>>>>         {
>>>> @@ -596,16 +599,14 @@ static struct section_perm nx_perms[] = {
>>>>                 .mask   = ~PMD_SECT_XN,
>>>>                 .prot   = PMD_SECT_XN,
>>>>         },
>>>> -#ifdef CONFIG_DEBUG_ALIGN_RODATA
>>>>         /* Make rodata NX (set RO in ro_perms below). */
>>>>         {
>>>>                 .name   = "rodata NX",
>>>> -               .start  = (unsigned long)__start_rodata,
>>>> +               .start  = (unsigned long)__start_rodata_section_aligned,
>>>>                 .end    = (unsigned long)__init_begin,
>>>
>>> What happens if start > end ?
>>
>> It isn't possible, since rodata will be after text and before
>> init_begin, so it must either less than or equal to init_begin. The
>> equal case is quite possible, and that's fine, since both cases are
>> (correctly) silently ignored by set_section_perms:
>>
>>                 for (addr = perms[i].start; addr < perms[i].end; addr
>> += SECTION_SIZE)
>>                     section_update(....
>>
>> -Kees
>
> Hi Ard,
>
> Is this v2 ok? I'd like to have your Ack before sending it to the patch tracker.
>

Looks fine to me

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2016-01-22 21:19     ` Kees Cook
  2016-01-22 22:08       ` Ard Biesheuvel
@ 2016-02-11 18:00       ` Russell King - ARM Linux
  2016-02-12  9:45         ` Arnd Bergmann
  1 sibling, 1 reply; 8+ messages in thread
From: Russell King - ARM Linux @ 2016-02-11 18:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 22, 2016 at 01:19:03PM -0800, Kees Cook wrote:
> On Tue, Dec 8, 2015 at 10:38 AM, Kees Cook <keescook@chromium.org> wrote:
> > On Mon, Dec 7, 2015 at 11:47 PM, Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> >> On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
> >>>  /*
> >>> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
> >>> + * be the first section-aligned location after __start_rodata. Otherwise,
> >>> + * it will be equal to __start_rodata.
> >>> + */
> >>> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
...

I'm afraid this causes build errors on two configurations:

./arch/arm/kernel/vmlinux.lds:701: undefined symbol `SECTION_SHIFT' referenced in expression

which I think is down to the new __start_rodata_section_aligned line.

Olof's builder errored out with this for both allnoconfig and
lpc18xx_defconfig builds.  Both probably due to CONFIG_MMU being
unset, and hence no definition for SECTION_SHIFT.

I think you need to make this conditional on CONFIG_MMU.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2016-02-11 18:00       ` Russell King - ARM Linux
@ 2016-02-12  9:45         ` Arnd Bergmann
  2016-02-15  8:46           ` Geert Uytterhoeven
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2016-02-12  9:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday 11 February 2016 18:00:32 Russell King - ARM Linux wrote:
> On Fri, Jan 22, 2016 at 01:19:03PM -0800, Kees Cook wrote:
> > On Tue, Dec 8, 2015 at 10:38 AM, Kees Cook <keescook@chromium.org> wrote:
> > > On Mon, Dec 7, 2015 at 11:47 PM, Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org> wrote:
> > >> On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
> > >>>  /*
> > >>> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
> > >>> + * be the first section-aligned location after __start_rodata. Otherwise,
> > >>> + * it will be equal to __start_rodata.
> > >>> + */
> > >>> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
> ...
> 
> I'm afraid this causes build errors on two configurations:
> 
> ./arch/arm/kernel/vmlinux.lds:701: undefined symbol `SECTION_SHIFT' referenced in expression
> 
> which I think is down to the new __start_rodata_section_aligned line.
> 
> Olof's builder errored out with this for both allnoconfig and
> lpc18xx_defconfig builds.  Both probably due to CONFIG_MMU being
> unset, and hence no definition for SECTION_SHIFT.
> 
> I think you need to make this conditional on CONFIG_MMU.

I've hit another problem with this patch, when using XIP_KERNEL, I now see

arch/arm/mm/built-in.o:(.data+0x4bc): undefined reference to `__start_rodata_section_aligned'

in linux-next, apparently because of the combination with "ARM: 8513/1: xip: Move
XIP linking to a separate file".

	Arnd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX
  2016-02-12  9:45         ` Arnd Bergmann
@ 2016-02-15  8:46           ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2016-02-15  8:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 12, 2016 at 10:45 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 11 February 2016 18:00:32 Russell King - ARM Linux wrote:
>> On Fri, Jan 22, 2016 at 01:19:03PM -0800, Kees Cook wrote:
>> > On Tue, Dec 8, 2015 at 10:38 AM, Kees Cook <keescook@chromium.org> wrote:
>> > > On Mon, Dec 7, 2015 at 11:47 PM, Ard Biesheuvel
>> > > <ard.biesheuvel@linaro.org> wrote:
>> > >> On 7 December 2015 at 23:35, Kees Cook <keescook@chromium.org> wrote:
>> > >>>  /*
>> > >>> + * Without CONFIG_DEBUG_ALIGN_RODATA, __start_rodata_section_aligned will
>> > >>> + * be the first section-aligned location after __start_rodata. Otherwise,
>> > >>> + * it will be equal to __start_rodata.
>> > >>> + */
>> > >>> +__start_rodata_section_aligned = ALIGN(__start_rodata, 1 << SECTION_SHIFT);
>> ...
>>
>> I'm afraid this causes build errors on two configurations:
>>
>> ./arch/arm/kernel/vmlinux.lds:701: undefined symbol `SECTION_SHIFT' referenced in expression
>>
>> which I think is down to the new __start_rodata_section_aligned line.
>>
>> Olof's builder errored out with this for both allnoconfig and
>> lpc18xx_defconfig builds.  Both probably due to CONFIG_MMU being
>> unset, and hence no definition for SECTION_SHIFT.
>>
>> I think you need to make this conditional on CONFIG_MMU.
>
> I've hit another problem with this patch, when using XIP_KERNEL, I now see
>
> arch/arm/mm/built-in.o:(.data+0x4bc): undefined reference to `__start_rodata_section_aligned'
>
> in linux-next, apparently because of the combination with "ARM: 8513/1: xip: Move
> XIP linking to a separate file".

Is this due to the copy not taking into account the recent changes to
vmlinux.lds.S?

"git show 538bf4694898b19e" with copy-rename detection shows:

copy from arch/arm/kernel/vmlinux.lds.S
copy to arch/arm/kernel/vmlinux-xip.lds.S
index a6e395c53a48e045..6f59ef290289e078 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S

[...]

@@ -117,7 +114,7 @@ SECTIONS
                        ARM_CPU_KEEP(PROC_INFO)
        }

-#ifdef CONFIG_DEBUG_ALIGN_RODATA
+#ifdef CONFIG_DEBUG_RODATA
        . = ALIGN(1<<SECTION_SHIFT);
 #endif
        RO_DATA(PAGE_SIZE)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-02-15  8:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-07 22:35 [PATCH v2] ARM: mm: mark section-aligned portion of rodata NX Kees Cook
2015-12-08  7:47 ` Ard Biesheuvel
2015-12-08 18:38   ` Kees Cook
2016-01-22 21:19     ` Kees Cook
2016-01-22 22:08       ` Ard Biesheuvel
2016-02-11 18:00       ` Russell King - ARM Linux
2016-02-12  9:45         ` Arnd Bergmann
2016-02-15  8:46           ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).