diff mbox series

[v2,2/4] modules: Add new special vfree flags

Message ID 20181212000354.31955-3-rick.p.edgecombe@intel.com (mailing list archive)
State New, archived
Headers show
Series Don’t leave executable TLB entries to freed pages | expand

Commit Message

Edgecombe, Rick P Dec. 12, 2018, 12:03 a.m. UTC
Add new flags for handling freeing of special permissioned memory in vmalloc,
and remove places where the handling was done in module.c.

This will enable this flag for all architectures.

Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
 kernel/module.c | 43 ++++++++++++-------------------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

Comments

Nadav Amit Dec. 12, 2018, 11:40 p.m. UTC | #1
> On Dec 11, 2018, at 4:03 PM, Rick Edgecombe <rick.p.edgecombe@intel.com> wrote:
> 
> Add new flags for handling freeing of special permissioned memory in vmalloc,
> and remove places where the handling was done in module.c.
> 
> This will enable this flag for all architectures.
> 
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> kernel/module.c | 43 ++++++++++++-------------------------------
> 1 file changed, 12 insertions(+), 31 deletions(-)
> 

I count on you for merging your patch-set with mine, since clearly they
conflict.
Edgecombe, Rick P Dec. 13, 2018, 7:02 p.m. UTC | #2
On Wed, 2018-12-12 at 23:40 +0000, Nadav Amit wrote:
> > On Dec 11, 2018, at 4:03 PM, Rick Edgecombe <rick.p.edgecombe@intel.com>
> > wrote:
> > 
> > Add new flags for handling freeing of special permissioned memory in
> > vmalloc,
> > and remove places where the handling was done in module.c.
> > 
> > This will enable this flag for all architectures.
> > 
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > ---
> > kernel/module.c | 43 ++++++++++++-------------------------------
> > 1 file changed, 12 insertions(+), 31 deletions(-)
> > 
> 
> I count on you for merging your patch-set with mine, since clearly they
> conflict.
> 
Yes, I can rebase on top of yours if you omit the changes around module_memfree 
for your next version. It should fit together pretty cleanly for BPF and modules
I think. Not sure what you are planning for kprobes and ftrace.
Nadav Amit Dec. 13, 2018, 7:27 p.m. UTC | #3
> On Dec 13, 2018, at 11:02 AM, Edgecombe, Rick P <rick.p.edgecombe@intel.com> wrote:
> 
> On Wed, 2018-12-12 at 23:40 +0000, Nadav Amit wrote:
>>> On Dec 11, 2018, at 4:03 PM, Rick Edgecombe <rick.p.edgecombe@intel.com>
>>> wrote:
>>> 
>>> Add new flags for handling freeing of special permissioned memory in
>>> vmalloc,
>>> and remove places where the handling was done in module.c.
>>> 
>>> This will enable this flag for all architectures.
>>> 
>>> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>>> ---
>>> kernel/module.c | 43 ++++++++++++-------------------------------
>>> 1 file changed, 12 insertions(+), 31 deletions(-)
>> 
>> I count on you for merging your patch-set with mine, since clearly they
>> conflict.
> Yes, I can rebase on top of yours if you omit the changes around module_memfree 
> for your next version. It should fit together pretty cleanly for BPF and modules
> I think. Not sure what you are planning for kprobes and ftrace.

Are you asking after looking at the latest version of my patch-set?

Kprobes is done and ack'd. ftrace needs to be broken into two separate
changes (setting x after writing, and using text_poke interfaces), unless
Steven ack’s them. The changes introduce some overhead (3x), but I think it
is a reasonable slowdown for a debug feature.

Can you have a look at the series I’ve sent and let me know which patches
to drop? It would be best (for me) if the two series are fully merged.
Edgecombe, Rick P Dec. 13, 2018, 9:48 p.m. UTC | #4
On Thu, 2018-12-13 at 19:27 +0000, Nadav Amit wrote:
> > On Dec 13, 2018, at 11:02 AM, Edgecombe, Rick P <rick.p.edgecombe@intel.com>
> > wrote:
> > 
> > On Wed, 2018-12-12 at 23:40 +0000, Nadav Amit wrote:
> > > > On Dec 11, 2018, at 4:03 PM, Rick Edgecombe <rick.p.edgecombe@intel.com>
> > > > wrote:
> > > > 
> > > > Add new flags for handling freeing of special permissioned memory in
> > > > vmalloc,
> > > > and remove places where the handling was done in module.c.
> > > > 
> > > > This will enable this flag for all architectures.
> > > > 
> > > > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > > > ---
> > > > kernel/module.c | 43 ++++++++++++-------------------------------
> > > > 1 file changed, 12 insertions(+), 31 deletions(-)
> > > 
> > > I count on you for merging your patch-set with mine, since clearly they
> > > conflict.
> > 
> > Yes, I can rebase on top of yours if you omit the changes around
> > module_memfree 
> > for your next version. It should fit together pretty cleanly for BPF and
> > modules
> > I think. Not sure what you are planning for kprobes and ftrace.
> 
> Are you asking after looking at the latest version of my patch-set?
> 
> Kprobes is done and ack'd. ftrace needs to be broken into two separate
> changes (setting x after writing, and using text_poke interfaces), unless
> Steven ack’s them. The changes introduce some overhead (3x), but I think it
> is a reasonable slowdown for a debug feature.
> 
> Can you have a look at the series I’ve sent and let me know which patches
> to drop? It would be best (for me) if the two series are fully merged.

Looking at v7, the only extra thing could be the tweaks to the existing lines in
kprobes free_insn_page be omitted, since those lines would just be removed in my
later changes.
diff mbox series

Patch

diff --git a/kernel/module.c b/kernel/module.c
index 49a405891587..910f92b402f8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1941,11 +1941,23 @@  void module_disable_ro(const struct module *mod)
 	frob_rodata(&mod->init_layout, set_memory_rw);
 }
 
+static void module_set_vm_flags(const struct module_layout *layout)
+{
+	struct vm_struct *vm = find_vm_area(layout->base);
+
+	if (vm) {
+		vm->flags |= VM_HAS_SPECIAL_PERMS;
+		vm->flags |= VM_IMMEDIATE_UNMAP;
+	}
+}
+
 void module_enable_ro(const struct module *mod, bool after_init)
 {
 	if (!rodata_enabled)
 		return;
 
+	module_set_vm_flags(&mod->core_layout);
+	module_set_vm_flags(&mod->init_layout);
 	frob_text(&mod->core_layout, set_memory_ro);
 	frob_rodata(&mod->core_layout, set_memory_ro);
 	frob_text(&mod->init_layout, set_memory_ro);
@@ -1964,15 +1976,6 @@  static void module_enable_nx(const struct module *mod)
 	frob_writable_data(&mod->init_layout, set_memory_nx);
 }
 
-static void module_disable_nx(const struct module *mod)
-{
-	frob_rodata(&mod->core_layout, set_memory_x);
-	frob_ro_after_init(&mod->core_layout, set_memory_x);
-	frob_writable_data(&mod->core_layout, set_memory_x);
-	frob_rodata(&mod->init_layout, set_memory_x);
-	frob_writable_data(&mod->init_layout, set_memory_x);
-}
-
 /* Iterate through all modules and set each module's text as RW */
 void set_all_modules_text_rw(void)
 {
@@ -2016,23 +2019,8 @@  void set_all_modules_text_ro(void)
 	}
 	mutex_unlock(&module_mutex);
 }
-
-static void disable_ro_nx(const struct module_layout *layout)
-{
-	if (rodata_enabled) {
-		frob_text(layout, set_memory_rw);
-		frob_rodata(layout, set_memory_rw);
-		frob_ro_after_init(layout, set_memory_rw);
-	}
-	frob_rodata(layout, set_memory_x);
-	frob_ro_after_init(layout, set_memory_x);
-	frob_writable_data(layout, set_memory_x);
-}
-
 #else
-static void disable_ro_nx(const struct module_layout *layout) { }
 static void module_enable_nx(const struct module *mod) { }
-static void module_disable_nx(const struct module *mod) { }
 #endif
 
 #ifdef CONFIG_LIVEPATCH
@@ -2163,7 +2151,6 @@  static void free_module(struct module *mod)
 	mutex_unlock(&module_mutex);
 
 	/* This may be empty, but that's OK */
-	disable_ro_nx(&mod->init_layout);
 	module_arch_freeing_init(mod);
 	module_memfree(mod->init_layout.base);
 	kfree(mod->args);
@@ -2173,7 +2160,6 @@  static void free_module(struct module *mod)
 	lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
 
 	/* Finally, free the core (containing the module structure) */
-	disable_ro_nx(&mod->core_layout);
 	module_memfree(mod->core_layout.base);
 }
 
@@ -3497,7 +3483,6 @@  static noinline int do_init_module(struct module *mod)
 #endif
 	module_enable_ro(mod, true);
 	mod_tree_remove_init(mod);
-	disable_ro_nx(&mod->init_layout);
 	module_arch_freeing_init(mod);
 	mod->init_layout.base = NULL;
 	mod->init_layout.size = 0;
@@ -3812,10 +3797,6 @@  static int load_module(struct load_info *info, const char __user *uargs,
 	module_bug_cleanup(mod);
 	mutex_unlock(&module_mutex);
 
-	/* we can't deallocate the module until we clear memory protection */
-	module_disable_ro(mod);
-	module_disable_nx(mod);
-
  ddebug_cleanup:
 	ftrace_release_mod(mod);
 	dynamic_debug_remove(mod, info->debug);