linux-kernel - Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210429224742.391154ae@oasis.local.home>
Date:   Thu, 29 Apr 2021 22:47:42 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Changbin Du <changbin.du@...il.com>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        linux-riscv@...ts.infradead.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        aou@...s.berkeley.edu, peterz@...radead.org, jpoimboe@...hat.com,
        jbaron@...mai.com, ardb@...nel.org,
        Atish Patra <Atish.Patra@....com>,
        Anup Patel <Anup.Patel@....com>, akpm@...ux-foundation.org,
        rppt@...nel.org, mhiramat@...nel.org, zong.li@...ive.com,
        guoren@...ux.alibaba.com, wangkefeng.wang@...wei.com,
        0x7f454c46@...il.com, chenhuang5@...wei.com,
        linux-kernel@...r.kernel.org, kernel-team@...roid.com,
        Palmer Dabbelt <palmerdabbelt@...gle.com>
Subject: Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*

On Fri, 30 Apr 2021 05:54:51 +0800
Changbin Du <changbin.du@...il.com> wrote:

> The problem is that lockdep cannot handle locks across tasks since we use
> stopmachine to patch code for risc-v. So there's a false positive report.
> See privious disscussion here:

> https://lkml.org/lkml/2021/4/29/63

Please use lore.kernel.org, lkml.org is highly unreliable, and is
considered deprecated for use of referencing linux kernel archives.

Would the following patch work?

(note, I did not even compile test it)

-- Steve

diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 845002cc2e57..19acbb4aaeff 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -25,6 +25,8 @@ struct dyn_arch_ftrace {
 };
 #endif
 
+extern int running_ftrace;
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 /*
  * A general call in RISC-V is a pair of insts:
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 7f1e5203de88..834ab4fad637 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -11,15 +11,19 @@
 #include <asm/cacheflush.h>
 #include <asm/patch.h>
 
+int running_ftrace;
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
 {
 	mutex_lock(&text_mutex);
+	running_ftrace = 1;
 	return 0;
 }
 
 int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex)
 {
+	running_ftrace = 0;
 	mutex_unlock(&text_mutex);
 	return 0;
 }
diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 0b552873a577..4cd1c79a9689 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -12,6 +12,7 @@
 #include <asm/cacheflush.h>
 #include <asm/fixmap.h>
 #include <asm/patch.h>
+#include <asm/ftrace.h>
 
 struct patch_insn {
 	void *addr;
@@ -59,8 +60,13 @@ static int patch_insn_write(void *addr, const void *insn, size_t len)
 	 * Before reaching here, it was expected to lock the text_mutex
 	 * already, so we don't need to give another lock here and could
 	 * ensure that it was safe between each cores.
+	 *
+	 * ftrace uses stop machine, and even though the text_mutex is
+	 * held, the stop machine task that calls this function will not
+	 * be the owner.
 	 */
-	lockdep_assert_held(&text_mutex);
+	if (!running_ftrace)
+		lockdep_assert_held(&text_mutex);
 
 	if (across_pages)
 		patch_map(addr + len, FIX_TEXT_POKE1);