[4/8] riscv: move exception table immediately after RO_DATA
diff mbox series

Message ID 20200217083223.2011-5-zong.li@sifive.com
State Superseded
Headers show
Series
  • Support strict kernel memory permissions for security
Related show

Commit Message

Zong Li Feb. 17, 2020, 8:32 a.m. UTC
Move EXCEPTION_TABLE immediately after RO_DATA. Make it easy to set the
attribution of the sections which should be read-only at a time.
Move .sdata to indicate the start of data section with write permission.
This patch is prepared for STRICT_KERNEL_RWX support.

Signed-off-by: Zong Li <zong.li@sifive.com>
---
 arch/riscv/kernel/vmlinux.lds.S | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Palmer Dabbelt March 5, 2020, 12:57 a.m. UTC | #1
On Mon, 17 Feb 2020 00:32:19 PST (-0800), zong.li@sifive.com wrote:
> Move EXCEPTION_TABLE immediately after RO_DATA. Make it easy to set the
> attribution of the sections which should be read-only at a time.
> Move .sdata to indicate the start of data section with write permission.
> This patch is prepared for STRICT_KERNEL_RWX support.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
>  arch/riscv/kernel/vmlinux.lds.S | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index 1e0193ded420..4ba8a5397e8b 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -9,6 +9,7 @@
>  #include <asm/page.h>
>  #include <asm/cache.h>
>  #include <asm/thread_info.h>
> +#include <asm/set_memory.h>
>
>  OUTPUT_ARCH(riscv)
>  ENTRY(_start)
> @@ -52,12 +53,15 @@ SECTIONS
>  	}
>
>  	/* Start of data section */
> -	_sdata = .;
>  	RO_DATA(L1_CACHE_BYTES)
>  	.srodata : {
>  		*(.srodata*)
>  	}
>
> +	EXCEPTION_TABLE(0x10)
> +
> +	_sdata = .;
> +
>  	RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_SIZE)
>  	.sdata : {
>  		__global_pointer$ = . + 0x800;
> @@ -69,8 +73,6 @@ SECTIONS
>
>  	BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
>
> -	EXCEPTION_TABLE(0x10)
> -
>  	.rel.dyn : {
>  		*(.rel.dyn*)
>  	}

As far as I can tell this is OK: core_kernel_data() explicitly says that RODATA
may or may not be between _sdata and _edata.  That said, I think we should add
__start_rodata and __end_rodata atomicly with this change (around RO_DATA and
.srodata).
Zong Li March 5, 2020, 4:01 a.m. UTC | #2
Palmer Dabbelt <palmer@dabbelt.com> 於 2020年3月5日 週四 上午8:58寫道:
>
> On Mon, 17 Feb 2020 00:32:19 PST (-0800), zong.li@sifive.com wrote:
> > Move EXCEPTION_TABLE immediately after RO_DATA. Make it easy to set the
> > attribution of the sections which should be read-only at a time.
> > Move .sdata to indicate the start of data section with write permission.
> > This patch is prepared for STRICT_KERNEL_RWX support.
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> >  arch/riscv/kernel/vmlinux.lds.S | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 1e0193ded420..4ba8a5397e8b 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -9,6 +9,7 @@
> >  #include <asm/page.h>
> >  #include <asm/cache.h>
> >  #include <asm/thread_info.h>
> > +#include <asm/set_memory.h>
> >
> >  OUTPUT_ARCH(riscv)
> >  ENTRY(_start)
> > @@ -52,12 +53,15 @@ SECTIONS
> >       }
> >
> >       /* Start of data section */
> > -     _sdata = .;
> >       RO_DATA(L1_CACHE_BYTES)
> >       .srodata : {
> >               *(.srodata*)
> >       }
> >
> > +     EXCEPTION_TABLE(0x10)
> > +
> > +     _sdata = .;
> > +
> >       RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_SIZE)
> >       .sdata : {
> >               __global_pointer$ = . + 0x800;
> > @@ -69,8 +73,6 @@ SECTIONS
> >
> >       BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> >
> > -     EXCEPTION_TABLE(0x10)
> > -
> >       .rel.dyn : {
> >               *(.rel.dyn*)
> >       }
>
> As far as I can tell this is OK: core_kernel_data() explicitly says that RODATA
> may or may not be between _sdata and _edata.  That said, I think we should add
> __start_rodata and __end_rodata atomicly with this change (around RO_DATA and
> .srodata).
>

OK, I'll move _sdata back. Actually, here I need a symbol to specify
the start address at writable data (RW_DATA), thus, I could remove the
executable permission of .data section (from this symbol), and make
.rodata, .srodata and __ex_table read-only at a time (from
__start_rodata to this symbol). So even if we use __end_rodata to wrap
.srodata together with .rodata, exception table still be excluded, and
we have no idea where is the .data section start address. Do you think
it would be OK if we use _data to specify the start address at
writable data? If it's OK, whether we still need to add __end_rodata
after .srodata?

Patch
diff mbox series

diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index 1e0193ded420..4ba8a5397e8b 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -9,6 +9,7 @@ 
 #include <asm/page.h>
 #include <asm/cache.h>
 #include <asm/thread_info.h>
+#include <asm/set_memory.h>
 
 OUTPUT_ARCH(riscv)
 ENTRY(_start)
@@ -52,12 +53,15 @@  SECTIONS
 	}
 
 	/* Start of data section */
-	_sdata = .;
 	RO_DATA(L1_CACHE_BYTES)
 	.srodata : {
 		*(.srodata*)
 	}
 
+	EXCEPTION_TABLE(0x10)
+
+	_sdata = .;
+
 	RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_SIZE)
 	.sdata : {
 		__global_pointer$ = . + 0x800;
@@ -69,8 +73,6 @@  SECTIONS
 
 	BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
 
-	EXCEPTION_TABLE(0x10)
-
 	.rel.dyn : {
 		*(.rel.dyn*)
 	}