Kernel-hardening archive on lore.kernel.org
 help / color / Atom feed
* [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
@ 2019-11-19 15:51 Tianlin Li
  2019-11-19 17:07 ` Kees Cook
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Tianlin Li @ 2019-11-19 15:51 UTC (permalink / raw)
  To: kernel-hardening, keescook
  Cc: Steven Rostedt, Ingo Molnar, Russell King, Catalin Marinas,
	Will Deacon, Greentime Hu, Vincent Chen, Thomas Gleixner,
	Borislav Petkov, H . Peter Anvin, x86, Jessica Yu,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Tianlin Li

Right now several architectures allow their set_memory_*() family of 
functions to fail, but callers may not be checking the return values. We 
need to fix the callers and add the __must_check attribute. They also may
not provide any level of atomicity, in the sense that the memory 
protections may be left incomplete on failure. This issue likely has a few 
steps on effects architectures[1]:
1)Have all callers of set_memory_*() helpers check the return value.
2)Add __much_check to all set_memory_*() helpers so that new uses do not 
ignore the return value.
3)Add atomicity to the calls so that the memory protections aren't left in 
a partial state.

Ideally, the failure of set_memory_*() should be passed up the call stack, 
and callers should examine the failure and deal with it. But currently, 
some callers just have void return type.

We need to fix the callers to handle the return all the way to the top of 
stack, and it will require a large series of patches to finish all the three 
steps mentioned above. I start with kernel/module, and will move onto other 
subsystems. I am not entirely sure about the failure modes for each caller. 
So I would like to get some comments before I move forward. This single 
patch is just for fixing the return value of set_memory_*() function in 
kernel/module, and also the related callers. Any feedback would be greatly 
appreciated.

[1]:https://github.com/KSPP/linux/issues/7

Signed-off-by: Tianlin Li <tli@digitalocean.com>
---
 arch/arm/kernel/ftrace.c   |   8 +-
 arch/arm64/kernel/ftrace.c |   6 +-
 arch/nds32/kernel/ftrace.c |   6 +-
 arch/x86/kernel/ftrace.c   |  13 ++-
 include/linux/module.h     |  16 ++--
 kernel/livepatch/core.c    |  15 +++-
 kernel/module.c            | 170 +++++++++++++++++++++++++++----------
 kernel/trace/ftrace.c      |  15 +++-
 8 files changed, 175 insertions(+), 74 deletions(-)

diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index bda949fd84e8..7ea1338821d6 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -59,13 +59,15 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
 
 int ftrace_arch_code_modify_prepare(void)
 {
-	set_all_modules_text_rw();
-	return 0;
+	return set_all_modules_text_rw();
 }
 
 int ftrace_arch_code_modify_post_process(void)
 {
-	set_all_modules_text_ro();
+	int ret;
+	ret = set_all_modules_text_ro();
+	if (ret)
+		return ret;
 	/* Make sure any TLB misses during machine stop are cleared. */
 	flush_tlb_all();
 	return 0;
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 171773257974..97a89c38f6b9 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -115,9 +115,11 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 			}
 
 			/* point the trampoline to our ftrace entry point */
-			module_disable_ro(mod);
+			if (module_disable_ro(mod))
+				return -EINVAL;
 			*dst = trampoline;
-			module_enable_ro(mod, true);
+			if (module_enable_ro(mod, true))
+				return -EINVAL;
 
 			/*
 			 * Ensure updated trampoline is visible to instruction
diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c
index fd2a54b8cd57..e9e63e703a3e 100644
--- a/arch/nds32/kernel/ftrace.c
+++ b/arch/nds32/kernel/ftrace.c
@@ -91,14 +91,12 @@ int __init ftrace_dyn_arch_init(void)
 
 int ftrace_arch_code_modify_prepare(void)
 {
-	set_all_modules_text_rw();
-	return 0;
+	return set_all_modules_text_rw();
 }
 
 int ftrace_arch_code_modify_post_process(void)
 {
-	set_all_modules_text_ro();
-	return 0;
+	return set_all_modules_text_ro();
 }
 
 static unsigned long gen_sethi_insn(unsigned long addr)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 024c3053dbba..7fdee06e2a76 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -42,19 +42,26 @@ int ftrace_arch_code_modify_prepare(void)
 	 * and live kernel patching from changing the text permissions while
 	 * ftrace has it set to "read/write".
 	 */
+	int ret;
 	mutex_lock(&text_mutex);
 	set_kernel_text_rw();
-	set_all_modules_text_rw();
+	ret = set_all_modules_text_rw();
+	if (ret) {
+		set_kernel_text_ro();
+		mutex_unlock(&text_mutex);
+		return ret;
+	}
 	return 0;
 }
 
 int ftrace_arch_code_modify_post_process(void)
     __releases(&text_mutex)
 {
-	set_all_modules_text_ro();
+	int ret;
+	ret = set_all_modules_text_ro();
 	set_kernel_text_ro();
 	mutex_unlock(&text_mutex);
-	return 0;
+	return ret;
 }
 
 union ftrace_code_union {
diff --git a/include/linux/module.h b/include/linux/module.h
index 1455812dd325..e6c7f3b719a3 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -847,15 +847,15 @@ extern int module_sysfs_initialized;
 #define __MODULE_STRING(x) __stringify(x)
 
 #ifdef CONFIG_STRICT_MODULE_RWX
-extern void set_all_modules_text_rw(void);
-extern void set_all_modules_text_ro(void);
-extern void module_enable_ro(const struct module *mod, bool after_init);
-extern void module_disable_ro(const struct module *mod);
+extern int set_all_modules_text_rw(void);
+extern int set_all_modules_text_ro(void);
+extern int module_enable_ro(const struct module *mod, bool after_init);
+extern int module_disable_ro(const struct module *mod);
 #else
-static inline void set_all_modules_text_rw(void) { }
-static inline void set_all_modules_text_ro(void) { }
-static inline void module_enable_ro(const struct module *mod, bool after_init) { }
-static inline void module_disable_ro(const struct module *mod) { }
+static inline int set_all_modules_text_rw(void) { return 0; }
+static inline int set_all_modules_text_ro(void) { return 0; }
+static inline int module_enable_ro(const struct module *mod, bool after_init) { return 0; }
+static inline int module_disable_ro(const struct module *mod) { return 0; }
 #endif
 
 #ifdef CONFIG_GENERIC_BUG
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index c4ce08f43bd6..39bfc0685854 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 
 	mutex_lock(&text_mutex);
 
-	module_disable_ro(patch->mod);
+	ret = module_disable_ro(patch->mod);
+	if (ret) {
+		mutex_unlock(&text_mutex);
+		return ret;
+	}
 	ret = klp_write_object_relocations(patch->mod, obj);
 	if (ret) {
-		module_enable_ro(patch->mod, true);
+		if (module_enable_ro(patch->mod, true));
+			pr_err("module_enable_ro failed.\n");
 		mutex_unlock(&text_mutex);
 		return ret;
 	}
 
 	arch_klp_init_object_loaded(patch, obj);
-	module_enable_ro(patch->mod, true);
+	ret = module_enable_ro(patch->mod, true);
+	if (ret) {
+		mutex_unlock(&text_mutex);
+		return ret;
+	}
 
 	mutex_unlock(&text_mutex);
 
diff --git a/kernel/module.c b/kernel/module.c
index 9ee93421269c..87125b5e315c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1900,111 +1900,162 @@ static void mod_sysfs_teardown(struct module *mod)
  *
  * These values are always page-aligned (as is base)
  */
-static void frob_text(const struct module_layout *layout,
+static int frob_text(const struct module_layout *layout,
 		      int (*set_memory)(unsigned long start, int num_pages))
 {
 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base,
+	return set_memory((unsigned long)layout->base,
 		   layout->text_size >> PAGE_SHIFT);
 }
 
 #ifdef CONFIG_STRICT_MODULE_RWX
-static void frob_rodata(const struct module_layout *layout,
+static int frob_rodata(const struct module_layout *layout,
 			int (*set_memory)(unsigned long start, int num_pages))
 {
 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base + layout->text_size,
+	return set_memory((unsigned long)layout->base + layout->text_size,
 		   (layout->ro_size - layout->text_size) >> PAGE_SHIFT);
 }
 
-static void frob_ro_after_init(const struct module_layout *layout,
+static int frob_ro_after_init(const struct module_layout *layout,
 				int (*set_memory)(unsigned long start, int num_pages))
 {
 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base + layout->ro_size,
+	return set_memory((unsigned long)layout->base + layout->ro_size,
 		   (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT);
 }
 
-static void frob_writable_data(const struct module_layout *layout,
+static int frob_writable_data(const struct module_layout *layout,
 			       int (*set_memory)(unsigned long start, int num_pages))
 {
 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
 	BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1));
-	set_memory((unsigned long)layout->base + layout->ro_after_init_size,
+	return set_memory((unsigned long)layout->base + layout->ro_after_init_size,
 		   (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT);
 }
 
 /* livepatching wants to disable read-only so it can frob module. */
-void module_disable_ro(const struct module *mod)
+int module_disable_ro(const struct module *mod)
 {
+	int ret;
 	if (!rodata_enabled)
-		return;
+		return 0;
 
-	frob_text(&mod->core_layout, set_memory_rw);
-	frob_rodata(&mod->core_layout, set_memory_rw);
-	frob_ro_after_init(&mod->core_layout, set_memory_rw);
-	frob_text(&mod->init_layout, set_memory_rw);
-	frob_rodata(&mod->init_layout, set_memory_rw);
+	ret = frob_text(&mod->core_layout, set_memory_rw);
+	if (ret)
+		return ret;
+	ret = frob_rodata(&mod->core_layout, set_memory_rw);
+	if (ret)
+		return ret;
+	ret = frob_ro_after_init(&mod->core_layout, set_memory_rw);
+	if (ret)
+		return ret;
+	ret = frob_text(&mod->init_layout, set_memory_rw);
+	if (ret)
+		return ret;
+	ret = frob_rodata(&mod->init_layout, set_memory_rw);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
-void module_enable_ro(const struct module *mod, bool after_init)
+int module_enable_ro(const struct module *mod, bool after_init)
 {
+	int ret;
 	if (!rodata_enabled)
-		return;
+		return 0;
 
 	set_vm_flush_reset_perms(mod->core_layout.base);
 	set_vm_flush_reset_perms(mod->init_layout.base);
-	frob_text(&mod->core_layout, set_memory_ro);
+	ret = frob_text(&mod->core_layout, set_memory_ro);
+	if (ret)
+		return ret;
 
-	frob_rodata(&mod->core_layout, set_memory_ro);
-	frob_text(&mod->init_layout, set_memory_ro);
-	frob_rodata(&mod->init_layout, set_memory_ro);
+	ret = frob_rodata(&mod->core_layout, set_memory_ro);
+	if (ret)
+		return ret;
+	ret = frob_text(&mod->init_layout, set_memory_ro);
+	if (ret)
+		return ret;
+	ret = frob_rodata(&mod->init_layout, set_memory_ro);
+	if (ret)
+		return ret;
 
-	if (after_init)
-		frob_ro_after_init(&mod->core_layout, set_memory_ro);
+	if (after_init) {
+		ret = frob_ro_after_init(&mod->core_layout, set_memory_ro);
+		if (ret)
+			return ret;
+	}
+	return 0;
 }
 
-static void module_enable_nx(const struct module *mod)
+static int module_enable_nx(const struct module *mod)
 {
-	frob_rodata(&mod->core_layout, set_memory_nx);
-	frob_ro_after_init(&mod->core_layout, set_memory_nx);
-	frob_writable_data(&mod->core_layout, set_memory_nx);
-	frob_rodata(&mod->init_layout, set_memory_nx);
-	frob_writable_data(&mod->init_layout, set_memory_nx);
+	int ret;
+
+	ret = frob_rodata(&mod->core_layout, set_memory_nx);
+	if (ret)
+		return ret;
+	ret = frob_ro_after_init(&mod->core_layout, set_memory_nx);
+	if (ret)
+		return ret;
+	ret = frob_writable_data(&mod->core_layout, set_memory_nx);
+	if (ret)
+		return ret;
+	ret = frob_rodata(&mod->init_layout, set_memory_nx);
+	if (ret)
+		return ret;
+	ret = frob_writable_data(&mod->init_layout, set_memory_nx);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 /* Iterate through all modules and set each module's text as RW */
-void set_all_modules_text_rw(void)
+int set_all_modules_text_rw(void)
 {
 	struct module *mod;
+	int ret;
 
 	if (!rodata_enabled)
-		return;
+		return 0;
 
 	mutex_lock(&module_mutex);
 	list_for_each_entry_rcu(mod, &modules, list) {
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
 
-		frob_text(&mod->core_layout, set_memory_rw);
-		frob_text(&mod->init_layout, set_memory_rw);
+		ret = frob_text(&mod->core_layout, set_memory_rw);
+		if (ret) {
+			mutex_unlock(&module_mutex);
+			return ret;
+		}
+		ret = frob_text(&mod->init_layout, set_memory_rw);
+		if (ret) {
+			mutex_unlock(&module_mutex);
+			return ret;
+		}
 	}
 	mutex_unlock(&module_mutex);
+	return 0;
 }
 
 /* Iterate through all modules and set each module's text as RO */
-void set_all_modules_text_ro(void)
+int set_all_modules_text_ro(void)
 {
 	struct module *mod;
+	int ret;
 
 	if (!rodata_enabled)
-		return;
+		return 0;
 
 	mutex_lock(&module_mutex);
 	list_for_each_entry_rcu(mod, &modules, list) {
@@ -2017,22 +2068,37 @@ void set_all_modules_text_ro(void)
 			mod->state == MODULE_STATE_GOING)
 			continue;
 
-		frob_text(&mod->core_layout, set_memory_ro);
-		frob_text(&mod->init_layout, set_memory_ro);
+		ret = frob_text(&mod->core_layout, set_memory_ro);
+		if (ret) {
+			mutex_unlock(&module_mutex);
+			return ret;
+		}
+		ret = frob_text(&mod->init_layout, set_memory_ro);
+		if (ret) {
+			mutex_unlock(&module_mutex);
+			return ret;
+		}
 	}
 	mutex_unlock(&module_mutex);
+	return 0;
 }
 #else /* !CONFIG_STRICT_MODULE_RWX */
-static void module_enable_nx(const struct module *mod) { }
+static int module_enable_nx(const struct module *mod) { return 0; }
 #endif /*  CONFIG_STRICT_MODULE_RWX */
-static void module_enable_x(const struct module *mod)
+static int module_enable_x(const struct module *mod)
 {
-	frob_text(&mod->core_layout, set_memory_x);
-	frob_text(&mod->init_layout, set_memory_x);
+	int ret;
+	ret = frob_text(&mod->core_layout, set_memory_x);
+	if (ret)
+		return ret;
+	ret = frob_text(&mod->init_layout, set_memory_x);
+	if (ret)
+		return ret;
+	return 0;
 }
 #else /* !CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
-static void module_enable_nx(const struct module *mod) { }
-static void module_enable_x(const struct module *mod) { }
+static int module_enable_nx(const struct module *mod) { return 0; }
+static int module_enable_x(const struct module *mod) { return 0; }
 #endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
 
 
@@ -3534,7 +3600,11 @@ static noinline int do_init_module(struct module *mod)
 	/* Switch to core kallsyms now init is done: kallsyms may be walking! */
 	rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms);
 #endif
-	module_enable_ro(mod, true);
+	ret = module_enable_ro(mod, true);
+	if (ret) {
+		mutex_unlock(&module_mutex);
+		goto fail_free_freeinit;
+	}
 	mod_tree_remove_init(mod);
 	module_arch_freeing_init(mod);
 	mod->init_layout.base = NULL;
@@ -3640,9 +3710,15 @@ static int complete_formation(struct module *mod, struct load_info *info)
 	/* This relies on module_mutex for list integrity. */
 	module_bug_finalize(info->hdr, info->sechdrs, mod);
 
-	module_enable_ro(mod, false);
-	module_enable_nx(mod);
-	module_enable_x(mod);
+	err = module_enable_ro(mod, false);
+	if (err)
+		goto out;
+	err = module_enable_nx(mod);
+	if (err)
+		goto out;
+	err = module_enable_x(mod);
+	if (err)
+		goto out;
 
 	/* Mark state as coming so strong_try_module_get() ignores us,
 	 * but kallsyms etc. can see us. */
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index f9821a3374e9..d4532bb65d1b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5814,8 +5814,13 @@ void ftrace_module_enable(struct module *mod)
 	 * text to read-only, as we now need to set it back to read-write
 	 * so that we can modify the text.
 	 */
-	if (ftrace_start_up)
-		ftrace_arch_code_modify_prepare();
+	if (ftrace_start_up) {
+		int ret = ftrace_arch_code_modify_prepare();
+		if (ret) {
+			FTRACE_WARN_ON(ret);
+			goto out_unlock;
+		}
+	}
 
 	do_for_each_ftrace_rec(pg, rec) {
 		int cnt;
@@ -5854,8 +5859,10 @@ void ftrace_module_enable(struct module *mod)
 	} while_for_each_ftrace_rec();
 
  out_loop:
-	if (ftrace_start_up)
-		ftrace_arch_code_modify_post_process();
+	if (ftrace_start_up) {
+		int ret = ftrace_arch_code_modify_post_process();
+		FTRACE_WARN_ON(ret);
+	}
 
  out_unlock:
 	mutex_unlock(&ftrace_lock);
-- 
2.17.1


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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-19 15:51 [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value Tianlin Li
@ 2019-11-19 17:07 ` Kees Cook
  2019-11-19 21:43   ` Peter Zijlstra
  2019-11-19 21:51   ` Tianlin Li
  2019-11-19 21:38 ` Peter Zijlstra
  2019-11-20  9:58 ` Borislav Petkov
  2 siblings, 2 replies; 8+ messages in thread
From: Kees Cook @ 2019-11-19 17:07 UTC (permalink / raw)
  To: Tianlin Li
  Cc: kernel-hardening, Steven Rostedt, Ingo Molnar, Russell King,
	Catalin Marinas, Will Deacon, Greentime Hu, Vincent Chen,
	Thomas Gleixner, Borislav Petkov, H . Peter Anvin, x86,
	Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
	Petr Mladek, Joe Lawrence

On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:
> Right now several architectures allow their set_memory_*() family of 
> functions to fail, but callers may not be checking the return values. We 
> need to fix the callers and add the __must_check attribute. They also may
> not provide any level of atomicity, in the sense that the memory 
> protections may be left incomplete on failure. This issue likely has a few 
> steps on effects architectures[1]:

Awesome; thanks for working on this!

A few suggestions on this patch, which will help reviewers, below...

> 1)Have all callers of set_memory_*() helpers check the return value.
> 2)Add __much_check to all set_memory_*() helpers so that new uses do not 
> ignore the return value.
> 3)Add atomicity to the calls so that the memory protections aren't left in 
> a partial state.

Maybe clarify to say something like "this series is step 1"?

> 
> Ideally, the failure of set_memory_*() should be passed up the call stack, 
> and callers should examine the failure and deal with it. But currently, 
> some callers just have void return type.
> 
> We need to fix the callers to handle the return all the way to the top of 
> stack, and it will require a large series of patches to finish all the three 
> steps mentioned above. I start with kernel/module, and will move onto other 
> subsystems. I am not entirely sure about the failure modes for each caller. 
> So I would like to get some comments before I move forward. This single 
> patch is just for fixing the return value of set_memory_*() function in 
> kernel/module, and also the related callers. Any feedback would be greatly 
> appreciated.
> 
> [1]:https://github.com/KSPP/linux/issues/7
> 
> Signed-off-by: Tianlin Li <tli@digitalocean.com>
> ---
>  arch/arm/kernel/ftrace.c   |   8 +-
>  arch/arm64/kernel/ftrace.c |   6 +-
>  arch/nds32/kernel/ftrace.c |   6 +-
>  arch/x86/kernel/ftrace.c   |  13 ++-
>  include/linux/module.h     |  16 ++--
>  kernel/livepatch/core.c    |  15 +++-
>  kernel/module.c            | 170 +++++++++++++++++++++++++++----------
>  kernel/trace/ftrace.c      |  15 +++-
>  8 files changed, 175 insertions(+), 74 deletions(-)

- Can you break this patch into 3 separate patches, by "subsystem":
	- ftrace
	- module
	- livepatch (unless Jessica thinks maybe livepatch should go
	  with module?)
- Please run patches through scripts/checkpatch.pl to catch common
  coding style issues (e.g. blank line between variable declarations
  and function body).
- These changes look pretty straight forward, so I think it'd be fine
  to drop the "RFC" tag and include linux-kernel@vger.kernel.org in the
  Cc list for the v2. For lots of detail, see:
  https://www.kernel.org/doc/html/latest/process/submitting-patches.html

More below...

> 
> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> index bda949fd84e8..7ea1338821d6 100644
> --- a/arch/arm/kernel/ftrace.c
> +++ b/arch/arm/kernel/ftrace.c
> @@ -59,13 +59,15 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
>  
>  int ftrace_arch_code_modify_prepare(void)
>  {
> -	set_all_modules_text_rw();
> -	return 0;
> +	return set_all_modules_text_rw();
>  }
>  
>  int ftrace_arch_code_modify_post_process(void)
>  {
> -	set_all_modules_text_ro();
> +	int ret;

Blank line here...

> +	ret = set_all_modules_text_ro();
> +	if (ret)
> +		return ret;
>  	/* Make sure any TLB misses during machine stop are cleared. */
>  	flush_tlb_all();
>  	return 0;

Are callers of these ftrace functions checking return values too?

> diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
> index 171773257974..97a89c38f6b9 100644
> --- a/arch/arm64/kernel/ftrace.c
> +++ b/arch/arm64/kernel/ftrace.c
> @@ -115,9 +115,11 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  			}
>  
>  			/* point the trampoline to our ftrace entry point */
> -			module_disable_ro(mod);
> +			if (module_disable_ro(mod))
> +				return -EINVAL;
>  			*dst = trampoline;
> -			module_enable_ro(mod, true);
> +			if (module_enable_ro(mod, true))
> +				return -EINVAL;
>  
>  			/*
>  			 * Ensure updated trampoline is visible to instruction
> diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c
> index fd2a54b8cd57..e9e63e703a3e 100644
> --- a/arch/nds32/kernel/ftrace.c
> +++ b/arch/nds32/kernel/ftrace.c
> @@ -91,14 +91,12 @@ int __init ftrace_dyn_arch_init(void)
>  
>  int ftrace_arch_code_modify_prepare(void)
>  {
> -	set_all_modules_text_rw();
> -	return 0;
> +	return set_all_modules_text_rw();
>  }
>  
>  int ftrace_arch_code_modify_post_process(void)
>  {
> -	set_all_modules_text_ro();
> -	return 0;
> +	return set_all_modules_text_ro();
>  }
>  
>  static unsigned long gen_sethi_insn(unsigned long addr)
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 024c3053dbba..7fdee06e2a76 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -42,19 +42,26 @@ int ftrace_arch_code_modify_prepare(void)
>  	 * and live kernel patching from changing the text permissions while
>  	 * ftrace has it set to "read/write".
>  	 */
> +	int ret;
>  	mutex_lock(&text_mutex);
>  	set_kernel_text_rw();
> -	set_all_modules_text_rw();
> +	ret = set_all_modules_text_rw();
> +	if (ret) {
> +		set_kernel_text_ro();

Is the set_kernel_text_ro() here is an atomicity fix? Also, won't a future
patch need to check the result of that function call? (i.e. should it
be left out of this series and should atomicity fixes happen inside the
set_memory_*() functions? Can they happen there?)

> +		mutex_unlock(&text_mutex);
> +		return ret;
> +	}
>  	return 0;
>  }
>  
>  int ftrace_arch_code_modify_post_process(void)
>      __releases(&text_mutex)
>  {
> -	set_all_modules_text_ro();
> +	int ret;

blank line needed...

> +	ret = set_all_modules_text_ro();
>  	set_kernel_text_ro();
>  	mutex_unlock(&text_mutex);
> -	return 0;
> +	return ret;
>  }
>  
>  union ftrace_code_union {
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 1455812dd325..e6c7f3b719a3 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -847,15 +847,15 @@ extern int module_sysfs_initialized;
>  #define __MODULE_STRING(x) __stringify(x)
>  
>  #ifdef CONFIG_STRICT_MODULE_RWX
> -extern void set_all_modules_text_rw(void);
> -extern void set_all_modules_text_ro(void);
> -extern void module_enable_ro(const struct module *mod, bool after_init);
> -extern void module_disable_ro(const struct module *mod);
> +extern int set_all_modules_text_rw(void);
> +extern int set_all_modules_text_ro(void);
> +extern int module_enable_ro(const struct module *mod, bool after_init);
> +extern int module_disable_ro(const struct module *mod);
>  #else
> -static inline void set_all_modules_text_rw(void) { }
> -static inline void set_all_modules_text_ro(void) { }
> -static inline void module_enable_ro(const struct module *mod, bool after_init) { }
> -static inline void module_disable_ro(const struct module *mod) { }
> +static inline int set_all_modules_text_rw(void) { return 0; }
> +static inline int set_all_modules_text_ro(void) { return 0; }
> +static inline int module_enable_ro(const struct module *mod, bool after_init) { return 0; }

Please wrap this line (yes, the old one wasn't...)

> +static inline int module_disable_ro(const struct module *mod) { return 0; }
>  #endif
>  
>  #ifdef CONFIG_GENERIC_BUG
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index c4ce08f43bd6..39bfc0685854 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,
>  
>  	mutex_lock(&text_mutex);
>  
> -	module_disable_ro(patch->mod);
> +	ret = module_disable_ro(patch->mod);
> +	if (ret) {
> +		mutex_unlock(&text_mutex);
> +		return ret;
> +	}
>  	ret = klp_write_object_relocations(patch->mod, obj);
>  	if (ret) {
> -		module_enable_ro(patch->mod, true);
> +		if (module_enable_ro(patch->mod, true));
> +			pr_err("module_enable_ro failed.\n");

Why the pr_err() here and not in other failure cases? (Also, should it
instead be a WARN_ONCE() inside the set_memory_*() functions instead?)

>  		mutex_unlock(&text_mutex);
>  		return ret;
>  	}
>  
>  	arch_klp_init_object_loaded(patch, obj);
> -	module_enable_ro(patch->mod, true);
> +	ret = module_enable_ro(patch->mod, true);
> +	if (ret) {
> +		mutex_unlock(&text_mutex);
> +		return ret;
> +	}
>  
>  	mutex_unlock(&text_mutex);
>  
> diff --git a/kernel/module.c b/kernel/module.c
> index 9ee93421269c..87125b5e315c 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1900,111 +1900,162 @@ static void mod_sysfs_teardown(struct module *mod)
>   *
>   * These values are always page-aligned (as is base)
>   */
> -static void frob_text(const struct module_layout *layout,
> +static int frob_text(const struct module_layout *layout,
>  		      int (*set_memory)(unsigned long start, int num_pages))
>  {
>  	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
> -	set_memory((unsigned long)layout->base,
> +	return set_memory((unsigned long)layout->base,
>  		   layout->text_size >> PAGE_SHIFT);
>  }
>  
>  #ifdef CONFIG_STRICT_MODULE_RWX
> -static void frob_rodata(const struct module_layout *layout,
> +static int frob_rodata(const struct module_layout *layout,
>  			int (*set_memory)(unsigned long start, int num_pages))
>  {
>  	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
> -	set_memory((unsigned long)layout->base + layout->text_size,
> +	return set_memory((unsigned long)layout->base + layout->text_size,
>  		   (layout->ro_size - layout->text_size) >> PAGE_SHIFT);
>  }
>  
> -static void frob_ro_after_init(const struct module_layout *layout,
> +static int frob_ro_after_init(const struct module_layout *layout,
>  				int (*set_memory)(unsigned long start, int num_pages))
>  {
>  	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
> -	set_memory((unsigned long)layout->base + layout->ro_size,
> +	return set_memory((unsigned long)layout->base + layout->ro_size,
>  		   (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT);
>  }
>  
> -static void frob_writable_data(const struct module_layout *layout,
> +static int frob_writable_data(const struct module_layout *layout,
>  			       int (*set_memory)(unsigned long start, int num_pages))
>  {
>  	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
>  	BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1));
> -	set_memory((unsigned long)layout->base + layout->ro_after_init_size,
> +	return set_memory((unsigned long)layout->base + layout->ro_after_init_size,
>  		   (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT);
>  }
>  
>  /* livepatching wants to disable read-only so it can frob module. */
> -void module_disable_ro(const struct module *mod)
> +int module_disable_ro(const struct module *mod)
>  {
> +	int ret;
>  	if (!rodata_enabled)
> -		return;
> +		return 0;
>  
> -	frob_text(&mod->core_layout, set_memory_rw);
> -	frob_rodata(&mod->core_layout, set_memory_rw);
> -	frob_ro_after_init(&mod->core_layout, set_memory_rw);
> -	frob_text(&mod->init_layout, set_memory_rw);
> -	frob_rodata(&mod->init_layout, set_memory_rw);
> +	ret = frob_text(&mod->core_layout, set_memory_rw);
> +	if (ret)
> +		return ret;
> +	ret = frob_rodata(&mod->core_layout, set_memory_rw);
> +	if (ret)
> +		return ret;
> +	ret = frob_ro_after_init(&mod->core_layout, set_memory_rw);
> +	if (ret)
> +		return ret;
> +	ret = frob_text(&mod->init_layout, set_memory_rw);
> +	if (ret)
> +		return ret;
> +	ret = frob_rodata(&mod->init_layout, set_memory_rw);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
>  }
>  
> -void module_enable_ro(const struct module *mod, bool after_init)
> +int module_enable_ro(const struct module *mod, bool after_init)
>  {
> +	int ret;
>  	if (!rodata_enabled)
> -		return;
> +		return 0;
>  
>  	set_vm_flush_reset_perms(mod->core_layout.base);
>  	set_vm_flush_reset_perms(mod->init_layout.base);
> -	frob_text(&mod->core_layout, set_memory_ro);
> +	ret = frob_text(&mod->core_layout, set_memory_ro);
> +	if (ret)
> +		return ret;
>  
> -	frob_rodata(&mod->core_layout, set_memory_ro);
> -	frob_text(&mod->init_layout, set_memory_ro);
> -	frob_rodata(&mod->init_layout, set_memory_ro);
> +	ret = frob_rodata(&mod->core_layout, set_memory_ro);
> +	if (ret)
> +		return ret;
> +	ret = frob_text(&mod->init_layout, set_memory_ro);
> +	if (ret)
> +		return ret;
> +	ret = frob_rodata(&mod->init_layout, set_memory_ro);
> +	if (ret)
> +		return ret;
>  
> -	if (after_init)
> -		frob_ro_after_init(&mod->core_layout, set_memory_ro);
> +	if (after_init) {
> +		ret = frob_ro_after_init(&mod->core_layout, set_memory_ro);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
>  }
>  
> -static void module_enable_nx(const struct module *mod)
> +static int module_enable_nx(const struct module *mod)
>  {
> -	frob_rodata(&mod->core_layout, set_memory_nx);
> -	frob_ro_after_init(&mod->core_layout, set_memory_nx);
> -	frob_writable_data(&mod->core_layout, set_memory_nx);
> -	frob_rodata(&mod->init_layout, set_memory_nx);
> -	frob_writable_data(&mod->init_layout, set_memory_nx);
> +	int ret;
> +
> +	ret = frob_rodata(&mod->core_layout, set_memory_nx);
> +	if (ret)
> +		return ret;
> +	ret = frob_ro_after_init(&mod->core_layout, set_memory_nx);
> +	if (ret)
> +		return ret;
> +	ret = frob_writable_data(&mod->core_layout, set_memory_nx);
> +	if (ret)
> +		return ret;
> +	ret = frob_rodata(&mod->init_layout, set_memory_nx);
> +	if (ret)
> +		return ret;
> +	ret = frob_writable_data(&mod->init_layout, set_memory_nx);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
>  }
>  
>  /* Iterate through all modules and set each module's text as RW */
> -void set_all_modules_text_rw(void)
> +int set_all_modules_text_rw(void)
>  {
>  	struct module *mod;
> +	int ret;
>  
>  	if (!rodata_enabled)
> -		return;
> +		return 0;
>  
>  	mutex_lock(&module_mutex);
>  	list_for_each_entry_rcu(mod, &modules, list) {
>  		if (mod->state == MODULE_STATE_UNFORMED)
>  			continue;
>  
> -		frob_text(&mod->core_layout, set_memory_rw);
> -		frob_text(&mod->init_layout, set_memory_rw);
> +		ret = frob_text(&mod->core_layout, set_memory_rw);
> +		if (ret) {
> +			mutex_unlock(&module_mutex);
> +			return ret;
> +		}
> +		ret = frob_text(&mod->init_layout, set_memory_rw);
> +		if (ret) {
> +			mutex_unlock(&module_mutex);
> +			return ret;
> +		}

This pattern feels like it might be better with a "goto out" style:

	mutex_lock...
	list_for_each... {
		ret = frob_...
		if (ret)
			goto out;
		ret = frob_...
		if (ret)
			goto out;
	}
	ret = 0;
out:
	mutex_unlock...
	return ret;

>  	}
>  	mutex_unlock(&module_mutex);
> +	return 0;
>  }
>  
>  /* Iterate through all modules and set each module's text as RO */
> -void set_all_modules_text_ro(void)
> +int set_all_modules_text_ro(void)
>  {
>  	struct module *mod;
> +	int ret;
>  
>  	if (!rodata_enabled)
> -		return;
> +		return 0;
>  
>  	mutex_lock(&module_mutex);
>  	list_for_each_entry_rcu(mod, &modules, list) {
> @@ -2017,22 +2068,37 @@ void set_all_modules_text_ro(void)
>  			mod->state == MODULE_STATE_GOING)
>  			continue;
>  
> -		frob_text(&mod->core_layout, set_memory_ro);
> -		frob_text(&mod->init_layout, set_memory_ro);
> +		ret = frob_text(&mod->core_layout, set_memory_ro);
> +		if (ret) {
> +			mutex_unlock(&module_mutex);
> +			return ret;
> +		}
> +		ret = frob_text(&mod->init_layout, set_memory_ro);
> +		if (ret) {
> +			mutex_unlock(&module_mutex);
> +			return ret;
> +		}
>  	}
>  	mutex_unlock(&module_mutex);
> +	return 0;
>  }
>  #else /* !CONFIG_STRICT_MODULE_RWX */
> -static void module_enable_nx(const struct module *mod) { }
> +static int module_enable_nx(const struct module *mod) { return 0; }
>  #endif /*  CONFIG_STRICT_MODULE_RWX */
> -static void module_enable_x(const struct module *mod)
> +static int module_enable_x(const struct module *mod)
>  {
> -	frob_text(&mod->core_layout, set_memory_x);
> -	frob_text(&mod->init_layout, set_memory_x);
> +	int ret;
> +	ret = frob_text(&mod->core_layout, set_memory_x);
> +	if (ret)
> +		return ret;
> +	ret = frob_text(&mod->init_layout, set_memory_x);
> +	if (ret)
> +		return ret;
> +	return 0;
>  }
>  #else /* !CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
> -static void module_enable_nx(const struct module *mod) { }
> -static void module_enable_x(const struct module *mod) { }
> +static int module_enable_nx(const struct module *mod) { return 0; }
> +static int module_enable_x(const struct module *mod) { return 0; }
>  #endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
>  
>  
> @@ -3534,7 +3600,11 @@ static noinline int do_init_module(struct module *mod)
>  	/* Switch to core kallsyms now init is done: kallsyms may be walking! */
>  	rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms);
>  #endif
> -	module_enable_ro(mod, true);
> +	ret = module_enable_ro(mod, true);
> +	if (ret) {
> +		mutex_unlock(&module_mutex);
> +		goto fail_free_freeinit;
> +	}
>  	mod_tree_remove_init(mod);
>  	module_arch_freeing_init(mod);
>  	mod->init_layout.base = NULL;
> @@ -3640,9 +3710,15 @@ static int complete_formation(struct module *mod, struct load_info *info)
>  	/* This relies on module_mutex for list integrity. */
>  	module_bug_finalize(info->hdr, info->sechdrs, mod);
>  
> -	module_enable_ro(mod, false);
> -	module_enable_nx(mod);
> -	module_enable_x(mod);
> +	err = module_enable_ro(mod, false);
> +	if (err)
> +		goto out;
> +	err = module_enable_nx(mod);
> +	if (err)
> +		goto out;
> +	err = module_enable_x(mod);
> +	if (err)
> +		goto out;
>  
>  	/* Mark state as coming so strong_try_module_get() ignores us,
>  	 * but kallsyms etc. can see us. */
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f9821a3374e9..d4532bb65d1b 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5814,8 +5814,13 @@ void ftrace_module_enable(struct module *mod)
>  	 * text to read-only, as we now need to set it back to read-write
>  	 * so that we can modify the text.
>  	 */
> -	if (ftrace_start_up)
> -		ftrace_arch_code_modify_prepare();
> +	if (ftrace_start_up) {
> +		int ret = ftrace_arch_code_modify_prepare();
> +		if (ret) {
> +			FTRACE_WARN_ON(ret);
> +			goto out_unlock;
> +		}

Can FTRACE_WARN_ON() be used in an "if" like WARN_ON? This could maybe
look like:

	ret = ftrace_arch...
	if (FTRACE_WARN_ON(ret))
		goto out_unlock

Though I not this doesn't result in an error getting passed up? i.e.
ftrace_module_enable() is still "void". Does that matter here?

> +	}
>  
>  	do_for_each_ftrace_rec(pg, rec) {
>  		int cnt;
> @@ -5854,8 +5859,10 @@ void ftrace_module_enable(struct module *mod)
>  	} while_for_each_ftrace_rec();
>  
>   out_loop:
> -	if (ftrace_start_up)
> -		ftrace_arch_code_modify_post_process();
> +	if (ftrace_start_up) {
> +		int ret = ftrace_arch_code_modify_post_process();
> +		FTRACE_WARN_ON(ret);
> +	}
>  
>   out_unlock:
>  	mutex_unlock(&ftrace_lock);
> -- 
> 2.17.1
> 

Thanks!

-- 
Kees Cook

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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-19 15:51 [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value Tianlin Li
  2019-11-19 17:07 ` Kees Cook
@ 2019-11-19 21:38 ` Peter Zijlstra
  2019-11-19 21:54   ` Tianlin Li
  2019-11-20  9:58 ` Borislav Petkov
  2 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2019-11-19 21:38 UTC (permalink / raw)
  To: Tianlin Li
  Cc: kernel-hardening, keescook, Steven Rostedt, Ingo Molnar,
	Russell King, Catalin Marinas, Will Deacon, Greentime Hu,
	Vincent Chen, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
	x86, Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
	Petr Mladek, Joe Lawrence

On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:
> Right now several architectures allow their set_memory_*() family of 
> functions to fail, but callers may not be checking the return values. We 
> need to fix the callers and add the __must_check attribute. They also may
> not provide any level of atomicity, in the sense that the memory 
> protections may be left incomplete on failure. This issue likely has a few 
> steps on effects architectures[1]:
> 1)Have all callers of set_memory_*() helpers check the return value.
> 2)Add __much_check to all set_memory_*() helpers so that new uses do not 
> ignore the return value.
> 3)Add atomicity to the calls so that the memory protections aren't left in 
> a partial state.
> 
> Ideally, the failure of set_memory_*() should be passed up the call stack, 
> and callers should examine the failure and deal with it. But currently, 
> some callers just have void return type.
> 
> We need to fix the callers to handle the return all the way to the top of 
> stack, and it will require a large series of patches to finish all the three 
> steps mentioned above. I start with kernel/module, and will move onto other 
> subsystems. I am not entirely sure about the failure modes for each caller. 
> So I would like to get some comments before I move forward. This single 
> patch is just for fixing the return value of set_memory_*() function in 
> kernel/module, and also the related callers. Any feedback would be greatly 
> appreciated.

Please have a look here:

  https://lkml.kernel.org/r/20191111131252.921588318@infradead.org

Much of the code you're patching is slated for removal.

Josh also has patches reworking KLP and there's some ARM64 patches
pending at which point we can also delete module_disable_ro().



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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-19 17:07 ` Kees Cook
@ 2019-11-19 21:43   ` Peter Zijlstra
  2019-11-19 21:51   ` Tianlin Li
  1 sibling, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-11-19 21:43 UTC (permalink / raw)
  To: Kees Cook
  Cc: Tianlin Li, kernel-hardening, Steven Rostedt, Ingo Molnar,
	Russell King, Catalin Marinas, Will Deacon, Greentime Hu,
	Vincent Chen, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
	x86, Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
	Petr Mladek, Joe Lawrence

On Tue, Nov 19, 2019 at 09:07:34AM -0800, Kees Cook wrote:
> > diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> > index bda949fd84e8..7ea1338821d6 100644
> > --- a/arch/arm/kernel/ftrace.c
> > +++ b/arch/arm/kernel/ftrace.c
> > @@ -59,13 +59,15 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
> >  
> >  int ftrace_arch_code_modify_prepare(void)
> >  {
> > -	set_all_modules_text_rw();
> > -	return 0;
> > +	return set_all_modules_text_rw();
> >  }
> >  
> >  int ftrace_arch_code_modify_post_process(void)
> >  {
> > -	set_all_modules_text_ro();
> > +	int ret;
> 
> Blank line here...
> 
> > +	ret = set_all_modules_text_ro();
> > +	if (ret)
> > +		return ret;
> >  	/* Make sure any TLB misses during machine stop are cleared. */
> >  	flush_tlb_all();
> >  	return 0;
> 
> Are callers of these ftrace functions checking return values too?

Aside from the fact that I just deleted the set_all_modules_text_*()
functions, ftrace actually does check the return value. It prvides a
nice WARN and kills ftrace dead.

> > diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
> > index 171773257974..97a89c38f6b9 100644
> > --- a/arch/arm64/kernel/ftrace.c
> > +++ b/arch/arm64/kernel/ftrace.c
> > @@ -115,9 +115,11 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> >  			}
> >  
> >  			/* point the trampoline to our ftrace entry point */
> > -			module_disable_ro(mod);
> > +			if (module_disable_ro(mod))
> > +				return -EINVAL;
> >  			*dst = trampoline;
> > -			module_enable_ro(mod, true);
> > +			if (module_enable_ro(mod, true))
> > +				return -EINVAL;
> >  
> >  			/*
> >  			 * Ensure updated trampoline is visible to instruction

Patching dead code again:

  https://lkml.kernel.org/r/20191029165832.33606-5-mark.rutland@arm.com



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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-19 17:07 ` Kees Cook
  2019-11-19 21:43   ` Peter Zijlstra
@ 2019-11-19 21:51   ` Tianlin Li
  1 sibling, 0 replies; 8+ messages in thread
From: Tianlin Li @ 2019-11-19 21:51 UTC (permalink / raw)
  To: Kees Cook
  Cc: kernel-hardening, Steven Rostedt, Ingo Molnar, Russell King,
	Catalin Marinas, Will Deacon, Greentime Hu, Vincent Chen,
	Thomas Gleixner, Borislav Petkov, H . Peter Anvin, x86,
	Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
	Petr Mladek, Joe Lawrence

[-- Attachment #1: Type: text/plain, Size: 21679 bytes --]


> On Nov 19, 2019, at 11:07 AM, Kees Cook <keescook@chromium.org> wrote:
> 
> On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:
>> Right now several architectures allow their set_memory_*() family of 
>> functions to fail, but callers may not be checking the return values. We 
>> need to fix the callers and add the __must_check attribute. They also may
>> not provide any level of atomicity, in the sense that the memory 
>> protections may be left incomplete on failure. This issue likely has a few 
>> steps on effects architectures[1]:
> 
> Awesome; thanks for working on this!
> 
> A few suggestions on this patch, which will help reviewers, below...
> 
>> 1)Have all callers of set_memory_*() helpers check the return value.
>> 2)Add __much_check to all set_memory_*() helpers so that new uses do not 
>> ignore the return value.
>> 3)Add atomicity to the calls so that the memory protections aren't left in 
>> a partial state.
> 
> Maybe clarify to say something like "this series is step 1”?
ok. Will add the clarification. Thanks!
> 
>> 
>> Ideally, the failure of set_memory_*() should be passed up the call stack, 
>> and callers should examine the failure and deal with it. But currently, 
>> some callers just have void return type.
>> 
>> We need to fix the callers to handle the return all the way to the top of 
>> stack, and it will require a large series of patches to finish all the three 
>> steps mentioned above. I start with kernel/module, and will move onto other 
>> subsystems. I am not entirely sure about the failure modes for each caller. 
>> So I would like to get some comments before I move forward. This single 
>> patch is just for fixing the return value of set_memory_*() function in 
>> kernel/module, and also the related callers. Any feedback would be greatly 
>> appreciated.
>> 
>> [1]:https://github.com/KSPP/linux/issues/7
>> 
>> Signed-off-by: Tianlin Li <tli@digitalocean.com>
>> ---
>> arch/arm/kernel/ftrace.c   |   8 +-
>> arch/arm64/kernel/ftrace.c |   6 +-
>> arch/nds32/kernel/ftrace.c |   6 +-
>> arch/x86/kernel/ftrace.c   |  13 ++-
>> include/linux/module.h     |  16 ++--
>> kernel/livepatch/core.c    |  15 +++-
>> kernel/module.c            | 170 +++++++++++++++++++++++++++----------
>> kernel/trace/ftrace.c      |  15 +++-
>> 8 files changed, 175 insertions(+), 74 deletions(-)
> 
> - Can you break this patch into 3 separate patches, by "subsystem":
> 	- ftrace
> 	- module
> 	- livepatch (unless Jessica thinks maybe livepatch should go
> 	  with module?)
> - Please run patches through scripts/checkpatch.pl to catch common
>  coding style issues (e.g. blank line between variable declarations
>  and function body).
> - These changes look pretty straight forward, so I think it'd be fine
>  to drop the "RFC" tag and include linux-kernel@vger.kernel.org <mailto:linux-kernel@vger.kernel.org> in the
>  Cc list for the v2. For lots of detail, see:
>  https://www.kernel.org/doc/html/latest/process/submitting-patches.html <https://www.kernel.org/doc/html/latest/process/submitting-patches.html>

ok. I will fix them in v2. Thanks a lot! 
> More below...
> 
>> 
>> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
>> index bda949fd84e8..7ea1338821d6 100644
>> --- a/arch/arm/kernel/ftrace.c
>> +++ b/arch/arm/kernel/ftrace.c
>> @@ -59,13 +59,15 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
>> 
>> int ftrace_arch_code_modify_prepare(void)
>> {
>> -	set_all_modules_text_rw();
>> -	return 0;
>> +	return set_all_modules_text_rw();
>> }
>> 
>> int ftrace_arch_code_modify_post_process(void)
>> {
>> -	set_all_modules_text_ro();
>> +	int ret;
> 
> Blank line here...
> 
>> +	ret = set_all_modules_text_ro();
>> +	if (ret)
>> +		return ret;
>> 	/* Make sure any TLB misses during machine stop are cleared. */
>> 	flush_tlb_all();
>> 	return 0;
> 
> Are callers of these ftrace functions checking return values too?
ftrace_arch_code_modify_prepare is called in ftrace_run_update_code and ftrace_module_enable. 
ftrace_run_update_code is checking the return value of ftrace_arch_code_modify_prepare already. 
ftrace_module_enable didn’t check. (I modifies this function in this patch to check the return value of ftrace_arch_code_modify_prepare ). 
Same for ftrace_arch_code_modify_post_process. 

>> diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
>> index 171773257974..97a89c38f6b9 100644
>> --- a/arch/arm64/kernel/ftrace.c
>> +++ b/arch/arm64/kernel/ftrace.c
>> @@ -115,9 +115,11 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>> 			}
>> 
>> 			/* point the trampoline to our ftrace entry point */
>> -			module_disable_ro(mod);
>> +			if (module_disable_ro(mod))
>> +				return -EINVAL;
>> 			*dst = trampoline;
>> -			module_enable_ro(mod, true);
>> +			if (module_enable_ro(mod, true))
>> +				return -EINVAL;
>> 
>> 			/*
>> 			 * Ensure updated trampoline is visible to instruction
>> diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c
>> index fd2a54b8cd57..e9e63e703a3e 100644
>> --- a/arch/nds32/kernel/ftrace.c
>> +++ b/arch/nds32/kernel/ftrace.c
>> @@ -91,14 +91,12 @@ int __init ftrace_dyn_arch_init(void)
>> 
>> int ftrace_arch_code_modify_prepare(void)
>> {
>> -	set_all_modules_text_rw();
>> -	return 0;
>> +	return set_all_modules_text_rw();
>> }
>> 
>> int ftrace_arch_code_modify_post_process(void)
>> {
>> -	set_all_modules_text_ro();
>> -	return 0;
>> +	return set_all_modules_text_ro();
>> }
>> 
>> static unsigned long gen_sethi_insn(unsigned long addr)
>> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
>> index 024c3053dbba..7fdee06e2a76 100644
>> --- a/arch/x86/kernel/ftrace.c
>> +++ b/arch/x86/kernel/ftrace.c
>> @@ -42,19 +42,26 @@ int ftrace_arch_code_modify_prepare(void)
>> 	 * and live kernel patching from changing the text permissions while
>> 	 * ftrace has it set to "read/write".
>> 	 */
>> +	int ret;
>> 	mutex_lock(&text_mutex);
>> 	set_kernel_text_rw();
>> -	set_all_modules_text_rw();
>> +	ret = set_all_modules_text_rw();
>> +	if (ret) {
>> +		set_kernel_text_ro();
> 
> Is the set_kernel_text_ro() here is an atomicity fix? Also, won't a future
> patch need to check the result of that function call? (i.e. should it
> be left out of this series and should atomicity fixes happen inside the
> set_memory_*() functions? Can they happen there?)
set_kernel_text_tro() here was not for an atomicity fix in step 3, it was just for reverting the status 
in ftrace_arch_code_modify_prepare, because set_kernel_text_rw() is called. 
Thanks for pointing it out.  I will check it. 
> 
>> +		mutex_unlock(&text_mutex);
>> +		return ret;
>> +	}
>> 	return 0;
>> }
>> 
>> int ftrace_arch_code_modify_post_process(void)
>>     __releases(&text_mutex)
>> {
>> -	set_all_modules_text_ro();
>> +	int ret;
> 
> blank line needed...
> 
>> +	ret = set_all_modules_text_ro();
>> 	set_kernel_text_ro();
>> 	mutex_unlock(&text_mutex);
>> -	return 0;
>> +	return ret;
>> }
>> 
>> union ftrace_code_union {
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 1455812dd325..e6c7f3b719a3 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -847,15 +847,15 @@ extern int module_sysfs_initialized;
>> #define __MODULE_STRING(x) __stringify(x)
>> 
>> #ifdef CONFIG_STRICT_MODULE_RWX
>> -extern void set_all_modules_text_rw(void);
>> -extern void set_all_modules_text_ro(void);
>> -extern void module_enable_ro(const struct module *mod, bool after_init);
>> -extern void module_disable_ro(const struct module *mod);
>> +extern int set_all_modules_text_rw(void);
>> +extern int set_all_modules_text_ro(void);
>> +extern int module_enable_ro(const struct module *mod, bool after_init);
>> +extern int module_disable_ro(const struct module *mod);
>> #else
>> -static inline void set_all_modules_text_rw(void) { }
>> -static inline void set_all_modules_text_ro(void) { }
>> -static inline void module_enable_ro(const struct module *mod, bool after_init) { }
>> -static inline void module_disable_ro(const struct module *mod) { }
>> +static inline int set_all_modules_text_rw(void) { return 0; }
>> +static inline int set_all_modules_text_ro(void) { return 0; }
>> +static inline int module_enable_ro(const struct module *mod, bool after_init) { return 0; }
> 
> Please wrap this line (yes, the old one wasn't...)
> 
>> +static inline int module_disable_ro(const struct module *mod) { return 0; }
>> #endif
>> 
>> #ifdef CONFIG_GENERIC_BUG
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index c4ce08f43bd6..39bfc0685854 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,
>> 
>> 	mutex_lock(&text_mutex);
>> 
>> -	module_disable_ro(patch->mod);
>> +	ret = module_disable_ro(patch->mod);
>> +	if (ret) {
>> +		mutex_unlock(&text_mutex);
>> +		return ret;
>> +	}
>> 	ret = klp_write_object_relocations(patch->mod, obj);
>> 	if (ret) {
>> -		module_enable_ro(patch->mod, true);
>> +		if (module_enable_ro(patch->mod, true));
>> +			pr_err("module_enable_ro failed.\n");
> 
> Why the pr_err() here and not in other failure cases? (Also, should it
> instead be a WARN_ONCE() inside the set_memory_*() functions instead?)
Because module_enable_ro() is called in the checking of another return value. I can not return the value, so I print the error. 
Similar to the previous question, should it be fixed by atomicity patches later? 
> 
>> 		mutex_unlock(&text_mutex);
>> 		return ret;
>> 	}
>> 
>> 	arch_klp_init_object_loaded(patch, obj);
>> -	module_enable_ro(patch->mod, true);
>> +	ret = module_enable_ro(patch->mod, true);
>> +	if (ret) {
>> +		mutex_unlock(&text_mutex);
>> +		return ret;
>> +	}
>> 
>> 	mutex_unlock(&text_mutex);
>> 
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 9ee93421269c..87125b5e315c 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -1900,111 +1900,162 @@ static void mod_sysfs_teardown(struct module *mod)
>>  *
>>  * These values are always page-aligned (as is base)
>>  */
>> -static void frob_text(const struct module_layout *layout,
>> +static int frob_text(const struct module_layout *layout,
>> 		      int (*set_memory)(unsigned long start, int num_pages))
>> {
>> 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
>> -	set_memory((unsigned long)layout->base,
>> +	return set_memory((unsigned long)layout->base,
>> 		   layout->text_size >> PAGE_SHIFT);
>> }
>> 
>> #ifdef CONFIG_STRICT_MODULE_RWX
>> -static void frob_rodata(const struct module_layout *layout,
>> +static int frob_rodata(const struct module_layout *layout,
>> 			int (*set_memory)(unsigned long start, int num_pages))
>> {
>> 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
>> -	set_memory((unsigned long)layout->base + layout->text_size,
>> +	return set_memory((unsigned long)layout->base + layout->text_size,
>> 		   (layout->ro_size - layout->text_size) >> PAGE_SHIFT);
>> }
>> 
>> -static void frob_ro_after_init(const struct module_layout *layout,
>> +static int frob_ro_after_init(const struct module_layout *layout,
>> 				int (*set_memory)(unsigned long start, int num_pages))
>> {
>> 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
>> -	set_memory((unsigned long)layout->base + layout->ro_size,
>> +	return set_memory((unsigned long)layout->base + layout->ro_size,
>> 		   (layout->ro_after_init_size - layout->ro_size) >> PAGE_SHIFT);
>> }
>> 
>> -static void frob_writable_data(const struct module_layout *layout,
>> +static int frob_writable_data(const struct module_layout *layout,
>> 			       int (*set_memory)(unsigned long start, int num_pages))
>> {
>> 	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->ro_after_init_size & (PAGE_SIZE-1));
>> 	BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1));
>> -	set_memory((unsigned long)layout->base + layout->ro_after_init_size,
>> +	return set_memory((unsigned long)layout->base + layout->ro_after_init_size,
>> 		   (layout->size - layout->ro_after_init_size) >> PAGE_SHIFT);
>> }
>> 
>> /* livepatching wants to disable read-only so it can frob module. */
>> -void module_disable_ro(const struct module *mod)
>> +int module_disable_ro(const struct module *mod)
>> {
>> +	int ret;
>> 	if (!rodata_enabled)
>> -		return;
>> +		return 0;
>> 
>> -	frob_text(&mod->core_layout, set_memory_rw);
>> -	frob_rodata(&mod->core_layout, set_memory_rw);
>> -	frob_ro_after_init(&mod->core_layout, set_memory_rw);
>> -	frob_text(&mod->init_layout, set_memory_rw);
>> -	frob_rodata(&mod->init_layout, set_memory_rw);
>> +	ret = frob_text(&mod->core_layout, set_memory_rw);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_rodata(&mod->core_layout, set_memory_rw);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_ro_after_init(&mod->core_layout, set_memory_rw);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_text(&mod->init_layout, set_memory_rw);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_rodata(&mod->init_layout, set_memory_rw);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> }
>> 
>> -void module_enable_ro(const struct module *mod, bool after_init)
>> +int module_enable_ro(const struct module *mod, bool after_init)
>> {
>> +	int ret;
>> 	if (!rodata_enabled)
>> -		return;
>> +		return 0;
>> 
>> 	set_vm_flush_reset_perms(mod->core_layout.base);
>> 	set_vm_flush_reset_perms(mod->init_layout.base);
>> -	frob_text(&mod->core_layout, set_memory_ro);
>> +	ret = frob_text(&mod->core_layout, set_memory_ro);
>> +	if (ret)
>> +		return ret;
>> 
>> -	frob_rodata(&mod->core_layout, set_memory_ro);
>> -	frob_text(&mod->init_layout, set_memory_ro);
>> -	frob_rodata(&mod->init_layout, set_memory_ro);
>> +	ret = frob_rodata(&mod->core_layout, set_memory_ro);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_text(&mod->init_layout, set_memory_ro);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_rodata(&mod->init_layout, set_memory_ro);
>> +	if (ret)
>> +		return ret;
>> 
>> -	if (after_init)
>> -		frob_ro_after_init(&mod->core_layout, set_memory_ro);
>> +	if (after_init) {
>> +		ret = frob_ro_after_init(&mod->core_layout, set_memory_ro);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +	return 0;
>> }
>> 
>> -static void module_enable_nx(const struct module *mod)
>> +static int module_enable_nx(const struct module *mod)
>> {
>> -	frob_rodata(&mod->core_layout, set_memory_nx);
>> -	frob_ro_after_init(&mod->core_layout, set_memory_nx);
>> -	frob_writable_data(&mod->core_layout, set_memory_nx);
>> -	frob_rodata(&mod->init_layout, set_memory_nx);
>> -	frob_writable_data(&mod->init_layout, set_memory_nx);
>> +	int ret;
>> +
>> +	ret = frob_rodata(&mod->core_layout, set_memory_nx);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_ro_after_init(&mod->core_layout, set_memory_nx);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_writable_data(&mod->core_layout, set_memory_nx);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_rodata(&mod->init_layout, set_memory_nx);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_writable_data(&mod->init_layout, set_memory_nx);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> }
>> 
>> /* Iterate through all modules and set each module's text as RW */
>> -void set_all_modules_text_rw(void)
>> +int set_all_modules_text_rw(void)
>> {
>> 	struct module *mod;
>> +	int ret;
>> 
>> 	if (!rodata_enabled)
>> -		return;
>> +		return 0;
>> 
>> 	mutex_lock(&module_mutex);
>> 	list_for_each_entry_rcu(mod, &modules, list) {
>> 		if (mod->state == MODULE_STATE_UNFORMED)
>> 			continue;
>> 
>> -		frob_text(&mod->core_layout, set_memory_rw);
>> -		frob_text(&mod->init_layout, set_memory_rw);
>> +		ret = frob_text(&mod->core_layout, set_memory_rw);
>> +		if (ret) {
>> +			mutex_unlock(&module_mutex);
>> +			return ret;
>> +		}
>> +		ret = frob_text(&mod->init_layout, set_memory_rw);
>> +		if (ret) {
>> +			mutex_unlock(&module_mutex);
>> +			return ret;
>> +		}
> 
> This pattern feels like it might be better with a "goto out" style:
> 
> 	mutex_lock...
> 	list_for_each... {
> 		ret = frob_...
> 		if (ret)
> 			goto out;
> 		ret = frob_...
> 		if (ret)
> 			goto out;
> 	}
> 	ret = 0;
> out:
> 	mutex_unlock...
> 	return ret;

Thanks! Will fix it in v2. 
> 
>> 	}
>> 	mutex_unlock(&module_mutex);
>> +	return 0;
>> }
>> 
>> /* Iterate through all modules and set each module's text as RO */
>> -void set_all_modules_text_ro(void)
>> +int set_all_modules_text_ro(void)
>> {
>> 	struct module *mod;
>> +	int ret;
>> 
>> 	if (!rodata_enabled)
>> -		return;
>> +		return 0;
>> 
>> 	mutex_lock(&module_mutex);
>> 	list_for_each_entry_rcu(mod, &modules, list) {
>> @@ -2017,22 +2068,37 @@ void set_all_modules_text_ro(void)
>> 			mod->state == MODULE_STATE_GOING)
>> 			continue;
>> 
>> -		frob_text(&mod->core_layout, set_memory_ro);
>> -		frob_text(&mod->init_layout, set_memory_ro);
>> +		ret = frob_text(&mod->core_layout, set_memory_ro);
>> +		if (ret) {
>> +			mutex_unlock(&module_mutex);
>> +			return ret;
>> +		}
>> +		ret = frob_text(&mod->init_layout, set_memory_ro);
>> +		if (ret) {
>> +			mutex_unlock(&module_mutex);
>> +			return ret;
>> +		}
>> 	}
>> 	mutex_unlock(&module_mutex);
>> +	return 0;
>> }
>> #else /* !CONFIG_STRICT_MODULE_RWX */
>> -static void module_enable_nx(const struct module *mod) { }
>> +static int module_enable_nx(const struct module *mod) { return 0; }
>> #endif /*  CONFIG_STRICT_MODULE_RWX */
>> -static void module_enable_x(const struct module *mod)
>> +static int module_enable_x(const struct module *mod)
>> {
>> -	frob_text(&mod->core_layout, set_memory_x);
>> -	frob_text(&mod->init_layout, set_memory_x);
>> +	int ret;
>> +	ret = frob_text(&mod->core_layout, set_memory_x);
>> +	if (ret)
>> +		return ret;
>> +	ret = frob_text(&mod->init_layout, set_memory_x);
>> +	if (ret)
>> +		return ret;
>> +	return 0;
>> }
>> #else /* !CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
>> -static void module_enable_nx(const struct module *mod) { }
>> -static void module_enable_x(const struct module *mod) { }
>> +static int module_enable_nx(const struct module *mod) { return 0; }
>> +static int module_enable_x(const struct module *mod) { return 0; }
>> #endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
>> 
>> 
>> @@ -3534,7 +3600,11 @@ static noinline int do_init_module(struct module *mod)
>> 	/* Switch to core kallsyms now init is done: kallsyms may be walking! */
>> 	rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms);
>> #endif
>> -	module_enable_ro(mod, true);
>> +	ret = module_enable_ro(mod, true);
>> +	if (ret) {
>> +		mutex_unlock(&module_mutex);
>> +		goto fail_free_freeinit;
>> +	}
>> 	mod_tree_remove_init(mod);
>> 	module_arch_freeing_init(mod);
>> 	mod->init_layout.base = NULL;
>> @@ -3640,9 +3710,15 @@ static int complete_formation(struct module *mod, struct load_info *info)
>> 	/* This relies on module_mutex for list integrity. */
>> 	module_bug_finalize(info->hdr, info->sechdrs, mod);
>> 
>> -	module_enable_ro(mod, false);
>> -	module_enable_nx(mod);
>> -	module_enable_x(mod);
>> +	err = module_enable_ro(mod, false);
>> +	if (err)
>> +		goto out;
>> +	err = module_enable_nx(mod);
>> +	if (err)
>> +		goto out;
>> +	err = module_enable_x(mod);
>> +	if (err)
>> +		goto out;
>> 
>> 	/* Mark state as coming so strong_try_module_get() ignores us,
>> 	 * but kallsyms etc. can see us. */
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index f9821a3374e9..d4532bb65d1b 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -5814,8 +5814,13 @@ void ftrace_module_enable(struct module *mod)
>> 	 * text to read-only, as we now need to set it back to read-write
>> 	 * so that we can modify the text.
>> 	 */
>> -	if (ftrace_start_up)
>> -		ftrace_arch_code_modify_prepare();
>> +	if (ftrace_start_up) {
>> +		int ret = ftrace_arch_code_modify_prepare();
>> +		if (ret) {
>> +			FTRACE_WARN_ON(ret);
>> +			goto out_unlock;
>> +		}
> 
> Can FTRACE_WARN_ON() be used in an "if" like WARN_ON? This could maybe
> look like:
> 
> 	ret = ftrace_arch...
> 	if (FTRACE_WARN_ON(ret))
> 		goto out_unlock
> 
> Though I not this doesn't result in an error getting passed up? i.e.
> ftrace_module_enable() is still "void". Does that matter here?
Thanks. I will fix it in v2. 

>> +	}
>> 
>> 	do_for_each_ftrace_rec(pg, rec) {
>> 		int cnt;
>> @@ -5854,8 +5859,10 @@ void ftrace_module_enable(struct module *mod)
>> 	} while_for_each_ftrace_rec();
>> 
>>  out_loop:
>> -	if (ftrace_start_up)
>> -		ftrace_arch_code_modify_post_process();
>> +	if (ftrace_start_up) {
>> +		int ret = ftrace_arch_code_modify_post_process();
>> +		FTRACE_WARN_ON(ret);
>> +	}
>> 
>>  out_unlock:
>> 	mutex_unlock(&ftrace_lock);
>> -- 
>> 2.17.1
>> 
> 
> Thanks,
Thanks for all detail suggestions! 
> -- 
> Kees Cook


[-- Attachment #2: Type: text/html, Size: 114608 bytes --]

<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 19, 2019, at 11:07 AM, Kees Cook &lt;<a href="mailto:keescook@chromium.org" class="">keescook@chromium.org</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Right now several architectures allow their set_memory_*() family of<span class="Apple-converted-space">&nbsp;</span><br class="">functions to fail, but callers may not be checking the return values. We<span class="Apple-converted-space">&nbsp;</span><br class="">need to fix the callers and add the __must_check attribute. They also may<br class="">not provide any level of atomicity, in the sense that the memory<span class="Apple-converted-space">&nbsp;</span><br class="">protections may be left incomplete on failure. This issue likely has a few<span class="Apple-converted-space">&nbsp;</span><br class="">steps on effects architectures[1]:<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Awesome; thanks for working on this!</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">A few suggestions on this patch, which will help reviewers, below...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">1)Have all callers of set_memory_*() helpers check the return value.<br class="">2)Add __much_check to all set_memory_*() helpers so that new uses do not<span class="Apple-converted-space">&nbsp;</span><br class="">ignore the return value.<br class="">3)Add atomicity to the calls so that the memory protections aren't left in<span class="Apple-converted-space">&nbsp;</span><br class="">a partial state.<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Maybe clarify to say something like "this series is step 1”?</span></div></blockquote><div>ok. Will add the clarification. Thanks!</div><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="">Ideally, the failure of set_memory_*() should be passed up the call stack,<span class="Apple-converted-space">&nbsp;</span><br class="">and callers should examine the failure and deal with it. But currently,<span class="Apple-converted-space">&nbsp;</span><br class="">some callers just have void return type.<br class=""><br class="">We need to fix the callers to handle the return all the way to the top of<span class="Apple-converted-space">&nbsp;</span><br class="">stack, and it will require a large series of patches to finish all the three<span class="Apple-converted-space">&nbsp;</span><br class="">steps mentioned above. I start with kernel/module, and will move onto other<span class="Apple-converted-space">&nbsp;</span><br class="">subsystems. I am not entirely sure about the failure modes for each caller.<span class="Apple-converted-space">&nbsp;</span><br class="">So I would like to get some comments before I move forward. This single<span class="Apple-converted-space">&nbsp;</span><br class="">patch is just for fixing the return value of set_memory_*() function in<span class="Apple-converted-space">&nbsp;</span><br class="">kernel/module, and also the related callers. Any feedback would be greatly<span class="Apple-converted-space">&nbsp;</span><br class="">appreciated.<br class=""><br class="">[1]:<a href="https://github.com/KSPP/linux/issues/7" class="">https://github.com/KSPP/linux/issues/7</a><br class=""><br class="">Signed-off-by: Tianlin Li &lt;<a href="mailto:tli@digitalocean.com" class="">tli@digitalocean.com</a>&gt;<br class="">---<br class="">arch/arm/kernel/ftrace.c &nbsp;&nbsp;| &nbsp;&nbsp;8 +-<br class="">arch/arm64/kernel/ftrace.c | &nbsp;&nbsp;6 +-<br class="">arch/nds32/kernel/ftrace.c | &nbsp;&nbsp;6 +-<br class="">arch/x86/kernel/ftrace.c &nbsp;&nbsp;| &nbsp;13 ++-<br class="">include/linux/module.h &nbsp;&nbsp;&nbsp;&nbsp;| &nbsp;16 ++--<br class="">kernel/livepatch/core.c &nbsp;&nbsp;&nbsp;| &nbsp;15 +++-<br class="">kernel/module.c &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;| 170 +++++++++++++++++++++++++++----------<br class="">kernel/trace/ftrace.c &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;| &nbsp;15 +++-<br class="">8 files changed, 175 insertions(+), 74 deletions(-)<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">- Can you break this patch into 3 separate patches, by "subsystem":</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">- ftrace</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">- module</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">- livepatch (unless Jessica thinks maybe livepatch should go</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class=""><span class="Apple-converted-space">&nbsp;</span>&nbsp;with module?)</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">- Please run patches through scripts/checkpatch.pl to catch common</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;coding style issues (e.g. blank line between variable declarations</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;and function body).</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">- These changes look pretty straight forward, so I think it'd be fine</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;to drop the "RFC" tag and include<span class="Apple-converted-space">&nbsp;</span></span><a href="mailto:linux-kernel@vger.kernel.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">linux-kernel@vger.kernel.org</a><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class=""><span class="Apple-converted-space">&nbsp;</span>in the</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;Cc list for the v2. For lots of detail, see:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;</span><a href="https://www.kernel.org/doc/html/latest/process/submitting-patches.html" class="">https://www.kernel.org/doc/html/latest/process/submitting-patches.html</a><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div><br class=""></div><div>ok. I will fix them in v2. Thanks a lot!&nbsp;</div><blockquote type="cite" class=""><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">More below...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="">diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c<br class="">index bda949fd84e8..7ea1338821d6 100644<br class="">--- a/arch/arm/kernel/ftrace.c<br class="">+++ b/arch/arm/kernel/ftrace.c<br class="">@@ -59,13 +59,15 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)<br class=""><br class="">int ftrace_arch_code_modify_prepare(void)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_all_modules_text_rw();<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_all_modules_text_rw();<br class="">}<br class=""><br class="">int ftrace_arch_code_modify_post_process(void)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_all_modules_text_ro();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Blank line here...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = set_all_modules_text_ro();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>/* Make sure any TLB misses during machine stop are cleared. */<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>flush_tlb_all();<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Are callers of these ftrace functions checking return values too?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div>ftrace_arch_code_modify_prepare is called in ftrace_run_update_code and ftrace_module_enable.&nbsp;</div><div>ftrace_run_update_code is checking the return value of ftrace_arch_code_modify_prepare already.&nbsp;</div><div>ftrace_module_enable didn’t check. (I modifies this function in this patch to check the return value of ftrace_arch_code_modify_prepare ).&nbsp;</div><div>Same for ftrace_arch_code_modify_post_process.&nbsp;</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c<br class="">index 171773257974..97a89c38f6b9 100644<br class="">--- a/arch/arm64/kernel/ftrace.c<br class="">+++ b/arch/arm64/kernel/ftrace.c<br class="">@@ -115,9 +115,11 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>/* point the trampoline to our ftrace entry point */<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>module_disable_ro(mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (module_disable_ro(mod))<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return -EINVAL;<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>*dst = trampoline;<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_ro(mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (module_enable_ro(mod, true))<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return -EINVAL;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>/*<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>* Ensure updated trampoline is visible to instruction<br class="">diff --git a/arch/nds32/kernel/ftrace.c b/arch/nds32/kernel/ftrace.c<br class="">index fd2a54b8cd57..e9e63e703a3e 100644<br class="">--- a/arch/nds32/kernel/ftrace.c<br class="">+++ b/arch/nds32/kernel/ftrace.c<br class="">@@ -91,14 +91,12 @@ int __init ftrace_dyn_arch_init(void)<br class=""><br class="">int ftrace_arch_code_modify_prepare(void)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_all_modules_text_rw();<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_all_modules_text_rw();<br class="">}<br class=""><br class="">int ftrace_arch_code_modify_post_process(void)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_all_modules_text_ro();<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_all_modules_text_ro();<br class="">}<br class=""><br class="">static unsigned long gen_sethi_insn(unsigned long addr)<br class="">diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c<br class="">index 024c3053dbba..7fdee06e2a76 100644<br class="">--- a/arch/x86/kernel/ftrace.c<br class="">+++ b/arch/x86/kernel/ftrace.c<br class="">@@ -42,19 +42,26 @@ int ftrace_arch_code_modify_prepare(void)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>* and live kernel patching from changing the text permissions while<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>* ftrace has it set to "read/write".<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>*/<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_lock(&amp;text_mutex);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>set_kernel_text_rw();<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_all_modules_text_rw();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = set_all_modules_text_rw();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>set_kernel_text_ro();<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Is the set_kernel_text_ro() here is an atomicity fix? Also, won't a future</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">patch need to check the result of that function call? (i.e. should it</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">be left out of this series and should atomicity fixes happen inside the</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">set_memory_*() functions? Can they happen there?)</span></div></blockquote><div>set_kernel_text_tro() here was not for an atomicity fix in step 3, it was just for reverting the status&nbsp;</div><div>in ftrace_arch_code_modify_prepare, because set_kernel_text_rw() is called.&nbsp;</div><div>Thanks for pointing it out. &nbsp;I will check it.&nbsp;</div></div><div><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class=""><br class="">int ftrace_arch_code_modify_post_process(void)<br class="">&nbsp;&nbsp;&nbsp;&nbsp;__releases(&amp;text_mutex)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_all_modules_text_ro();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">blank line needed...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = set_all_modules_text_ro();<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>set_kernel_text_ro();<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">}<br class=""><br class="">union ftrace_code_union {<br class="">diff --git a/include/linux/module.h b/include/linux/module.h<br class="">index 1455812dd325..e6c7f3b719a3 100644<br class="">--- a/include/linux/module.h<br class="">+++ b/include/linux/module.h<br class="">@@ -847,15 +847,15 @@ extern int module_sysfs_initialized;<br class="">#define __MODULE_STRING(x) __stringify(x)<br class=""><br class="">#ifdef CONFIG_STRICT_MODULE_RWX<br class="">-extern void set_all_modules_text_rw(void);<br class="">-extern void set_all_modules_text_ro(void);<br class="">-extern void module_enable_ro(const struct module *mod, bool after_init);<br class="">-extern void module_disable_ro(const struct module *mod);<br class="">+extern int set_all_modules_text_rw(void);<br class="">+extern int set_all_modules_text_ro(void);<br class="">+extern int module_enable_ro(const struct module *mod, bool after_init);<br class="">+extern int module_disable_ro(const struct module *mod);<br class="">#else<br class="">-static inline void set_all_modules_text_rw(void) { }<br class="">-static inline void set_all_modules_text_ro(void) { }<br class="">-static inline void module_enable_ro(const struct module *mod, bool after_init) { }<br class="">-static inline void module_disable_ro(const struct module *mod) { }<br class="">+static inline int set_all_modules_text_rw(void) { return 0; }<br class="">+static inline int set_all_modules_text_ro(void) { return 0; }<br class="">+static inline int module_enable_ro(const struct module *mod, bool after_init) { return 0; }<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Please wrap this line (yes, the old one wasn't...)</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+static inline int module_disable_ro(const struct module *mod) { return 0; }<br class="">#endif<br class=""><br class="">#ifdef CONFIG_GENERIC_BUG<br class="">diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c<br class="">index c4ce08f43bd6..39bfc0685854 100644<br class="">--- a/kernel/livepatch/core.c<br class="">+++ b/kernel/livepatch/core.c<br class="">@@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_lock(&amp;text_mutex);<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_disable_ro(patch-&gt;mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = module_disable_ro(patch-&gt;mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = klp_write_object_relocations(patch-&gt;mod, obj);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_ro(patch-&gt;mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (module_enable_ro(patch-&gt;mod, true));<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>pr_err("module_enable_ro failed.\n");<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Why the pr_err() here and not in other failure cases? (Also, should it</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">instead be a WARN_ONCE() inside the set_memory_*() functions instead?)</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote>Because module_enable_ro() is called in the checking of another return value. I can not return the value, so I print the error.&nbsp;</div><div>Similar to the previous question, should it be fixed by atomicity patches later?&nbsp;</div><div><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>arch_klp_init_object_loaded(patch, obj);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_ro(patch-&gt;mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = module_enable_ro(patch-&gt;mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class=""><br class="">diff --git a/kernel/module.c b/kernel/module.c<br class="">index 9ee93421269c..87125b5e315c 100644<br class="">--- a/kernel/module.c<br class="">+++ b/kernel/module.c<br class="">@@ -1900,111 +1900,162 @@ static void mod_sysfs_teardown(struct module *mod)<br class="">&nbsp;*<br class="">&nbsp;* These values are always page-aligned (as is base)<br class="">&nbsp;*/<br class="">-static void frob_text(const struct module_layout *layout,<br class="">+static int frob_text(const struct module_layout *layout,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int (*set_memory)(unsigned long start, int num_pages))<br class="">{<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;base &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;text_size &amp; (PAGE_SIZE-1));<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_memory((unsigned long)layout-&gt;base,<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_memory((unsigned long)layout-&gt;base,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>&nbsp;&nbsp;layout-&gt;text_size &gt;&gt; PAGE_SHIFT);<br class="">}<br class=""><br class="">#ifdef CONFIG_STRICT_MODULE_RWX<br class="">-static void frob_rodata(const struct module_layout *layout,<br class="">+static int frob_rodata(const struct module_layout *layout,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>int (*set_memory)(unsigned long start, int num_pages))<br class="">{<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;base &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;text_size &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;ro_size &amp; (PAGE_SIZE-1));<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_memory((unsigned long)layout-&gt;base + layout-&gt;text_size,<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_memory((unsigned long)layout-&gt;base + layout-&gt;text_size,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>&nbsp;&nbsp;(layout-&gt;ro_size - layout-&gt;text_size) &gt;&gt; PAGE_SHIFT);<br class="">}<br class=""><br class="">-static void frob_ro_after_init(const struct module_layout *layout,<br class="">+static int frob_ro_after_init(const struct module_layout *layout,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>int (*set_memory)(unsigned long start, int num_pages))<br class="">{<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;base &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;ro_size &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;ro_after_init_size &amp; (PAGE_SIZE-1));<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_memory((unsigned long)layout-&gt;base + layout-&gt;ro_size,<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_memory((unsigned long)layout-&gt;base + layout-&gt;ro_size,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>&nbsp;&nbsp;(layout-&gt;ro_after_init_size - layout-&gt;ro_size) &gt;&gt; PAGE_SHIFT);<br class="">}<br class=""><br class="">-static void frob_writable_data(const struct module_layout *layout,<br class="">+static int frob_writable_data(const struct module_layout *layout,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int (*set_memory)(unsigned long start, int num_pages))<br class="">{<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;base &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;ro_after_init_size &amp; (PAGE_SIZE-1));<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>BUG_ON((unsigned long)layout-&gt;size &amp; (PAGE_SIZE-1));<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>set_memory((unsigned long)layout-&gt;base + layout-&gt;ro_after_init_size,<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return set_memory((unsigned long)layout-&gt;base + layout-&gt;ro_after_init_size,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>&nbsp;&nbsp;(layout-&gt;size - layout-&gt;ro_after_init_size) &gt;&gt; PAGE_SHIFT);<br class="">}<br class=""><br class="">/* livepatching wants to disable read-only so it can frob module. */<br class="">-void module_disable_ro(const struct module *mod)<br class="">+int module_disable_ro(const struct module *mod)<br class="">{<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>if (!rodata_enabled)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_rodata(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_ro_after_init(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;init_layout, set_memory_rw);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_rodata(&amp;mod-&gt;init_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_rodata(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_ro_after_init(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;init_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_rodata(&amp;mod-&gt;init_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class=""><br class="">-void module_enable_ro(const struct module *mod, bool after_init)<br class="">+int module_enable_ro(const struct module *mod, bool after_init)<br class="">{<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>if (!rodata_enabled)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>set_vm_flush_reset_perms(mod-&gt;core_layout.base);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>set_vm_flush_reset_perms(mod-&gt;init_layout.base);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_rodata(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;init_layout, set_memory_ro);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_rodata(&amp;mod-&gt;init_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_rodata(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;init_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_rodata(&amp;mod-&gt;init_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>if (after_init)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>frob_ro_after_init(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (after_init) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_ro_after_init(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class=""><br class="">-static void module_enable_nx(const struct module *mod)<br class="">+static int module_enable_nx(const struct module *mod)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_rodata(&amp;mod-&gt;core_layout, set_memory_nx);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_ro_after_init(&amp;mod-&gt;core_layout, set_memory_nx);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_writable_data(&amp;mod-&gt;core_layout, set_memory_nx);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_rodata(&amp;mod-&gt;init_layout, set_memory_nx);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_writable_data(&amp;mod-&gt;init_layout, set_memory_nx);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class="">+<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_rodata(&amp;mod-&gt;core_layout, set_memory_nx);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_ro_after_init(&amp;mod-&gt;core_layout, set_memory_nx);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_writable_data(&amp;mod-&gt;core_layout, set_memory_nx);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_rodata(&amp;mod-&gt;init_layout, set_memory_nx);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_writable_data(&amp;mod-&gt;init_layout, set_memory_nx);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class=""><br class="">/* Iterate through all modules and set each module's text as RW */<br class="">-void set_all_modules_text_rw(void)<br class="">+int set_all_modules_text_rw(void)<br class="">{<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>struct module *mod;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>if (!rodata_enabled)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_lock(&amp;module_mutex);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>list_for_each_entry_rcu(mod, &amp;modules, list) {<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (mod-&gt;state == MODULE_STATE_UNFORMED)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>continue;<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;init_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;core_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;init_layout, set_memory_rw);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">This pattern feels like it might be better with a "goto out" style:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">mutex_lock...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">list_for_each... {</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">ret = frob_...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">if (ret)</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">goto out;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">ret = frob_...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">if (ret)</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">goto out;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">}</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">ret = 0;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">out:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">mutex_unlock...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">return ret;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div><br class=""></div>Thanks! Will fix it in v2.&nbsp;<br class=""><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class=""><br class="">/* Iterate through all modules and set each module's text as RO */<br class="">-void set_all_modules_text_ro(void)<br class="">+int set_all_modules_text_ro(void)<br class="">{<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>struct module *mod;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>if (!rodata_enabled)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_lock(&amp;module_mutex);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>list_for_each_entry_rcu(mod, &amp;modules, list) {<br class="">@@ -2017,22 +2068,37 @@ void set_all_modules_text_ro(void)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mod-&gt;state == MODULE_STATE_GOING)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>continue;<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;init_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;core_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;init_layout, set_memory_ro);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class="">#else /* !CONFIG_STRICT_MODULE_RWX */<br class="">-static void module_enable_nx(const struct module *mod) { }<br class="">+static int module_enable_nx(const struct module *mod) { return 0; }<br class="">#endif /* &nbsp;CONFIG_STRICT_MODULE_RWX */<br class="">-static void module_enable_x(const struct module *mod)<br class="">+static int module_enable_x(const struct module *mod)<br class="">{<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;core_layout, set_memory_x);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>frob_text(&amp;mod-&gt;init_layout, set_memory_x);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>int ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;core_layout, set_memory_x);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = frob_text(&amp;mod-&gt;init_layout, set_memory_x);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>return 0;<br class="">}<br class="">#else /* !CONFIG_ARCH_HAS_STRICT_MODULE_RWX */<br class="">-static void module_enable_nx(const struct module *mod) { }<br class="">-static void module_enable_x(const struct module *mod) { }<br class="">+static int module_enable_nx(const struct module *mod) { return 0; }<br class="">+static int module_enable_x(const struct module *mod) { return 0; }<br class="">#endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */<br class=""><br class=""><br class="">@@ -3534,7 +3600,11 @@ static noinline int do_init_module(struct module *mod)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>/* Switch to core kallsyms now init is done: kallsyms may be walking! */<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>rcu_assign_pointer(mod-&gt;kallsyms, &amp;mod-&gt;core_kallsyms);<br class="">#endif<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_ro(mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = module_enable_ro(mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;module_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>goto fail_free_freeinit;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mod_tree_remove_init(mod);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>module_arch_freeing_init(mod);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mod-&gt;init_layout.base = NULL;<br class="">@@ -3640,9 +3710,15 @@ static int complete_formation(struct module *mod, struct load_info *info)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>/* This relies on module_mutex for list integrity. */<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>module_bug_finalize(info-&gt;hdr, info-&gt;sechdrs, mod);<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_ro(mod, false);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_nx(mod);<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_x(mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>err = module_enable_ro(mod, false);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (err)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>goto out;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>err = module_enable_nx(mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (err)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>goto out;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>err = module_enable_x(mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (err)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>goto out;<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>/* Mark state as coming so strong_try_module_get() ignores us,<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>* but kallsyms etc. can see us. */<br class="">diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c<br class="">index f9821a3374e9..d4532bb65d1b 100644<br class="">--- a/kernel/trace/ftrace.c<br class="">+++ b/kernel/trace/ftrace.c<br class="">@@ -5814,8 +5814,13 @@ void ftrace_module_enable(struct module *mod)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>* text to read-only, as we now need to set it back to read-write<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>* so that we can modify the text.<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-converted-space">&nbsp;</span>*/<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ftrace_start_up)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ftrace_arch_code_modify_prepare();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ftrace_start_up) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>int ret = ftrace_arch_code_modify_prepare();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>FTRACE_WARN_ON(ret);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>goto out_unlock;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Can FTRACE_WARN_ON() be used in an "if" like WARN_ON? This could maybe</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">look like:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">ret = ftrace_arch...</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">if (FTRACE_WARN_ON(ret))</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">goto out_unlock</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Though I not this doesn't result in an error getting passed up? i.e.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">ftrace_module_enable() is still "void". Does that matter here?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div>Thanks. I will fix it in v2.&nbsp;</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>do_for_each_ftrace_rec(pg, rec) {<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>int cnt;<br class="">@@ -5854,8 +5859,10 @@ void ftrace_module_enable(struct module *mod)<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>} while_for_each_ftrace_rec();<br class=""><br class="">&nbsp;out_loop:<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ftrace_start_up)<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>ftrace_arch_code_modify_post_process();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ftrace_start_up) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>int ret = ftrace_arch_code_modify_post_process();<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>FTRACE_WARN_ON(ret);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><br class="">&nbsp;out_unlock:<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;ftrace_lock);<br class="">--<span class="Apple-converted-space">&nbsp;</span><br class="">2.17.1<br class=""><br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Thanks,</span></div></blockquote>Thanks for all detail suggestions!&nbsp;<br class=""><blockquote type="cite" class=""><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">--<span class="Apple-converted-space">&nbsp;</span></span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Kees Cook</span></div></blockquote></div><br class=""></body></html>

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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-19 21:38 ` Peter Zijlstra
@ 2019-11-19 21:54   ` Tianlin Li
  0 siblings, 0 replies; 8+ messages in thread
From: Tianlin Li @ 2019-11-19 21:54 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: kernel-hardening, Kees Cook, Steven Rostedt, Ingo Molnar,
	Russell King, Catalin Marinas, Will Deacon, Greentime Hu,
	Vincent Chen, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
	x86, Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
	Petr Mladek, Joe Lawrence

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]



> On Nov 19, 2019, at 3:38 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:
>> Right now several architectures allow their set_memory_*() family of 
>> functions to fail, but callers may not be checking the return values. We 
>> need to fix the callers and add the __must_check attribute. They also may
>> not provide any level of atomicity, in the sense that the memory 
>> protections may be left incomplete on failure. This issue likely has a few 
>> steps on effects architectures[1]:
>> 1)Have all callers of set_memory_*() helpers check the return value.
>> 2)Add __much_check to all set_memory_*() helpers so that new uses do not 
>> ignore the return value.
>> 3)Add atomicity to the calls so that the memory protections aren't left in 
>> a partial state.
>> 
>> Ideally, the failure of set_memory_*() should be passed up the call stack, 
>> and callers should examine the failure and deal with it. But currently, 
>> some callers just have void return type.
>> 
>> We need to fix the callers to handle the return all the way to the top of 
>> stack, and it will require a large series of patches to finish all the three 
>> steps mentioned above. I start with kernel/module, and will move onto other 
>> subsystems. I am not entirely sure about the failure modes for each caller. 
>> So I would like to get some comments before I move forward. This single 
>> patch is just for fixing the return value of set_memory_*() function in 
>> kernel/module, and also the related callers. Any feedback would be greatly 
>> appreciated.
> 
> Please have a look here:
> 
>  https://lkml.kernel.org/r/20191111131252.921588318@infradead.org <https://lkml.kernel.org/r/20191111131252.921588318@infradead.org>
> 
> Much of the code you're patching is slated for removal.
> 
> Josh also has patches reworking KLP and there's some ARM64 patches
> pending at which point we can also delete module_disable_ro().
Thanks for the information. I will check the code. 

[-- Attachment #2: Type: text/html, Size: 9435 bytes --]

<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Nov 19, 2019, at 3:38 PM, Peter Zijlstra &lt;<a href="mailto:peterz@infradead.org" class="">peterz@infradead.org</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Right now several architectures allow their set_memory_*() family of<span class="Apple-converted-space">&nbsp;</span><br class="">functions to fail, but callers may not be checking the return values. We<span class="Apple-converted-space">&nbsp;</span><br class="">need to fix the callers and add the __must_check attribute. They also may<br class="">not provide any level of atomicity, in the sense that the memory<span class="Apple-converted-space">&nbsp;</span><br class="">protections may be left incomplete on failure. This issue likely has a few<span class="Apple-converted-space">&nbsp;</span><br class="">steps on effects architectures[1]:<br class="">1)Have all callers of set_memory_*() helpers check the return value.<br class="">2)Add __much_check to all set_memory_*() helpers so that new uses do not<span class="Apple-converted-space">&nbsp;</span><br class="">ignore the return value.<br class="">3)Add atomicity to the calls so that the memory protections aren't left in<span class="Apple-converted-space">&nbsp;</span><br class="">a partial state.<br class=""><br class="">Ideally, the failure of set_memory_*() should be passed up the call stack,<span class="Apple-converted-space">&nbsp;</span><br class="">and callers should examine the failure and deal with it. But currently,<span class="Apple-converted-space">&nbsp;</span><br class="">some callers just have void return type.<br class=""><br class="">We need to fix the callers to handle the return all the way to the top of<span class="Apple-converted-space">&nbsp;</span><br class="">stack, and it will require a large series of patches to finish all the three<span class="Apple-converted-space">&nbsp;</span><br class="">steps mentioned above. I start with kernel/module, and will move onto other<span class="Apple-converted-space">&nbsp;</span><br class="">subsystems. I am not entirely sure about the failure modes for each caller.<span class="Apple-converted-space">&nbsp;</span><br class="">So I would like to get some comments before I move forward. This single<span class="Apple-converted-space">&nbsp;</span><br class="">patch is just for fixing the return value of set_memory_*() function in<span class="Apple-converted-space">&nbsp;</span><br class="">kernel/module, and also the related callers. Any feedback would be greatly<span class="Apple-converted-space">&nbsp;</span><br class="">appreciated.<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Please have a look here:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;</span><a href="https://lkml.kernel.org/r/20191111131252.921588318@infradead.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">https://lkml.kernel.org/r/20191111131252.921588318@infradead.org</a><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Much of the code you're patching is slated for removal.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Josh also has patches reworking KLP and there's some ARM64 patches</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">pending at which point we can also delete module_disable_ro().</span></div></blockquote></div>Thanks for the information. I will check the code.&nbsp;</body></html>

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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-19 15:51 [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value Tianlin Li
  2019-11-19 17:07 ` Kees Cook
  2019-11-19 21:38 ` Peter Zijlstra
@ 2019-11-20  9:58 ` Borislav Petkov
  2019-11-20 16:19   ` Tianlin Li
  2 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2019-11-20  9:58 UTC (permalink / raw)
  To: Tianlin Li
  Cc: kernel-hardening, keescook, Steven Rostedt, Ingo Molnar,
	Russell King, Catalin Marinas, Will Deacon, Greentime Hu,
	Vincent Chen, Thomas Gleixner, H . Peter Anvin, x86, Jessica Yu,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence

On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:
> Right now several architectures allow their set_memory_*() family of 
> functions to fail, but callers may not be checking the return values. We 
> need to fix the callers and add the __must_check attribute.

Please formulate commit messages in passive tone. "we" is ambiguous.

From Documentation/process/submitting-patches.rst:

 "Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
  to do frotz", as if you are giving orders to the codebase to change
  its behaviour."

Also, you could add a high-level summary of the failure case from:

https://lore.kernel.org/netdev/20180628213459.28631-4-daniel@iogearbox.net/

as a more real-life, convincing justification for this.

> They also may not provide any level of atomicity, in the sense that
> the memory protections may be left incomplete on failure.
> This issue likely has a few 
> steps on effects architectures[1]:
> 1)Have all callers of set_memory_*() helpers check the return value.
> 2)Add __much_check to all set_memory_*() helpers so that new uses do not 

__must_check

> ignore the return value.
> 3)Add atomicity to the calls so that the memory protections aren't left in 
> a partial state.
> 
> Ideally, the failure of set_memory_*() should be passed up the call stack, 
> and callers should examine the failure and deal with it. But currently, 
> some callers just have void return type.
> 
> We need to fix the callers to handle the return all the way to the top of 
> stack, and it will require a large series of patches to finish all the three 
> steps mentioned above. I start with kernel/module, and will move onto other 
> subsystems. I am not entirely sure about the failure modes for each caller. 
> So I would like to get some comments before I move forward. This single 
> patch is just for fixing the return value of set_memory_*() function in 
> kernel/module, and also the related callers. Any feedback would be greatly 
> appreciated.
> 
> [1]:https://github.com/KSPP/linux/issues/7
> 
> Signed-off-by: Tianlin Li <tli@digitalocean.com>
> ---
>  arch/arm/kernel/ftrace.c   |   8 +-
>  arch/arm64/kernel/ftrace.c |   6 +-
>  arch/nds32/kernel/ftrace.c |   6 +-
>  arch/x86/kernel/ftrace.c   |  13 ++-
>  include/linux/module.h     |  16 ++--
>  kernel/livepatch/core.c    |  15 +++-
>  kernel/module.c            | 170 +++++++++++++++++++++++++++----------
>  kernel/trace/ftrace.c      |  15 +++-
>  8 files changed, 175 insertions(+), 74 deletions(-)

