* [PATCH] arm64: mm: Mark .rodata as RO @ 2016-02-12 16:13 Jeremy Linton 2016-02-12 18:25 ` Mark Rutland 0 siblings, 1 reply; 5+ messages in thread From: Jeremy Linton @ 2016-02-12 16:13 UTC (permalink / raw) To: linux-arm-kernel Currently the .rodata section is actually still executable when DEBUG_RODATA is enabled. This changes that so the .rodata is actually read only, no execute. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> --- arch/arm64/kernel/vmlinux.lds.S | 5 +++-- arch/arm64/mm/mmu.c | 14 +++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index ab4e436..2e2c053 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -114,8 +114,9 @@ SECTIONS *(.got) /* Global offset table */ } - RO_DATA(PAGE_SIZE) - EXCEPTION_TABLE(8) + ALIGN_DEBUG_RO_MIN(0) + RO_DATA(PAGE_SIZE) /* everything from this point to */ + EXCEPTION_TABLE(8) /* _etext will be marked RO NX */ NOTES ALIGN_DEBUG_RO_MIN(PAGE_SIZE) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index ab69a99..a3f4112 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -453,10 +453,18 @@ static void __init map_mem(pgd_t *pgd) #ifdef CONFIG_DEBUG_RODATA void mark_rodata_ro(void) { - create_mapping_late(__pa(_stext), (unsigned long)_stext, - (unsigned long)_etext - (unsigned long)_stext, - PAGE_KERNEL_ROX); + unsigned long section_size; + section_size = (unsigned long)__start_rodata - (unsigned long)_stext; + create_mapping_late(__pa(_stext), (unsigned long)_stext, + section_size, PAGE_KERNEL_ROX); + /* + * mark .rodata as read only. Use _etext rather than __end_rodata to + * cover NOTES and EXCEPTION_TABLE. + */ + section_size = (unsigned long)_etext - (unsigned long)__start_rodata; + create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata, + section_size, PAGE_KERNEL_RO); } #endif -- 2.4.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: Mark .rodata as RO 2016-02-12 16:13 [PATCH] arm64: mm: Mark .rodata as RO Jeremy Linton @ 2016-02-12 18:25 ` Mark Rutland 2016-02-16 18:10 ` Kees Cook 0 siblings, 1 reply; 5+ messages in thread From: Mark Rutland @ 2016-02-12 18:25 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 12, 2016 at 10:13:19AM -0600, Jeremy Linton wrote: > Currently the .rodata section is actually still executable when DEBUG_RODATA > is enabled. This changes that so the .rodata is actually read only, no execute. > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > --- > arch/arm64/kernel/vmlinux.lds.S | 5 +++-- > arch/arm64/mm/mmu.c | 14 +++++++++++--- > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index ab4e436..2e2c053 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -114,8 +114,9 @@ SECTIONS > *(.got) /* Global offset table */ > } > > - RO_DATA(PAGE_SIZE) > - EXCEPTION_TABLE(8) > + ALIGN_DEBUG_RO_MIN(0) > + RO_DATA(PAGE_SIZE) /* everything from this point to */ > + EXCEPTION_TABLE(8) /* _etext will be marked RO NX */ > NOTES > > ALIGN_DEBUG_RO_MIN(PAGE_SIZE) > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index ab69a99..a3f4112 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -453,10 +453,18 @@ static void __init map_mem(pgd_t *pgd) > #ifdef CONFIG_DEBUG_RODATA > void mark_rodata_ro(void) > { > - create_mapping_late(__pa(_stext), (unsigned long)_stext, > - (unsigned long)_etext - (unsigned long)_stext, > - PAGE_KERNEL_ROX); > + unsigned long section_size; > > + section_size = (unsigned long)__start_rodata - (unsigned long)_stext; > + create_mapping_late(__pa(_stext), (unsigned long)_stext, > + section_size, PAGE_KERNEL_ROX); > + /* > + * mark .rodata as read only. Use _etext rather than __end_rodata to > + * cover NOTES and EXCEPTION_TABLE. > + */ > + section_size = (unsigned long)_etext - (unsigned long)__start_rodata; > + create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata, > + section_size, PAGE_KERNEL_RO); > } As you pointed out in the other thread, we'll also need to update map_kernel to use equivalent chunks for .text and .rodata. I think we can probably make .rodata RO from the outset in map_kernel, too. Could you please update mem_init to log .text and .rodata separately? It looks like some core code makes assumptions about _etext, so I guess that has to cover .rodata regardless. Otherwise, looks good! Thanks, Mark. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: Mark .rodata as RO 2016-02-12 18:25 ` Mark Rutland @ 2016-02-16 18:10 ` Kees Cook 2016-02-16 18:48 ` Laura Abbott 0 siblings, 1 reply; 5+ messages in thread From: Kees Cook @ 2016-02-16 18:10 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 12, 2016 at 10:25 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Feb 12, 2016 at 10:13:19AM -0600, Jeremy Linton wrote: >> Currently the .rodata section is actually still executable when DEBUG_RODATA >> is enabled. This changes that so the .rodata is actually read only, no execute. >> >> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> Yikes, good catch. Is anyone running the lkdtm tests that check these things? Reviewed-by: Kees Cook <keescook@chromium.org> >> --- >> arch/arm64/kernel/vmlinux.lds.S | 5 +++-- >> arch/arm64/mm/mmu.c | 14 +++++++++++--- >> 2 files changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S >> index ab4e436..2e2c053 100644 >> --- a/arch/arm64/kernel/vmlinux.lds.S >> +++ b/arch/arm64/kernel/vmlinux.lds.S >> @@ -114,8 +114,9 @@ SECTIONS >> *(.got) /* Global offset table */ >> } >> >> - RO_DATA(PAGE_SIZE) >> - EXCEPTION_TABLE(8) >> + ALIGN_DEBUG_RO_MIN(0) >> + RO_DATA(PAGE_SIZE) /* everything from this point to */ >> + EXCEPTION_TABLE(8) /* _etext will be marked RO NX */ >> NOTES >> >> ALIGN_DEBUG_RO_MIN(PAGE_SIZE) >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index ab69a99..a3f4112 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -453,10 +453,18 @@ static void __init map_mem(pgd_t *pgd) >> #ifdef CONFIG_DEBUG_RODATA >> void mark_rodata_ro(void) >> { >> - create_mapping_late(__pa(_stext), (unsigned long)_stext, >> - (unsigned long)_etext - (unsigned long)_stext, >> - PAGE_KERNEL_ROX); >> + unsigned long section_size; >> >> + section_size = (unsigned long)__start_rodata - (unsigned long)_stext; >> + create_mapping_late(__pa(_stext), (unsigned long)_stext, >> + section_size, PAGE_KERNEL_ROX); >> + /* >> + * mark .rodata as read only. Use _etext rather than __end_rodata to >> + * cover NOTES and EXCEPTION_TABLE. >> + */ >> + section_size = (unsigned long)_etext - (unsigned long)__start_rodata; >> + create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata, >> + section_size, PAGE_KERNEL_RO); >> } > > As you pointed out in the other thread, we'll also need to update > map_kernel to use equivalent chunks for .text and .rodata. > > I think we can probably make .rodata RO from the outset in map_kernel, > too. There's a benefit to marking it late in that .rodata can live in the same (upcoming) section as .data..ro_after_init, which is marked ro after mark_rodata_ro() to help constify more things. > Could you please update mem_init to log .text and .rodata separately? > > It looks like some core code makes assumptions about _etext, so I guess > that has to cover .rodata regardless. > > Otherwise, looks good! > > Thanks, > Mark. -Kees -- Kees Cook Chrome OS & Brillo Security ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: Mark .rodata as RO 2016-02-16 18:10 ` Kees Cook @ 2016-02-16 18:48 ` Laura Abbott 2016-02-16 20:35 ` Kees Cook 0 siblings, 1 reply; 5+ messages in thread From: Laura Abbott @ 2016-02-16 18:48 UTC (permalink / raw) To: linux-arm-kernel On 2/16/16 10:10 AM, Kees Cook wrote: > On Fri, Feb 12, 2016 at 10:25 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> On Fri, Feb 12, 2016 at 10:13:19AM -0600, Jeremy Linton wrote: >>> Currently the .rodata section is actually still executable when DEBUG_RODATA >>> is enabled. This changes that so the .rodata is actually read only, no execute. >>> >>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > > Yikes, good catch. Is anyone running the lkdtm tests that check these things? > I don't think the current lkdtm test would have caught this since the exec test is using rw data and not ro data. That test could be expanded though to include a rodata buffer as well. Thanks, Laura ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] arm64: mm: Mark .rodata as RO 2016-02-16 18:48 ` Laura Abbott @ 2016-02-16 20:35 ` Kees Cook 0 siblings, 0 replies; 5+ messages in thread From: Kees Cook @ 2016-02-16 20:35 UTC (permalink / raw) To: linux-arm-kernel On Tue, Feb 16, 2016 at 10:48 AM, Laura Abbott <laura@labbott.name> wrote: > On 2/16/16 10:10 AM, Kees Cook wrote: >> >> On Fri, Feb 12, 2016 at 10:25 AM, Mark Rutland <mark.rutland@arm.com> >> wrote: >>> >>> On Fri, Feb 12, 2016 at 10:13:19AM -0600, Jeremy Linton wrote: >>>> >>>> Currently the .rodata section is actually still executable when >>>> DEBUG_RODATA >>>> is enabled. This changes that so the .rodata is actually read only, no >>>> execute. >>>> >>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> >> >> >> Yikes, good catch. Is anyone running the lkdtm tests that check these >> things? >> > > I don't think the current lkdtm test would have caught this since the exec > test is using rw data and not ro data. That test could be expanded though > to include a rodata buffer as well. Oh, yeah, excellent point. I'll send a patch. -Kees -- Kees Cook Chrome OS & Brillo Security ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-16 20:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-12 16:13 [PATCH] arm64: mm: Mark .rodata as RO Jeremy Linton 2016-02-12 18:25 ` Mark Rutland 2016-02-16 18:10 ` Kees Cook 2016-02-16 18:48 ` Laura Abbott 2016-02-16 20:35 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox