Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1584096 > unrolled thread

[RFC 1/7] arch: add __ro_mostly_after_init section marker

Back to article view | Back to linux.kernel

Contents

  [RFC 1/7] arch: add __ro_mostly_after_init section marker Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-19 11:10 +0100
    [RFC 7/7] arm64: map seperately rodata sections for __ro_mostly_after_init section Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-19 11:10 +0100
      Re: [RFC 7/7] arm64: map seperately rodata sections for  __ro_mostly_after_init section Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-19 12:50 +0100
        Re: [RFC 7/7] arm64: map seperately rodata sections for  __ro_mostly_after_init section Mark Rutland <mark.rutland@arm.com> - 2017-02-20 13:50 +0100
          Re: [RFC 7/7] arm64: map seperately rodata sections for  __ro_mostly_after_init section Kees Cook <keescook@chromium.org> - 2017-02-21 21:40 +0100
    [RFC 5/7] cpu: mark ro_mostly_after_init for cpuhp_ap/bp_states Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-19 11:10 +0100
      Re: [RFC 5/7] cpu: mark ro_mostly_after_init for cpuhp_ap/bp_states Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2017-02-20 09:30 +0100
        Re: [RFC 5/7] cpu: mark ro_mostly_after_init for cpuhp_ap/bp_states Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-02-21 06:50 +0100
    [RFC 4/7] selinux: mark __ro_mostly_after_init for selinux_hooks/selinux_nf_ops Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-19 11:10 +0100
      Re: [RFC 4/7] selinux: mark __ro_mostly_after_init for  selinux_hooks/selinux_nf_ops Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-02-21 11:40 +0100
    [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-19 11:10 +0100
      Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for  __ro_mostly_after_init during module_init/exit Mark Rutland <mark.rutland@arm.com> - 2017-02-20 11:40 +0100
        Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for  __ro_mostly_after_init during module_init/exit Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-02-21 14:40 +0100
          Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for  __ro_mostly_after_init during module_init/exit Mark Rutland <mark.rutland@arm.com> - 2017-02-21 15:00 +0100
            Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-22 14:50 +0100
    [RFC 6/7] arm64: add __map_kernel_segment to accept additional vm flags Hoeun Ryu <hoeun.ryu@gmail.com> - 2017-02-19 11:10 +0100
      Re: [RFC 6/7] arm64: add __map_kernel_segment to accept additional vm flags Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-19 12:30 +0100
    Re: [kernel-hardening] [RFC 1/7] arch: add __ro_mostly_after_init  section marker Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-19 12:30 +0100
      Re: [kernel-hardening] [RFC 1/7] arch: add __ro_mostly_after_init  section marker Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-02-21 07:40 +0100

#1584096 — [RFC 1/7] arch: add __ro_mostly_after_init section marker

 After `__ro_after_init` marker is included in kernel, many kernel data
objects can be read-only-after-init. But there are many other places that
would be good to read-only-after-init but `__ro_after_init` can not be simply
applicable to them because they should be writable at some points, which are
during module_init/exit or dynamic de/registration for a specific subsystem.
 `__ro_mostly_after_init` is basically the same to `__ro_after_init`. The
section is mapped as read-only after kernel init. The different thing is
this section is temporarily mapped as read-write during module_init/exit and
de/registration of a subsystem using set_ro_mostly_after_init_rw/ro pair.
 Use `__ro_mostly_after_init` as a way to mark such memory instead when
`__ro_after_init` is not applicable because the memory should be writable
at the described points of time. They are read-only right after kernel init
and writable temporarily only during module_init/exit and dynamic
de/registration for a subsystem.

Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
---
 include/asm-generic/sections.h    |  1 +
 include/asm-generic/vmlinux.lds.h | 10 ++++++++++
 include/linux/cache.h             | 11 +++++++++++
 3 files changed, 22 insertions(+)

diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 4df64a1..16a6f21 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -34,6 +34,7 @@ extern char __bss_start[], __bss_stop[];
 extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
 extern char __start_data_ro_after_init[], __end_data_ro_after_init[];
+extern char __start_data_ro_mostly_after_init[], __end_data_ro_mostly_after_init[];
 extern char _end[];
 extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[];
 extern char __kprobes_text_start[], __kprobes_text_end[];
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 4e09b28..cc5f44e 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -265,6 +265,15 @@
 	__end_data_ro_after_init = .;
 #endif
 
+#ifndef RO_MOSTLY_AFTER_INIT_DATA
+#define RO_MOSTLY_AFTER_INIT_DATA(align)				\
+	. = ALIGN(align);						\
+	VMLINUX_SYMBOL(__start_data_ro_mostly_after_init) = .;		\
+	*(.data..ro_mostly_after_init)					\
+	. = ALIGN(align);						\
+	VMLINUX_SYMBOL(__end_data_ro_mostly_after_init) = .;
+#endif
+
 /*
  * Read only Data
  */
@@ -275,6 +284,7 @@
 		*(.rodata) *(.rodata.*)					\
 		RO_AFTER_INIT_DATA	/* Read only after init */	\
 		KEEP(*(__vermagic))	/* Kernel version magic */	\
+		RO_MOSTLY_AFTER_INIT_DATA(align)			\
 		. = ALIGN(8);						\
 		VMLINUX_SYMBOL(__start___tracepoints_ptrs) = .;		\
 		KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
diff --git a/include/linux/cache.h b/include/linux/cache.h
index 1be04f8..fd1cb9b 100644
--- a/include/linux/cache.h
+++ b/include/linux/cache.h
@@ -30,6 +30,17 @@
 #define __ro_after_init __attribute__((__section__(".data..ro_after_init")))
 #endif
 
+/*
+ * __ro_mostly_after_init is almost like __ro_after_init.
+ * but __ro_mostly_after_init section is temporarily writable only during
+ * module_init/exit or dynamic de/registeration of a subsystem using
+ * set_ro_mostly_after_init_rw/ro pair.
+ */
+#ifndef __ro_mostly_after_init
+#define __ro_mostly_after_init \
+	__attribute__((__section__(".data..ro_mostly_after_init")))
+#endif
+
 #ifndef ____cacheline_aligned
 #define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
 #endif
-- 
2.7.4

[toc] | [next] | [standalone]

#1584097 — [RFC 7/7] arm64: map seperately rodata sections for __ro_mostly_after_init section

Map rodata sections seperately for the new __ro_mostly_after_init section.
Attribute of memory for __ro_mostly_after_init section can be changed later
so we need a dedicated vmalloced region for set_memory_rw/ro api.

Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
---
 arch/arm64/mm/mmu.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 91271b1..4a89a2e 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -434,8 +434,22 @@ void mark_rodata_ro(void)
 	 * mark .rodata as read only. Use __init_begin rather than __end_rodata
 	 * to cover NOTES and EXCEPTION_TABLE.
 	 */
-	section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
-	create_mapping_late(__pa_symbol(__start_rodata), (unsigned long)__start_rodata,
+	section_size = (unsigned long)__start_data_ro_mostly_after_init -
+		(unsigned long)__start_rodata;
+	create_mapping_late(__pa_symbol(__start_rodata),
+			    (unsigned long)__start_rodata,
+			    section_size, PAGE_KERNEL_RO);
+
+	section_size = (unsigned long)__end_data_ro_mostly_after_init -
+		(unsigned long)__start_data_ro_mostly_after_init;
+	create_mapping_late(__pa_symbol(__start_data_ro_mostly_after_init),
+			    (unsigned long)__start_data_ro_mostly_after_init,
+			    section_size, PAGE_KERNEL_RO);
+
+	section_size = (unsigned long)__init_begin -
+		(unsigned long)__end_data_ro_mostly_after_init;
+	create_mapping_late(__pa_symbol(__end_data_ro_mostly_after_init),
+			    (unsigned long)__end_data_ro_mostly_after_init,
 			    section_size, PAGE_KERNEL_RO);
 
 	/* flush the TLBs after updating live kernel mappings */
@@ -478,10 +492,18 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
  */
 static void __init map_kernel(pgd_t *pgd)
 {
-	static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_init, vmlinux_data;
+	static struct vm_struct vmlinux_text, vmlinux_rodata1, vmlinux_rodata2, vmlinux_ro_mostly_after_init, vmlinux_init, vmlinux_data;
 
 	map_kernel_segment(pgd, _text, _etext, PAGE_KERNEL_EXEC, &vmlinux_text);
-	map_kernel_segment(pgd, __start_rodata, __init_begin, PAGE_KERNEL, &vmlinux_rodata);
+	map_kernel_segment(pgd, __start_rodata, __start_data_ro_mostly_after_init, PAGE_KERNEL, &vmlinux_rodata1);
+	__map_kernel_segment(pgd,
+			     __start_data_ro_mostly_after_init,
+			     __end_data_ro_mostly_after_init,
+			     PAGE_KERNEL,
+			     &vmlinux_ro_mostly_after_init,
+			     VM_MAP | VM_ALLOC);
+	map_kernel_segment(pgd, __end_data_ro_mostly_after_init, __init_begin, PAGE_KERNEL, &vmlinux_rodata2);
+
 	map_kernel_segment(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC,
 			   &vmlinux_init);
 	map_kernel_segment(pgd, _data, _end, PAGE_KERNEL, &vmlinux_data);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1584116 — Re: [RFC 7/7] arm64: map seperately rodata sections for __ro_mostly_after_init section

On 19 February 2017 at 10:04, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
> Map rodata sections seperately for the new __ro_mostly_after_init section.
> Attribute of memory for __ro_mostly_after_init section can be changed later
> so we need a dedicated vmalloced region for set_memory_rw/ro api.
>
> Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
> ---
>  arch/arm64/mm/mmu.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 91271b1..4a89a2e 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -434,8 +434,22 @@ void mark_rodata_ro(void)
>          * mark .rodata as read only. Use __init_begin rather than __end_rodata
>          * to cover NOTES and EXCEPTION_TABLE.
>          */
> -       section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
> -       create_mapping_late(__pa_symbol(__start_rodata), (unsigned long)__start_rodata,
> +       section_size = (unsigned long)__start_data_ro_mostly_after_init -
> +               (unsigned long)__start_rodata;
> +       create_mapping_late(__pa_symbol(__start_rodata),
> +                           (unsigned long)__start_rodata,
> +                           section_size, PAGE_KERNEL_RO);
> +
> +       section_size = (unsigned long)__end_data_ro_mostly_after_init -
> +               (unsigned long)__start_data_ro_mostly_after_init;
> +       create_mapping_late(__pa_symbol(__start_data_ro_mostly_after_init),
> +                           (unsigned long)__start_data_ro_mostly_after_init,
> +                           section_size, PAGE_KERNEL_RO);
> +
> +       section_size = (unsigned long)__init_begin -
> +               (unsigned long)__end_data_ro_mostly_after_init;
> +       create_mapping_late(__pa_symbol(__end_data_ro_mostly_after_init),
> +                           (unsigned long)__end_data_ro_mostly_after_init,
>                             section_size, PAGE_KERNEL_RO);
>
>         /* flush the TLBs after updating live kernel mappings */
> @@ -478,10 +492,18 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
>   */
>  static void __init map_kernel(pgd_t *pgd)
>  {
> -       static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_init, vmlinux_data;
> +       static struct vm_struct vmlinux_text, vmlinux_rodata1, vmlinux_rodata2, vmlinux_ro_mostly_after_init, vmlinux_init, vmlinux_data;
>
>         map_kernel_segment(pgd, _text, _etext, PAGE_KERNEL_EXEC, &vmlinux_text);
> -       map_kernel_segment(pgd, __start_rodata, __init_begin, PAGE_KERNEL, &vmlinux_rodata);
> +       map_kernel_segment(pgd, __start_rodata, __start_data_ro_mostly_after_init, PAGE_KERNEL, &vmlinux_rodata1);
> +       __map_kernel_segment(pgd,
> +                            __start_data_ro_mostly_after_init,
> +                            __end_data_ro_mostly_after_init,
> +                            PAGE_KERNEL,
> +                            &vmlinux_ro_mostly_after_init,
> +                            VM_MAP | VM_ALLOC);
> +       map_kernel_segment(pgd, __end_data_ro_mostly_after_init, __init_begin, PAGE_KERNEL, &vmlinux_rodata2);
> +
>         map_kernel_segment(pgd, __init_begin, __init_end, PAGE_KERNEL_EXEC,
>                            &vmlinux_init);
>         map_kernel_segment(pgd, _data, _end, PAGE_KERNEL, &vmlinux_data);
> --
> 2.7.4
>

While it is correct that you are splitting this into three separate
segments (otherwise we would not be able to change the permissions
later without risking splitting to occur), I think this leads to
unnecessary fragmentation.

If there is demand for this feature (but you still need to make the
argument for that), I wonder if it wouldn't be sufficient, and much
more straightforward, to redefine the __ro_after_init semantics to
include the kind of subsystem registration and module init context you
are targeting, and implement some hooks to temporarily lift the
__ro_after_init r/o permission restrictions in a controlled manner.

Kees: any thoughts?

[toc] | [prev] | [next] | [standalone]

#1584582 — Re: [RFC 7/7] arm64: map seperately rodata sections for __ro_mostly_after_init section

On Sun, Feb 19, 2017 at 11:35:51AM +0000, Ard Biesheuvel wrote:
> On 19 February 2017 at 10:04, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
> > Map rodata sections seperately for the new __ro_mostly_after_init section.
> > Attribute of memory for __ro_mostly_after_init section can be changed later
> > so we need a dedicated vmalloced region for set_memory_rw/ro api.

> While it is correct that you are splitting this into three separate
> segments (otherwise we would not be able to change the permissions
> later without risking splitting to occur), I think this leads to
> unnecessary fragmentation.
> 
> If there is demand for this feature (but you still need to make the
> argument for that), I wonder if it wouldn't be sufficient, and much
> more straightforward, to redefine the __ro_after_init semantics to
> include the kind of subsystem registration and module init context you
> are targeting, and implement some hooks to temporarily lift the
> __ro_after_init r/o permission restrictions in a controlled manner.

From a look over the series, I think this is just __write_rarely in
disguise. I personally think that we should keep __write_rarely and
__ro_after_init separate, the later being a strictly one-shot affair.

I had some ideas [1] as to how we could implement __write_rarely without
carving up the kernel mapping further (and keeping the RW permissions
local to the thread needing it), but I have not had the time to look
into that further.

Thanks,
Mark.

[1] http://www.openwall.com/lists/kernel-hardening/2016/11/18/3

[toc] | [prev] | [next] | [standalone]

#1585723 — Re: [RFC 7/7] arm64: map seperately rodata sections for __ro_mostly_after_init section

On Mon, Feb 20, 2017 at 4:45 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Sun, Feb 19, 2017 at 11:35:51AM +0000, Ard Biesheuvel wrote:
>> On 19 February 2017 at 10:04, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
>> > Map rodata sections seperately for the new __ro_mostly_after_init section.
>> > Attribute of memory for __ro_mostly_after_init section can be changed later
>> > so we need a dedicated vmalloced region for set_memory_rw/ro api.
>
>> While it is correct that you are splitting this into three separate
>> segments (otherwise we would not be able to change the permissions
>> later without risking splitting to occur), I think this leads to
>> unnecessary fragmentation.
>>
>> If there is demand for this feature (but you still need to make the
>> argument for that), I wonder if it wouldn't be sufficient, and much
>> more straightforward, to redefine the __ro_after_init semantics to
>> include the kind of subsystem registration and module init context you
>> are targeting, and implement some hooks to temporarily lift the
>> __ro_after_init r/o permission restrictions in a controlled manner.
>
> From a look over the series, I think this is just __write_rarely in
> disguise. I personally think that we should keep __write_rarely and
> __ro_after_init separate, the later being a strictly one-shot affair.

That's my thinking too.

> I had some ideas [1] as to how we could implement __write_rarely without
> carving up the kernel mapping further (and keeping the RW permissions
> local to the thread needing it), but I have not had the time to look
> into that further.

I'm working on a series to do this for x86, but I keep getting
distracted. I hope to get an RFC posted this week.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1584098 — [RFC 5/7] cpu: mark ro_mostly_after_init for cpuhp_ap/bp_states

It would be good that `__ro_mostly_after_init` is marked to cpuhp state
objects. They can not be simply marked as `__ro_after_init` because they
should be writable during module_init/exit. Now that they can be read-only
except during module_init/exit

Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
---
 kernel/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 0a5f630..12ad4c2 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1126,7 +1126,7 @@ core_initcall(cpu_hotplug_pm_sync_init);
 #endif /* CONFIG_SMP */
 
 /* Boot processor state steps */
-static struct cpuhp_step cpuhp_bp_states[] = {
+static struct cpuhp_step cpuhp_bp_states[] __ro_mostly_after_init = {
 	[CPUHP_OFFLINE] = {
 		.name			= "offline",
 		.startup.single		= NULL,
@@ -1212,7 +1212,7 @@ static struct cpuhp_step cpuhp_bp_states[] = {
 };
 
 /* Application processor state steps */
-static struct cpuhp_step cpuhp_ap_states[] = {
+static struct cpuhp_step cpuhp_ap_states[] __ro_mostly_after_init = {
 #ifdef CONFIG_SMP
 	/* Final state before CPU kills itself */
 	[CPUHP_AP_IDLE_DEAD] = {
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1584400 — Re: [RFC 5/7] cpu: mark ro_mostly_after_init for cpuhp_ap/bp_states

On 2017-02-19 19:04:08 [+0900], Hoeun Ryu wrote:
> It would be good that `__ro_mostly_after_init` is marked to cpuhp state
> objects. 
why?

Sebastian

[toc] | [prev] | [next] | [standalone]

#1585056 — Re: [RFC 5/7] cpu: mark ro_mostly_after_init for cpuhp_ap/bp_states

> On 20 Feb 2017, at 5:20 PM, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
> On 2017-02-19 19:04:08 [+0900], Hoeun Ryu wrote:
>> It would be good that `__ro_mostly_after_init` is marked to cpuhp state
>> objects. 
> why?
> 

I’m requesting for comments of a new feature called __ro_mostly_after_init section marker.
It’s similar to __ro_after_init, but the section can be writable for some point of time.
Please see the cover letter [1] and the first patch [2].

[1] : https://lkml.org/lkml/2017/2/19/29
[2] : https://lkml.org/lkml/2017/2/19/32

> Sebastian

[toc] | [prev] | [next] | [standalone]

#1584100 — [RFC 4/7] selinux: mark __ro_mostly_after_init for selinux_hooks/selinux_nf_ops

 It would be good that selinux hooks objects are marked as
`__ro_mostly_after_init`. They can not be simply marked as `__ro_after_init'
because they should be writable during selinux_disable procedure.
`__ro_mostly_after_init` section is temporarily read-write during
selinux_disable procedure via set_ro_mostly_after_init_rw/ro pair. Now that
they can be read-only except during the procedure.

Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
---
 security/selinux/hooks.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9a8f12f..64fd799 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6106,7 +6106,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer)
 
 #endif
 
-static struct security_hook_list selinux_hooks[] = {
+static struct security_hook_list selinux_hooks[] __ro_mostly_after_init = {
 	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
 	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
 	LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder),
@@ -6381,7 +6381,7 @@ security_initcall(selinux_init);
 
 #if defined(CONFIG_NETFILTER)
 
-static struct nf_hook_ops selinux_nf_ops[] = {
+static struct nf_hook_ops selinux_nf_ops[] __ro_mostly_after_init = {
 	{
 		.hook =		selinux_ipv4_postroute,
 		.pf =		NFPROTO_IPV4,
@@ -6477,13 +6477,17 @@ int selinux_disable(void)
 	selinux_disabled = 1;
 	selinux_enabled = 0;
 
+	set_ro_mostly_after_init_rw();
 	security_delete_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks));
+	set_ro_mostly_after_init_ro();
 
 	/* Try to destroy the avc node cache */
 	avc_disable();
 
 	/* Unregister netfilter hooks. */
+	set_ro_mostly_after_init_ro();
 	selinux_nf_ip_exit();
+	set_ro_mostly_after_init_rw();
 
 	/* Unregister selinuxfs. */
 	exit_sel_fs();
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1585196 — Re: [RFC 4/7] selinux: mark __ro_mostly_after_init for selinux_hooks/selinux_nf_ops

On 2017/02/19 19:04, Hoeun Ryu wrote:
>  It would be good that selinux hooks objects are marked as
> `__ro_mostly_after_init`. They can not be simply marked as `__ro_after_init'
> because they should be writable during selinux_disable procedure.
> `__ro_mostly_after_init` section is temporarily read-write during
> selinux_disable procedure via set_ro_mostly_after_init_rw/ro pair. Now that
> they can be read-only except during the procedure.
> 
> -static struct security_hook_list selinux_hooks[] = {
> +static struct security_hook_list selinux_hooks[] __ro_mostly_after_init = {

This won't work. This variable is array of "struct list_head".
You need to set same attribute to variables pointed by
"struct list_head"->next and "struct list_head"->prev .

>  	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
>  	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
>  	LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder),

[toc] | [prev] | [next] | [standalone]

#1584102 — [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit

 `__ro_mostly_after_init` is almost like `__ro_after_init`. The section is
read-only as same as `__ro_after_init` after kernel init. This patch makes
`__ro_mostly_after_init` section read-write temporarily only during
module_init/module_exit.

Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
---
 kernel/module.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 7eba6de..3b25e0e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -987,8 +987,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 
 	mutex_unlock(&module_mutex);
 	/* Final destruction now no one is using it. */
-	if (mod->exit != NULL)
+	if (mod->exit != NULL) {
+		set_ro_mostly_after_init_rw();
 		mod->exit();
+		set_ro_mostly_after_init_ro();
+	}
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_GOING, mod);
 	klp_module_going(mod);
@@ -3396,8 +3399,11 @@ static noinline int do_init_module(struct module *mod)
 
 	do_mod_ctors(mod);
 	/* Start the module */
-	if (mod->init != NULL)
+	if (mod->init != NULL) {
+		set_ro_mostly_after_init_rw();
 		ret = do_one_initcall(mod->init);
+		set_ro_mostly_after_init_ro();
+	}
 	if (ret < 0) {
 		goto fail_free_freeinit;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1584498 — Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit

On Sun, Feb 19, 2017 at 07:04:06PM +0900, Hoeun Ryu wrote:
>  `__ro_mostly_after_init` is almost like `__ro_after_init`. The section is
> read-only as same as `__ro_after_init` after kernel init. This patch makes
> `__ro_mostly_after_init` section read-write temporarily only during
> module_init/module_exit.
> 
> Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
> ---
>  kernel/module.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 7eba6de..3b25e0e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -987,8 +987,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>  
>  	mutex_unlock(&module_mutex);
>  	/* Final destruction now no one is using it. */
> -	if (mod->exit != NULL)
> +	if (mod->exit != NULL) {
> +		set_ro_mostly_after_init_rw();
>  		mod->exit();
> +		set_ro_mostly_after_init_ro();
> +	}
>  	blocking_notifier_call_chain(&module_notify_list,
>  				     MODULE_STATE_GOING, mod);
>  	klp_module_going(mod);
> @@ -3396,8 +3399,11 @@ static noinline int do_init_module(struct module *mod)
>  
>  	do_mod_ctors(mod);
>  	/* Start the module */
> -	if (mod->init != NULL)
> +	if (mod->init != NULL) {
> +		set_ro_mostly_after_init_rw();
>  		ret = do_one_initcall(mod->init);
> +		set_ro_mostly_after_init_ro();
> +	}

This looks very much like the pax_{open,close}_kernel() approach for
write-rarely data.

I think it would be better to implement a first class write-rarely
mechanism rather than trying to extend __ro_after_init to cover this
case.

As mentioned previously, I *think* we can have a generic implementation
that uses an mm to temporarily map a (thread/cpu-local) RW alias of the
data in question in what would otherwise be the user half of the address
space. Regardless, we can have a generic interface [1] that can cater
for that style of approach and/or something like ARM's domains or x86's
pkeys.

Thanks,
Mark.

[1] http://www.openwall.com/lists/kernel-hardening/2016/11/18/3

[toc] | [prev] | [next] | [standalone]

#1585336 — Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit

> On 20 Feb 2017, at 7:30 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Sun, Feb 19, 2017 at 07:04:06PM +0900, Hoeun Ryu wrote:
>> `__ro_mostly_after_init` is almost like `__ro_after_init`. The section is
>> read-only as same as `__ro_after_init` after kernel init. This patch makes
>> `__ro_mostly_after_init` section read-write temporarily only during
>> module_init/module_exit.
>> 
>> Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
>> ---
>> kernel/module.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 7eba6de..3b25e0e 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -987,8 +987,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>> 
>> 	mutex_unlock(&module_mutex);
>> 	/* Final destruction now no one is using it. */
>> -	if (mod->exit != NULL)
>> +	if (mod->exit != NULL) {
>> +		set_ro_mostly_after_init_rw();
>> 		mod->exit();
>> +		set_ro_mostly_after_init_ro();
>> +	}
>> 	blocking_notifier_call_chain(&module_notify_list,
>> 				     MODULE_STATE_GOING, mod);
>> 	klp_module_going(mod);
>> @@ -3396,8 +3399,11 @@ static noinline int do_init_module(struct module *mod)
>> 
>> 	do_mod_ctors(mod);
>> 	/* Start the module */
>> -	if (mod->init != NULL)
>> +	if (mod->init != NULL) {
>> +		set_ro_mostly_after_init_rw();
>> 		ret = do_one_initcall(mod->init);
>> +		set_ro_mostly_after_init_ro();
>> +	}
> 
> This looks very much like the pax_{open,close}_kernel() approach for
> write-rarely data.

I read the discussion [1] and I agree that __ro_mostly_after_init marker
looks very similar to __write_rarely. 

> 
> I think it would be better to implement a first class write-rarely
> mechanism rather than trying to extend __ro_after_init to cover this
> case.

I’m not extending __ro_after_init. __ro_mostly_after_init resides in the same section of rodata though.

> 
> As mentioned previously, I *think* we can have a generic implementation
> that uses an mm to temporarily map a (thread/cpu-local) RW alias of the
> data in question in what would otherwise be the user half of the address
> space. Regardless, we can have a generic interface [1] that can cater
> for that style of approach and/or something like ARM's domains or x86's
> pkeys.
> 

I’m still learning cpu/kernel architectures, It would be very thankful if you tell me more about the detail of the implementation itself.

The mm that maps temporary RW alias is like
    * special mm like idmap/init_mm which have its own page tables?
    * the page tables have the same content of page tables of init_mm’s swapper_pg_dir except for RW permissions for a specific section (let’s say __write_rarely)
    * then use switch_mm(special_rw_mm) to change the address space before the access happens to the section
    * then use switch_mm(current->mm) to change the address space to original after the access is done

And the interface itself. rare_write(__val, __val), is it a single value access interface.
I’m intending to make data in __ro_mostly_after_init section RW during multiple accesses like during module_init/exit.
and __rare_rw_map()/unmap() used in rare_write() seems to work like open/close api.

How could __rare_rw_ptr() be implemented and what happens when `__rw_var = __rare_rw_ptr(&(__var))` is done ?

However the interface will look like, Do we still need a special data section that is mapped RO in general but RW in some cases ?
if then, doesn’t __ro_mostly_after_init marker itself make sense and we still need it ?

> Thanks,
> Mark.
> 
> [1] http://www.openwall.com/lists/kernel-hardening/2016/11/18/3

[toc] | [prev] | [next] | [standalone]

#1585349 — Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit

On Tue, Feb 21, 2017 at 10:36:05PM +0900, Ho-Eun Ryu wrote:
> > On 20 Feb 2017, at 7:30 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Sun, Feb 19, 2017 at 07:04:06PM +0900, Hoeun Ryu wrote:

> >> @@ -3396,8 +3399,11 @@ static noinline int do_init_module(struct module *mod)
> >> 
> >> 	do_mod_ctors(mod);
> >> 	/* Start the module */
> >> -	if (mod->init != NULL)
> >> +	if (mod->init != NULL) {
> >> +		set_ro_mostly_after_init_rw();
> >> 		ret = do_one_initcall(mod->init);
> >> +		set_ro_mostly_after_init_ro();
> >> +	}
> > 
> > This looks very much like the pax_{open,close}_kernel() approach for
> > write-rarely data.
> 
> I read the discussion [1] and I agree that __ro_mostly_after_init marker
> looks very similar to __write_rarely. 
> 
> > I think it would be better to implement a first class write-rarely
> > mechanism rather than trying to extend __ro_after_init to cover this
> > case.
> 
> I’m not extending __ro_after_init. __ro_mostly_after_init resides in
> the same section of rodata though.

Sorry; I was confused when I wrote that email. I now understand that
you're adding a separate annotation.

> > As mentioned previously, I *think* we can have a generic implementation
> > that uses an mm to temporarily map a (thread/cpu-local) RW alias of the
> > data in question in what would otherwise be the user half of the address
> > space. Regardless, we can have a generic interface [1] that can cater
> > for that style of approach and/or something like ARM's domains or x86's
> > pkeys.
> > 
> 
> I’m still learning cpu/kernel architectures, It would be very thankful if you tell me more about the detail of the implementation itself.
> 
> The mm that maps temporary RW alias is like
>     * special mm like idmap/init_mm which have its own page tables?
>     * the page tables have the same content of page tables of
>       init_mm’s swapper_pg_dir except for RW permissions for a
>       specific section (let’s say __write_rarely)

This would be a special mm, like a user mm, that only mapped the
relevant VA(s).

That might map the relevant variable on-demand, or the mapping could
cover the whole write_rarely area.

>     * then use switch_mm(special_rw_mm) to change the address space
>       before the access happens to the section
>     * then use switch_mm(current->mm) to change the address space to
>       original after the access is done

Yes.

> And the interface itself. rare_write(__val, __val), is it a single
> value access interface.
> I’m intending to make data in __ro_mostly_after_init section RW during
> multiple accesses like during module_init/exit.
> and __rare_rw_map()/unmap() used in rare_write() seems to work like
> open/close api.

The __rare_rw_{map,unmap}() functions would map in the RW alias, but do
not necessarily change the RO alias to RW. This is why __rare_rw_ptr()
would be necessary, and is the major difference to the open/close API.

We could certainly allow several writes between a map/unmap. The key
requirement is that each write is instrumented so that it goes via the
RW alias.

> How could __rare_rw_ptr() be implemented and what happens when
> `__rw_var = __rare_rw_ptr(&(__var))` is done ?

__rare_rw_ptr() would take a pointer to the usual RO alias, and derive
its RW alias. What exactly this should do depends on how the RW alias is
implemented.

On a system using an RW mm, let's assume we place all __write_rarely
variables in a region bounded by __rare_write_begin/__rare_write_end,
and when the mm is installed place, we have an RW alias of this region
beginning at __rw_alias_start. In this case, it'd look something like:

#define __rare_rw_ptr(ptr) ({				\
	unsigned long __ptr = (unsigned long)(ptr);	\
	__ptr -= __rare_write_start;			\
	__ptr += __rw_alias_start;			\
	(typeof(ptr))__ptr;				\
})

... does that make sense?

For systems where you can freely/easily alter (local) permissions (e.g.
using ARM's domains), that can be done within __rare_rw_{map,unmap}(),
and __rare_rw_ptr can just return the original pointer.

> However the interface will look like, Do we still need a special data
> section that is mapped RO in general but RW in some cases ?

With the above, I think the usual mapping can always be RO.

> if then, doesn’t __ro_mostly_after_init marker itself make sense and
> we still need it ?

We may need a marker to bound the set of variables we wish to map in
this way.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1586153 — Re: [kernel-hardening] [RFC 3/7] module: modify memory attrs for __ro_mostly_after_init during module_init/exit

Thank you for your detailed explanation. It helped a lot for understandings.

> On Feb 21, 2017, at 10:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Tue, Feb 21, 2017 at 10:36:05PM +0900, Ho-Eun Ryu wrote:
>>> On 20 Feb 2017, at 7:30 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Sun, Feb 19, 2017 at 07:04:06PM +0900, Hoeun Ryu wrote:
> 
>>>> @@ -3396,8 +3399,11 @@ static noinline int do_init_module(struct module *mod)
>>>> 
>>>>   do_mod_ctors(mod);
>>>>   /* Start the module */
>>>> -    if (mod->init != NULL)
>>>> +    if (mod->init != NULL) {
>>>> +        set_ro_mostly_after_init_rw();
>>>>       ret = do_one_initcall(mod->init);
>>>> +        set_ro_mostly_after_init_ro();
>>>> +    }
>>> 
>>> This looks very much like the pax_{open,close}_kernel() approach for
>>> write-rarely data.
>> 
>> I read the discussion [1] and I agree that __ro_mostly_after_init marker
>> looks very similar to __write_rarely. 
>> 
>>> I think it would be better to implement a first class write-rarely
>>> mechanism rather than trying to extend __ro_after_init to cover this
>>> case.
>> 
>> I’m not extending __ro_after_init. __ro_mostly_after_init resides in
>> the same section of rodata though.
> 
> Sorry; I was confused when I wrote that email. I now understand that
> you're adding a separate annotation.
> 
>>> As mentioned previously, I *think* we can have a generic implementation
>>> that uses an mm to temporarily map a (thread/cpu-local) RW alias of the
>>> data in question in what would otherwise be the user half of the address
>>> space. Regardless, we can have a generic interface [1] that can cater
>>> for that style of approach and/or something like ARM's domains or x86's
>>> pkeys.
>> 
>> I’m still learning cpu/kernel architectures, It would be very thankful if you tell me more about the detail of the implementation itself.
>> 
>> The mm that maps temporary RW alias is like
>>   * special mm like idmap/init_mm which have its own page tables?
>>   * the page tables have the same content of page tables of
>>     init_mm’s swapper_pg_dir except for RW permissions for a
>>     specific section (let’s say __write_rarely)
> 
> This would be a special mm, like a user mm, that only mapped the
> relevant VA(s).

we need a separate mm/pgd for ttbr0_el1 in kernel image section as idmap and swapper_pg_dir currently do and we make VA alias mapping for RO section with RW permission under TASK_SIZE during kernel init. And then we can switch to the mm by setting the pgd to ttbr0_el1. Right ?

It came to my mind that how about the relationship with SW_TTBR0_PAN .
What if copy_from_user tries to do something against RW alias ?

val_rw = __rw_ptr(&val);
__rw_map();
copy_from_user(&val_rw, user_ptr);
__re_unmap();

__rw_map() will install rw_mm->gpd to ttbr0_el1 but uaccess_enable() will immediately reinstall thread_info->pgd to ttbr0_el1 and we loose RW alias.
Am I something wrong or confused ?

> 
> That might map the relevant variable on-demand, or the mapping could
> cover the whole write_rarely area.
> 
>>   * then use switch_mm(special_rw_mm) to change the address space
>>     before the access happens to the section
>>   * then use switch_mm(current->mm) to change the address space to
>>     original after the access is done
> 
> Yes.
> 
>> And the interface itself. rare_write(__val, __val), is it a single
>> value access interface.
>> I’m intending to make data in __ro_mostly_after_init section RW during
>> multiple accesses like during module_init/exit.
>> and __rare_rw_map()/unmap() used in rare_write() seems to work like
>> open/close api.
> 
> The __rare_rw_{map,unmap}() functions would map in the RW alias, but do
> not necessarily change the RO alias to RW. This is why __rare_rw_ptr()
> would be necessary, and is the major difference to the open/close API.
> 
> We could certainly allow several writes between a map/unmap. The key
> requirement is that each write is instrumented so that it goes via the
> RW alias.
> 
>> How could __rare_rw_ptr() be implemented and what happens when
>> `__rw_var = __rare_rw_ptr(&(__var))` is done ?
> 
> __rare_rw_ptr() would take a pointer to the usual RO alias, and derive
> its RW alias. What exactly this should do depends on how the RW alias is
> implemented.
> 
> On a system using an RW mm, let's assume we place all __write_rarely
> variables in a region bounded by __rare_write_begin/__rare_write_end,
> and when the mm is installed place, we have an RW alias of this region
> beginning at __rw_alias_start. In this case, it'd look something like:
> 
> #define __rare_rw_ptr(ptr) ({                \
>   unsigned long __ptr = (unsigned long)(ptr);    \
>   __ptr -= __rare_write_start;            \
>   __ptr += __rw_alias_start;            \
>   (typeof(ptr))__ptr;                \
> })
> 
> ... does that make sense?

Yes. Cool.

> 
> For systems where you can freely/easily alter (local) permissions (e.g.
> using ARM's domains), that can be done within __rare_rw_{map,unmap}(),
> and __rare_rw_ptr can just return the original pointer.
> 
>> However the interface will look like, Do we still need a special data
>> section that is mapped RO in general but RW in some cases ?
> 
> With the above, I think the usual mapping can always be RO.
> 
>> if then, doesn’t __ro_mostly_after_init marker itself make sense and
>> we still need it ?
> 
> We may need a marker to bound the set of variables we wish to map in
> this way.
> 
> Thanks,
> Mark.

[toc] | [prev] | [next] | [standalone]

#1584104 — [RFC 6/7] arm64: add __map_kernel_segment to accept additional vm flags

 Memory attribute for `__ro_mostly_after_init` section should be changed
via set_memory_rw/ro that doesn't work against vm areas which don't have
VM_ALLOC. Add this function to map `__ro_mostly_after_init` section with
VM_ALLOC flag set in map_kernel.

Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
---
 arch/arm64/mm/mmu.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index b805c01..91271b1 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -444,8 +444,10 @@ void mark_rodata_ro(void)
 	debug_checkwx();
 }
 
-static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
-				      pgprot_t prot, struct vm_struct *vma)
+static void __init __map_kernel_segment(pgd_t *pgd,
+					void *va_start, void *va_end,
+					pgprot_t prot, struct vm_struct *vma,
+					unsigned long flags)
 {
 	phys_addr_t pa_start = __pa_symbol(va_start);
 	unsigned long size = va_end - va_start;
@@ -459,12 +461,18 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
 	vma->addr	= va_start;
 	vma->phys_addr	= pa_start;
 	vma->size	= size;
-	vma->flags	= VM_MAP;
+	vma->flags	= flags;
 	vma->caller	= __builtin_return_address(0);
 
 	vm_area_add_early(vma);
 }
 
+static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
+				      pgprot_t prot, struct vm_struct *vma)
+{
+	return __map_kernel_segment(pgd, va_start, va_end, prot, vma, VM_MAP);
+}
+
 /*
  * Create fine-grained mappings for the kernel.
  */
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1584112 — Re: [RFC 6/7] arm64: add __map_kernel_segment to accept additional vm flags

On 19 February 2017 at 10:04, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
>  Memory attribute for `__ro_mostly_after_init` section should be changed
> via set_memory_rw/ro that doesn't work against vm areas which don't have
> VM_ALLOC.

This is for a good reason: VMALLOC regions are guaranteed to be mapped
down to pages, which is the only thing set_memory_rX() supports. On
arm64, kernel segments may be mapped using contiguous page mappings or
block mappings, which cannot be modified like this.

> Add this function to map `__ro_mostly_after_init` section with
> VM_ALLOC flag set in map_kernel.
>

If we add this functionality, I'd rather we have a special handler for
the __ro_mostly_after_init section and not use set_memory_rX().

> Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
> ---
>  arch/arm64/mm/mmu.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index b805c01..91271b1 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -444,8 +444,10 @@ void mark_rodata_ro(void)
>         debug_checkwx();
>  }
>
> -static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
> -                                     pgprot_t prot, struct vm_struct *vma)
> +static void __init __map_kernel_segment(pgd_t *pgd,
> +                                       void *va_start, void *va_end,
> +                                       pgprot_t prot, struct vm_struct *vma,
> +                                       unsigned long flags)
>  {
>         phys_addr_t pa_start = __pa_symbol(va_start);
>         unsigned long size = va_end - va_start;
> @@ -459,12 +461,18 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
>         vma->addr       = va_start;
>         vma->phys_addr  = pa_start;
>         vma->size       = size;
> -       vma->flags      = VM_MAP;
> +       vma->flags      = flags;
>         vma->caller     = __builtin_return_address(0);
>
>         vm_area_add_early(vma);
>  }
>
> +static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
> +                                     pgprot_t prot, struct vm_struct *vma)
> +{
> +       return __map_kernel_segment(pgd, va_start, va_end, prot, vma, VM_MAP);
> +}
> +
>  /*
>   * Create fine-grained mappings for the kernel.
>   */
> --
> 2.7.4
>

[toc] | [prev] | [next] | [standalone]

#1584114 — Re: [kernel-hardening] [RFC 1/7] arch: add __ro_mostly_after_init section marker

On 19 February 2017 at 10:04, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
>  After `__ro_after_init` marker is included in kernel, many kernel data
> objects can be read-only-after-init. But there are many other places that
> would be good to read-only-after-init but `__ro_after_init` can not be simply
> applicable to them because they should be writable at some points, which are
> during module_init/exit or dynamic de/registration for a specific subsystem.
>  `__ro_mostly_after_init` is basically the same to `__ro_after_init`. The
> section is mapped as read-only after kernel init. The different thing is
> this section is temporarily mapped as read-write during module_init/exit and
> de/registration of a subsystem using set_ro_mostly_after_init_rw/ro pair.
>  Use `__ro_mostly_after_init` as a way to mark such memory instead when
> `__ro_after_init` is not applicable because the memory should be writable
> at the described points of time. They are read-only right after kernel init
> and writable temporarily only during module_init/exit and dynamic
> de/registration for a subsystem.
>
> Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
> ---
>  include/asm-generic/sections.h    |  1 +
>  include/asm-generic/vmlinux.lds.h | 10 ++++++++++
>  include/linux/cache.h             | 11 +++++++++++
>  3 files changed, 22 insertions(+)
>
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 4df64a1..16a6f21 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -34,6 +34,7 @@ extern char __bss_start[], __bss_stop[];
>  extern char __init_begin[], __init_end[];
>  extern char _sinittext[], _einittext[];
>  extern char __start_data_ro_after_init[], __end_data_ro_after_init[];
> +extern char __start_data_ro_mostly_after_init[], __end_data_ro_mostly_after_init[];
>  extern char _end[];
>  extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[];
>  extern char __kprobes_text_start[], __kprobes_text_end[];
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 4e09b28..cc5f44e 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -265,6 +265,15 @@
>         __end_data_ro_after_init = .;
>  #endif
>
> +#ifndef RO_MOSTLY_AFTER_INIT_DATA
> +#define RO_MOSTLY_AFTER_INIT_DATA(align)                               \
> +       . = ALIGN(align);                                               \
> +       VMLINUX_SYMBOL(__start_data_ro_mostly_after_init) = .;          \
> +       *(.data..ro_mostly_after_init)                                  \
> +       . = ALIGN(align);                                               \
> +       VMLINUX_SYMBOL(__end_data_ro_mostly_after_init) = .;
> +#endif
> +
>  /*
>   * Read only Data
>   */
> @@ -275,6 +284,7 @@
>                 *(.rodata) *(.rodata.*)                                 \
>                 RO_AFTER_INIT_DATA      /* Read only after init */      \
>                 KEEP(*(__vermagic))     /* Kernel version magic */      \
> +               RO_MOSTLY_AFTER_INIT_DATA(align)                        \

You can't really drop this in the middle of a section like this. On
arm64, we try very hard to use a minimal segment alignment of 64 KB
(of 2 MB if DEBUG_ALIGN_RODATA=y), to ensure that the TLB footprint of
the kernel image is minimized.

So this should be a separate section in the arm64 linker script.

>                 . = ALIGN(8);                                           \
>                 VMLINUX_SYMBOL(__start___tracepoints_ptrs) = .;         \
>                 KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
> diff --git a/include/linux/cache.h b/include/linux/cache.h
> index 1be04f8..fd1cb9b 100644
> --- a/include/linux/cache.h
> +++ b/include/linux/cache.h
> @@ -30,6 +30,17 @@
>  #define __ro_after_init __attribute__((__section__(".data..ro_after_init")))
>  #endif
>
> +/*
> + * __ro_mostly_after_init is almost like __ro_after_init.
> + * but __ro_mostly_after_init section is temporarily writable only during
> + * module_init/exit or dynamic de/registeration of a subsystem using
> + * set_ro_mostly_after_init_rw/ro pair.
> + */
> +#ifndef __ro_mostly_after_init
> +#define __ro_mostly_after_init \
> +       __attribute__((__section__(".data..ro_mostly_after_init")))
> +#endif
> +
>  #ifndef ____cacheline_aligned
>  #define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
>  #endif
> --
> 2.7.4
>

[toc] | [prev] | [next] | [standalone]

#1585070 — Re: [kernel-hardening] [RFC 1/7] arch: add __ro_mostly_after_init section marker

> On 19 Feb 2017, at 8:24 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> On 19 February 2017 at 10:04, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
>> After `__ro_after_init` marker is included in kernel, many kernel data
>> objects can be read-only-after-init. But there are many other places that
>> would be good to read-only-after-init but `__ro_after_init` can not be simply
>> applicable to them because they should be writable at some points, which are
>> during module_init/exit or dynamic de/registration for a specific subsystem.
>> `__ro_mostly_after_init` is basically the same to `__ro_after_init`. The
>> section is mapped as read-only after kernel init. The different thing is
>> this section is temporarily mapped as read-write during module_init/exit and
>> de/registration of a subsystem using set_ro_mostly_after_init_rw/ro pair.
>> Use `__ro_mostly_after_init` as a way to mark such memory instead when
>> `__ro_after_init` is not applicable because the memory should be writable
>> at the described points of time. They are read-only right after kernel init
>> and writable temporarily only during module_init/exit and dynamic
>> de/registration for a subsystem.
>> 
>> Signed-off-by: Hoeun Ryu <hoeun.ryu@gmail.com>
>> ---
>> include/asm-generic/sections.h    |  1 +
>> include/asm-generic/vmlinux.lds.h | 10 ++++++++++
>> include/linux/cache.h             | 11 +++++++++++
>> 3 files changed, 22 insertions(+)
>> 
>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>> index 4df64a1..16a6f21 100644
>> --- a/include/asm-generic/sections.h
>> +++ b/include/asm-generic/sections.h
>> @@ -34,6 +34,7 @@ extern char __bss_start[], __bss_stop[];
>> extern char __init_begin[], __init_end[];
>> extern char _sinittext[], _einittext[];
>> extern char __start_data_ro_after_init[], __end_data_ro_after_init[];
>> +extern char __start_data_ro_mostly_after_init[], __end_data_ro_mostly_after_init[];
>> extern char _end[];
>> extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[];
>> extern char __kprobes_text_start[], __kprobes_text_end[];
>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> index 4e09b28..cc5f44e 100644
>> --- a/include/asm-generic/vmlinux.lds.h
>> +++ b/include/asm-generic/vmlinux.lds.h
>> @@ -265,6 +265,15 @@
>>        __end_data_ro_after_init = .;
>> #endif
>> 
>> +#ifndef RO_MOSTLY_AFTER_INIT_DATA
>> +#define RO_MOSTLY_AFTER_INIT_DATA(align)                               \
>> +       . = ALIGN(align);                                               \
>> +       VMLINUX_SYMBOL(__start_data_ro_mostly_after_init) = .;          \
>> +       *(.data..ro_mostly_after_init)                                  \
>> +       . = ALIGN(align);                                               \
>> +       VMLINUX_SYMBOL(__end_data_ro_mostly_after_init) = .;
>> +#endif
>> +
>> /*
>>  * Read only Data
>>  */
>> @@ -275,6 +284,7 @@
>>                *(.rodata) *(.rodata.*)                                 \
>>                RO_AFTER_INIT_DATA      /* Read only after init */      \
>>                KEEP(*(__vermagic))     /* Kernel version magic */      \
>> +               RO_MOSTLY_AFTER_INIT_DATA(align)                        \
> 
> You can't really drop this in the middle of a section like this. On
> arm64, we try very hard to use a minimal segment alignment of 64 KB
> (of 2 MB if DEBUG_ALIGN_RODATA=y), to ensure that the TLB footprint of
> the kernel image is minimized.
> 
> So this should be a separate section in the arm64 linker script.
> 

Could we achieve that using ALIGN(SECTION_SIZE) not ALIGN(PAGE_SIZE) for RO_DATA ?

>>                . = ALIGN(8);                                           \
>>                VMLINUX_SYMBOL(__start___tracepoints_ptrs) = .;         \
>>                KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
>> diff --git a/include/linux/cache.h b/include/linux/cache.h
>> index 1be04f8..fd1cb9b 100644
>> --- a/include/linux/cache.h
>> +++ b/include/linux/cache.h
>> @@ -30,6 +30,17 @@
>> #define __ro_after_init __attribute__((__section__(".data..ro_after_init")))
>> #endif
>> 
>> +/*
>> + * __ro_mostly_after_init is almost like __ro_after_init.
>> + * but __ro_mostly_after_init section is temporarily writable only during
>> + * module_init/exit or dynamic de/registeration of a subsystem using
>> + * set_ro_mostly_after_init_rw/ro pair.
>> + */
>> +#ifndef __ro_mostly_after_init
>> +#define __ro_mostly_after_init \
>> +       __attribute__((__section__(".data..ro_mostly_after_init")))
>> +#endif
>> +
>> #ifndef ____cacheline_aligned
>> #define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
>> #endif
>> --
>> 2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web