Yeah, general idea makes sense but you'd need to redo your patch ontop
of linux-next because there are some changes in flight in ftrace-land at
least and your patch won't apply anymore after next week, when the merge
window opens.

Also, you should use checkpatch before sending a patch as sometimes it makes
sense what it complains about:

WARNING: Missing a blank line after declarations
#79: FILE: arch/arm/kernel/ftrace.c:68:
+       int ret;
+       ret = set_all_modules_text_ro();

WARNING: Missing a blank line after declarations
#150: FILE: arch/x86/kernel/ftrace.c:61:
+       int ret;
+       ret = set_all_modules_text_ro();

WARNING: trailing semicolon indicates no statements, indent implies otherwise
#203: FILE: kernel/livepatch/core.c:731:
+               if (module_enable_ro(patch->mod, true));
+                       pr_err("module_enable_ro failed.\n");

ERROR: trailing statements should be on next line
#203: FILE: kernel/livepatch/core.c:731:
+               if (module_enable_ro(patch->mod, true));

WARNING: Missing a blank line after declarations
#451: FILE: kernel/module.c:2091:
+       int ret;
+       ret = frob_text(&mod->core_layout, set_memory_x);

WARNING: Missing a blank line after declarations
#511: FILE: kernel/trace/ftrace.c:5819:
+               int ret = ftrace_arch_code_modify_prepare();
+               if (ret) {

WARNING: Missing a blank line after declarations
#527: FILE: kernel/trace/ftrace.c:5864:
+               int ret = ftrace_arch_code_modify_post_process();
+               FTRACE_WARN_ON(ret);

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index c4ce08f43bd6..39bfc0685854 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,
>  
>  	mutex_lock(&text_mutex);
>  
> -	module_disable_ro(patch->mod);
> +	ret = module_disable_ro(patch->mod);
> +	if (ret) {
> +		mutex_unlock(&text_mutex);
> +		return ret;
> +	}
>  	ret = klp_write_object_relocations(patch->mod, obj);
>  	if (ret) {
> -		module_enable_ro(patch->mod, true);
> +		if (module_enable_ro(patch->mod, true));
		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

and if you look at its output above closely, it might even help you
catch the bug you've added.

 [ Don't worry, happens to the best of us. :-) ]

Also, what would help review is if you split your patch:

patch 1: Change functions to return a retval
patch 2-n: Change call sites to handle retval properly

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value
  2019-11-20  9:58 ` Borislav Petkov
@ 2019-11-20 16:19   ` Tianlin Li
  0 siblings, 0 replies; 8+ messages in thread
From: Tianlin Li @ 2019-11-20 16:19 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: kernel-hardening, Kees Cook, Steven Rostedt, Ingo Molnar,
	Russell King, Catalin Marinas, Will Deacon, Greentime Hu,
	Vincent Chen, Thomas Gleixner, H . Peter Anvin, x86, Jessica Yu,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence

[-- Attachment #1: Type: text/plain, Size: 5949 bytes --]



> On Nov 20, 2019, at 3:58 AM, Borislav Petkov <bp@alien8.de> wrote:
> 
> On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:
>> Right now several architectures allow their set_memory_*() family of 
>> functions to fail, but callers may not be checking the return values. We 
>> need to fix the callers and add the __must_check attribute.
> 
> Please formulate commit messages in passive tone. "we" is ambiguous.
> 
> From Documentation/process/submitting-patches.rst:
> 
> "Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
>  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
>  to do frotz", as if you are giving orders to the codebase to change
>  its behaviour."
> 
> Also, you could add a high-level summary of the failure case from:
> 
> https://lore.kernel.org/netdev/20180628213459.28631-4-daniel@iogearbox.net/ <https://lore.kernel.org/netdev/20180628213459.28631-4-daniel@iogearbox.net/>
> 
> as a more real-life, convincing justification for this.
Thanks for pointing it out. I will fix them in v2. 

>> They also may not provide any level of atomicity, in the sense that
>> the memory protections may be left incomplete on failure.
>> This issue likely has a few 
>> steps on effects architectures[1]:
>> 1)Have all callers of set_memory_*() helpers check the return value.
>> 2)Add __much_check to all set_memory_*() helpers so that new uses do not 
> 
> __must_check
> 
>> ignore the return value.
>> 3)Add atomicity to the calls so that the memory protections aren't left in 
>> a partial state.
>> 
>> Ideally, the failure of set_memory_*() should be passed up the call stack, 
>> and callers should examine the failure and deal with it. But currently, 
>> some callers just have void return type.
>> 
>> We need to fix the callers to handle the return all the way to the top of 
>> stack, and it will require a large series of patches to finish all the three 
>> steps mentioned above. I start with kernel/module, and will move onto other 
>> subsystems. I am not entirely sure about the failure modes for each caller. 
>> So I would like to get some comments before I move forward. This single 
>> patch is just for fixing the return value of set_memory_*() function in 
>> kernel/module, and also the related callers. Any feedback would be greatly 
>> appreciated.
>> 
>> [1]:https://github.com/KSPP/linux/issues/7
>> 
>> Signed-off-by: Tianlin Li <tli@digitalocean.com>
>> ---
>> arch/arm/kernel/ftrace.c   |   8 +-
>> arch/arm64/kernel/ftrace.c |   6 +-
>> arch/nds32/kernel/ftrace.c |   6 +-
>> arch/x86/kernel/ftrace.c   |  13 ++-
>> include/linux/module.h     |  16 ++--
>> kernel/livepatch/core.c    |  15 +++-
>> kernel/module.c            | 170 +++++++++++++++++++++++++++----------
>> kernel/trace/ftrace.c      |  15 +++-
>> 8 files changed, 175 insertions(+), 74 deletions(-)
> 
> Yeah, general idea makes sense but you'd need to redo your patch ontop
> of linux-next because there are some changes in flight in ftrace-land at
> least and your patch won't apply anymore after next week, when the merge
> window opens.
> 
> Also, you should use checkpatch before sending a patch as sometimes it makes
> sense what it complains about:
> 
> WARNING: Missing a blank line after declarations
> #79: FILE: arch/arm/kernel/ftrace.c:68:
> +       int ret;
> +       ret = set_all_modules_text_ro();
> 
> WARNING: Missing a blank line after declarations
> #150: FILE: arch/x86/kernel/ftrace.c:61:
> +       int ret;
> +       ret = set_all_modules_text_ro();
> 
> WARNING: trailing semicolon indicates no statements, indent implies otherwise
> #203: FILE: kernel/livepatch/core.c:731:
> +               if (module_enable_ro(patch->mod, true));
> +                       pr_err("module_enable_ro failed.\n");
> 
> ERROR: trailing statements should be on next line
> #203: FILE: kernel/livepatch/core.c:731:
> +               if (module_enable_ro(patch->mod, true));
> 
> WARNING: Missing a blank line after declarations
> #451: FILE: kernel/module.c:2091:
> +       int ret;
> +       ret = frob_text(&mod->core_layout, set_memory_x);
> 
> WARNING: Missing a blank line after declarations
> #511: FILE: kernel/trace/ftrace.c:5819:
> +               int ret = ftrace_arch_code_modify_prepare();
> +               if (ret) {
> 
> WARNING: Missing a blank line after declarations
> #527: FILE: kernel/trace/ftrace.c:5864:
> +               int ret = ftrace_arch_code_modify_post_process();
> +               FTRACE_WARN_ON(ret);
> 
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index c4ce08f43bd6..39bfc0685854 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,
>> 
>> 	mutex_lock(&text_mutex);
>> 
>> -	module_disable_ro(patch->mod);
>> +	ret = module_disable_ro(patch->mod);
>> +	if (ret) {
>> +		mutex_unlock(&text_mutex);
>> +		return ret;
>> +	}
>> 	ret = klp_write_object_relocations(patch->mod, obj);
>> 	if (ret) {
>> -		module_enable_ro(patch->mod, true);
>> +		if (module_enable_ro(patch->mod, true));
> 		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> and if you look at its output above closely, it might even help you
> catch the bug you've added.
> 
> [ Don't worry, happens to the best of us. :-) ]
> 
> Also, what would help review is if you split your patch:
> 
> patch 1: Change functions to return a retval
> patch 2-n: Change call sites to handle retval properly
ok. I will redo the patch on top of linux-next. I will use checkpatch and split the patch properly in v2. 
Thanks for all valuable comments. Really appreciate it. 

> Thx.
> 
> -- 
> Regards/Gruss,
>    Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette <https://people.kernel.org/tglx/notes-about-netiquette>

[-- Attachment #2: Type: text/html, Size: 59731 bytes --]

<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Nov 20, 2019, at 3:58 AM, Borislav Petkov &lt;<a href="mailto:bp@alien8.de" class="">bp@alien8.de</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">On Tue, Nov 19, 2019 at 09:51:49AM -0600, Tianlin Li wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Right now several architectures allow their set_memory_*() family of<span class="Apple-converted-space">&nbsp;</span><br class="">functions to fail, but callers may not be checking the return values. We<span class="Apple-converted-space">&nbsp;</span><br class="">need to fix the callers and add the __must_check attribute.<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Please formulate commit messages in passive tone. "we" is ambiguous.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">From Documentation/process/submitting-patches.rst:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">"Describe your changes in imperative mood, e.g. "make xyzzy do frotz"</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;to do frotz", as if you are giving orders to the codebase to change</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;its behaviour."</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Also, you could add a high-level summary of the failure case from:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="https://lore.kernel.org/netdev/20180628213459.28631-4-daniel@iogearbox.net/" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">https://lore.kernel.org/netdev/20180628213459.28631-4-daniel@iogearbox.net/</a><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">as a more real-life, convincing justification for this.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div>Thanks for pointing it out. I will fix them in v2.&nbsp;</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">They also may not provide any level of atomicity, in the sense that<br class="">the memory protections may be left incomplete on failure.<br class="">This issue likely has a few<span class="Apple-converted-space">&nbsp;</span><br class="">steps on effects architectures[1]:<br class="">1)Have all callers of set_memory_*() helpers check the return value.<br class="">2)Add __much_check to all set_memory_*() helpers so that new uses do not<span class="Apple-converted-space">&nbsp;</span><br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">__must_check</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">ignore the return value.<br class="">3)Add atomicity to the calls so that the memory protections aren't left in<span class="Apple-converted-space">&nbsp;</span><br class="">a partial state.<br class=""><br class="">Ideally, the failure of set_memory_*() should be passed up the call stack,<span class="Apple-converted-space">&nbsp;</span><br class="">and callers should examine the failure and deal with it. But currently,<span class="Apple-converted-space">&nbsp;</span><br class="">some callers just have void return type.<br class=""><br class="">We need to fix the callers to handle the return all the way to the top of<span class="Apple-converted-space">&nbsp;</span><br class="">stack, and it will require a large series of patches to finish all the three<span class="Apple-converted-space">&nbsp;</span><br class="">steps mentioned above. I start with kernel/module, and will move onto other<span class="Apple-converted-space">&nbsp;</span><br class="">subsystems. I am not entirely sure about the failure modes for each caller.<span class="Apple-converted-space">&nbsp;</span><br class="">So I would like to get some comments before I move forward. This single<span class="Apple-converted-space">&nbsp;</span><br class="">patch is just for fixing the return value of set_memory_*() function in<span class="Apple-converted-space">&nbsp;</span><br class="">kernel/module, and also the related callers. Any feedback would be greatly<span class="Apple-converted-space">&nbsp;</span><br class="">appreciated.<br class=""><br class="">[1]:<a href="https://github.com/KSPP/linux/issues/7" class="">https://github.com/KSPP/linux/issues/7</a><br class=""><br class="">Signed-off-by: Tianlin Li &lt;<a href="mailto:tli@digitalocean.com" class="">tli@digitalocean.com</a>&gt;<br class="">---<br class="">arch/arm/kernel/ftrace.c &nbsp;&nbsp;| &nbsp;&nbsp;8 +-<br class="">arch/arm64/kernel/ftrace.c | &nbsp;&nbsp;6 +-<br class="">arch/nds32/kernel/ftrace.c | &nbsp;&nbsp;6 +-<br class="">arch/x86/kernel/ftrace.c &nbsp;&nbsp;| &nbsp;13 ++-<br class="">include/linux/module.h &nbsp;&nbsp;&nbsp;&nbsp;| &nbsp;16 ++--<br class="">kernel/livepatch/core.c &nbsp;&nbsp;&nbsp;| &nbsp;15 +++-<br class="">kernel/module.c &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;| 170 +++++++++++++++++++++++++++----------<br class="">kernel/trace/ftrace.c &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;| &nbsp;15 +++-<br class="">8 files changed, 175 insertions(+), 74 deletions(-)<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Yeah, general idea makes sense but you'd need to redo your patch ontop</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">of linux-next because there are some changes in flight in ftrace-land at</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">least and your patch won't apply anymore after next week, when the merge</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">window opens.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Also, you should use checkpatch before sending a patch as sometimes it makes</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">sense what it complains about:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">WARNING: Missing a blank line after declarations</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#79: FILE: arch/arm/kernel/ftrace.c:68:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int ret;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;ret = set_all_modules_text_ro();</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">WARNING: Missing a blank line after declarations</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#150: FILE: arch/x86/kernel/ftrace.c:61:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int ret;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;ret = set_all_modules_text_ro();</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">WARNING: trailing semicolon indicates no statements, indent implies otherwise</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#203: FILE: kernel/livepatch/core.c:731:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if (module_enable_ro(patch-&gt;mod, true));</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;pr_err("module_enable_ro failed.\n");</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">ERROR: trailing statements should be on next line</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#203: FILE: kernel/livepatch/core.c:731:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if (module_enable_ro(patch-&gt;mod, true));</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">WARNING: Missing a blank line after declarations</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#451: FILE: kernel/module.c:2091:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int ret;</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;ret = frob_text(&amp;mod-&gt;core_layout, set_memory_x);</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">WARNING: Missing a blank line after declarations</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#511: FILE: kernel/trace/ftrace.c:5819:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int ret = ftrace_arch_code_modify_prepare();</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if (ret) {</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">WARNING: Missing a blank line after declarations</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">#527: FILE: kernel/trace/ftrace.c:5864:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int ret = ftrace_arch_code_modify_post_process();</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">+ &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;FTRACE_WARN_ON(ret);</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c<br class="">index c4ce08f43bd6..39bfc0685854 100644<br class="">--- a/kernel/livepatch/core.c<br class="">+++ b/kernel/livepatch/core.c<br class="">@@ -721,16 +721,25 @@ static int klp_init_object_loaded(struct klp_patch *patch,<br class=""><br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_lock(&amp;text_mutex);<br class=""><br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span>module_disable_ro(patch-&gt;mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>ret = module_disable_ro(patch-&gt;mod);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>mutex_unlock(&amp;text_mutex);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>return ret;<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span>}<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>ret = klp_write_object_relocations(patch-&gt;mod, obj);<br class=""><span class="Apple-tab-span" style="white-space: pre;">	</span>if (ret) {<br class="">-<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>module_enable_ro(patch-&gt;mod, true);<br class="">+<span class="Apple-tab-span" style="white-space: pre;">	</span><span class="Apple-tab-span" style="white-space: pre;">	</span>if (module_enable_ro(patch-&gt;mod, true));<br class=""></blockquote><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span class="Apple-tab-span" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">	</span><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">and if you look at its output above closely, it might even help you</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">catch the bug you've added.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">[ Don't worry, happens to the best of us. :-) ]</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Also, what would help review is if you split your patch:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">patch 1: Change functions to return a retval</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">patch 2-n: Change call sites to handle retval properly</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div>ok. I will redo the patch on top of linux-next. I will use checkpatch and split the patch properly in v2.&nbsp;</div><div>Thanks for all valuable comments. Really appreciate it.&nbsp;</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Thx.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">--<span class="Apple-converted-space">&nbsp;</span></span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Regards/Gruss,</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">&nbsp;&nbsp;&nbsp;Boris.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="https://people.kernel.org/tglx/notes-about-netiquette" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">https://people.kernel.org/tglx/notes-about-netiquette</a></div></blockquote></div><br class=""></body></html>

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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19 15:51 [RFC PATCH] kernel/module: have the callers of set_memory_*() check the return value Tianlin Li
2019-11-19 17:07 ` Kees Cook
2019-11-19 21:43   ` Peter Zijlstra
2019-11-19 21:51   ` Tianlin Li
2019-11-19 21:38 ` Peter Zijlstra
2019-11-19 21:54   ` Tianlin Li
2019-11-20  9:58 ` Borislav Petkov
2019-11-20 16:19   ` Tianlin Li

Kernel-hardening archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernel-hardening/0 kernel-hardening/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernel-hardening kernel-hardening/ https://lore.kernel.org/kernel-hardening \
		kernel-hardening@lists.openwall.com
	public-inbox-index kernel-hardening

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.openwall.lists.kernel-hardening


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git