All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Johannes Erdfelt <johannes@erdfelt.com>,
	Jessica Yu <jeyu@kernel.org>, Petr Mladek <pmladek@suse.com>,
	Miroslav Benes <mbenes@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sasha Levin <sashal@kernel.org>,
	live-patching@vger.kernel.org
Subject: [PATCH AUTOSEL 4.19 33/34] module: Fix livepatch/ftrace module text permissions race
Date: Tue, 25 Jun 2019 23:43:34 -0400	[thread overview]
Message-ID: <20190626034335.23767-33-sashal@kernel.org> (raw)
In-Reply-To: <20190626034335.23767-1-sashal@kernel.org>

From: Josh Poimboeuf <jpoimboe@redhat.com>

[ Upstream commit 9f255b632bf12c4dd7fc31caee89aa991ef75176 ]

It's possible for livepatch and ftrace to be toggling a module's text
permissions at the same time, resulting in the following panic:

  BUG: unable to handle page fault for address: ffffffffc005b1d9
  #PF: supervisor write access in kernel mode
  #PF: error_code(0x0003) - permissions violation
  PGD 3ea0c067 P4D 3ea0c067 PUD 3ea0e067 PMD 3cc13067 PTE 3b8a1061
  Oops: 0003 [#1] PREEMPT SMP PTI
  CPU: 1 PID: 453 Comm: insmod Tainted: G           O  K   5.2.0-rc1-a188339ca5 #1
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-20181126_142135-anatol 04/01/2014
  RIP: 0010:apply_relocate_add+0xbe/0x14c
  Code: fa 0b 74 21 48 83 fa 18 74 38 48 83 fa 0a 75 40 eb 08 48 83 38 00 74 33 eb 53 83 38 00 75 4e 89 08 89 c8 eb 0a 83 38 00 75 43 <89> 08 48 63 c1 48 39 c8 74 2e eb 48 83 38 00 75 32 48 29 c1 89 08
  RSP: 0018:ffffb223c00dbb10 EFLAGS: 00010246
  RAX: ffffffffc005b1d9 RBX: 0000000000000000 RCX: ffffffff8b200060
  RDX: 000000000000000b RSI: 0000004b0000000b RDI: ffff96bdfcd33000
  RBP: ffffb223c00dbb38 R08: ffffffffc005d040 R09: ffffffffc005c1f0
  R10: ffff96bdfcd33c40 R11: ffff96bdfcd33b80 R12: 0000000000000018
  R13: ffffffffc005c1f0 R14: ffffffffc005e708 R15: ffffffff8b2fbc74
  FS:  00007f5f447beba8(0000) GS:ffff96bdff900000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: ffffffffc005b1d9 CR3: 000000003cedc002 CR4: 0000000000360ea0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
  Call Trace:
   klp_init_object_loaded+0x10f/0x219
   ? preempt_latency_start+0x21/0x57
   klp_enable_patch+0x662/0x809
   ? virt_to_head_page+0x3a/0x3c
   ? kfree+0x8c/0x126
   patch_init+0x2ed/0x1000 [livepatch_test02]
   ? 0xffffffffc0060000
   do_one_initcall+0x9f/0x1c5
   ? kmem_cache_alloc_trace+0xc4/0xd4
   ? do_init_module+0x27/0x210
   do_init_module+0x5f/0x210
   load_module+0x1c41/0x2290
   ? fsnotify_path+0x3b/0x42
   ? strstarts+0x2b/0x2b
   ? kernel_read+0x58/0x65
   __do_sys_finit_module+0x9f/0xc3
   ? __do_sys_finit_module+0x9f/0xc3
   __x64_sys_finit_module+0x1a/0x1c
   do_syscall_64+0x52/0x61
   entry_SYSCALL_64_after_hwframe+0x44/0xa9

The above panic occurs when loading two modules at the same time with
ftrace enabled, where at least one of the modules is a livepatch module:

CPU0					CPU1
klp_enable_patch()
  klp_init_object_loaded()
    module_disable_ro()
    					ftrace_module_enable()
					  ftrace_arch_code_modify_post_process()
				    	    set_all_modules_text_ro()
      klp_write_object_relocations()
        apply_relocate_add()
	  *patches read-only code* - BOOM

A similar race exists when toggling ftrace while loading a livepatch
module.

Fix it by ensuring that the livepatch and ftrace code patching
operations -- and their respective permissions changes -- are protected
by the text_mutex.

Link: http://lkml.kernel.org/r/ab43d56ab909469ac5d2520c5d944ad6d4abd476.1560474114.git.jpoimboe@redhat.com

Reported-by: Johannes Erdfelt <johannes@erdfelt.com>
Fixes: 444d13ff10fb ("modules: add ro_after_init support")
Acked-by: Jessica Yu <jeyu@kernel.org>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 kernel/livepatch/core.c |  6 ++++++
 kernel/trace/ftrace.c   | 10 +++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 5b77a7314e01..722c27c40e5b 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -30,6 +30,7 @@
 #include <linux/elf.h>
 #include <linux/moduleloader.h>
 #include <linux/completion.h>
+#include <linux/memory.h>
 #include <asm/cacheflush.h>
 #include "core.h"
 #include "patch.h"
@@ -708,16 +709,21 @@ static int klp_init_object_loaded(struct klp_patch *patch,
 	struct klp_func *func;
 	int ret;
 
+	mutex_lock(&text_mutex);
+
 	module_disable_ro(patch->mod);
 	ret = klp_write_object_relocations(patch->mod, obj);
 	if (ret) {
 		module_enable_ro(patch->mod, true);
+		mutex_unlock(&text_mutex);
 		return ret;
 	}
 
 	arch_klp_init_object_loaded(patch, obj);
 	module_enable_ro(patch->mod, true);
 
+	mutex_unlock(&text_mutex);
+
 	klp_for_each_func(obj, func) {
 		ret = klp_find_object_symbol(obj->name, func->old_name,
 					     func->old_sympos,
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 90348b343460..50ba14591996 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -35,6 +35,7 @@
 #include <linux/hash.h>
 #include <linux/rcupdate.h>
 #include <linux/kprobes.h>
+#include <linux/memory.h>
 
 #include <trace/events/sched.h>
 
@@ -2627,10 +2628,12 @@ static void ftrace_run_update_code(int command)
 {
 	int ret;
 
+	mutex_lock(&text_mutex);
+
 	ret = ftrace_arch_code_modify_prepare();
 	FTRACE_WARN_ON(ret);
 	if (ret)
-		return;
+		goto out_unlock;
 
 	/*
 	 * By default we use stop_machine() to modify the code.
@@ -2642,6 +2645,9 @@ static void ftrace_run_update_code(int command)
 
 	ret = ftrace_arch_code_modify_post_process();
 	FTRACE_WARN_ON(ret);
+
+out_unlock:
+	mutex_unlock(&text_mutex);
 }
 
 static void ftrace_run_modify_code(struct ftrace_ops *ops, int command,
@@ -5762,6 +5768,7 @@ void ftrace_module_enable(struct module *mod)
 	struct ftrace_page *pg;
 
 	mutex_lock(&ftrace_lock);
+	mutex_lock(&text_mutex);
 
 	if (ftrace_disabled)
 		goto out_unlock;
@@ -5823,6 +5830,7 @@ void ftrace_module_enable(struct module *mod)
 		ftrace_arch_code_modify_post_process();
 
  out_unlock:
+	mutex_unlock(&text_mutex);
 	mutex_unlock(&ftrace_lock);
 
 	process_cached_mods(mod->name);
-- 
2.20.1


  parent reply	other threads:[~2019-06-26  3:45 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-26  3:43 [PATCH AUTOSEL 4.19 01/34] ASoC : cs4265 : readable register too low Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 02/34] ASoC: ak4458: add return value for ak4458_probe Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 03/34] ASoC: soc-pcm: BE dai needs prepare when pause release after resume Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 04/34] ASoC: ak4458: rstn_control - return a non-zero on error only Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 05/34] spi: bitbang: Fix NULL pointer dereference in spi_unregister_master Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 06/34] ASoC: core: lock client_mutex while removing link components Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 07/34] ASoC: sun4i-codec: fix first delay on Speaker Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 08/34] drm/mediatek: fix unbind functions Sasha Levin
2019-06-26  3:43   ` Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 09/34] drm/mediatek: unbind components in mtk_drm_unbind() Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 10/34] drm/mediatek: call drm_atomic_helper_shutdown() when unbinding driver Sasha Levin
2019-06-26  3:43   ` Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 11/34] drm/mediatek: clear num_pipes when unbind driver Sasha Levin
2019-06-26  3:43   ` Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 12/34] drm/mediatek: call mtk_dsi_stop() after mtk_drm_crtc_atomic_disable() Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 13/34] ASoC: max98090: remove 24-bit format support if RJ is 0 Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 14/34] ASoC: sun4i-i2s: Fix sun8i tx channel offset mask Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 15/34] ASoC: sun4i-i2s: Add offset to RX channel select Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 16/34] x86/CPU: Add more Icelake model numbers Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 17/34] usb: gadget: fusb300_udc: Fix memory leak of fusb300->ep[i] Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 18/34] usb: gadget: udc: lpc32xx: allocate descriptor with GFP_ATOMIC Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 19/34] ALSA: hdac: fix memory release for SST and SOF drivers Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 20/34] SoC: rt274: Fix internal jack assignment in set_jack callback Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 21/34] scsi: hpsa: correct ioaccel2 chaining Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 22/34] drm: panel-orientation-quirks: Add quirk for GPD pocket2 Sasha Levin
2019-06-26  3:43   ` Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 23/34] drm: panel-orientation-quirks: Add quirk for GPD MicroPC Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 24/34] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 25/34] platform/x86: intel-vbtn: Report switch events when event wakes device Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 26/34] platform/x86: mlx-platform: Fix parent device in i2c-mux-reg device registration Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 27/34] platform/mellanox: mlxreg-hotplug: Add devm_free_irq call to remove flow Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 28/34] i2c: pca-platform: Fix GPIO lookup code Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 29/34] cpuset: restore sanity to cpuset_cpus_allowed_fallback() Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 30/34] scripts/decode_stacktrace.sh: prefix addr2line with $CROSS_COMPILE Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 31/34] mm/mlock.c: change count_mm_mlocked_page_nr return type Sasha Levin
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 32/34] tracing: avoid build warning with HAVE_NOP_MCOUNT Sasha Levin
2019-06-26  3:43 ` Sasha Levin [this message]
2019-06-26  3:43 ` [PATCH AUTOSEL 4.19 34/34] ftrace: Fix NULL pointer dereference in free_ftrace_func_mapper() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190626034335.23767-33-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=jeyu@kernel.org \
    --cc=johannes@erdfelt.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.