linux-kernel - [RFC][PATCH] x86/mm: Sync all vmalloc mappings before 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-next>] [day] [month] [year] [list]

Message-ID: <20200429054857.66e8e333@oasis.local.home>
Date:   Wed, 29 Apr 2020 05:48:57 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     LKML <linux-kernel@...r.kernel.org>
Cc:     Joerg Roedel <jroedel@...e.de>, Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Borislav Petkov <bp@...en8.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Shile Zhang <shile.zhang@...ux.alibaba.com>,
        Andy Lutomirski <luto@...capital.net>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Tzvetomir Stoyanov <tz.stoyanov@...il.com>
Subject: [RFC][PATCH] x86/mm: Sync all vmalloc mappings before text_poke()

From: Steven Rostedt (VMware) <rostedt@...dmis.org>

Tzvetomir was adding a feature to trace-cmd that would allow the user
to specify filtering on process IDs within a tracing instance (or
buffer). When he added this feature and tested it on tracing PIDs 1 and
2, it caused his kernel to hang.

He sent me his code and I was able to reproduce the hang as well. I
bisected it down to this commit 763802b53a42 ("x86/mm: split
vmalloc_sync_all()"). It was 100% reproducible. With the commit it
would hang, and reverting the commit, it would work.

Adding a bunch of printk()s, I found where it locked up. It was after
the recording was finished, and a write of "0" to
tracefs/instance/foo/events/enable. And in the code, it was:

(you may skip to the end of the chain)

system_enable_write() {
  __ftrace_set_clr_event() {
    __ftrace_set_clr_event_nolock() {
      ftrace_event_enable_disable() {
        __ftrace_event_enable_disable() {
          call->class->reg() <trace_event_reg()> {
            trace_point_probe_unregister() {
              tracepoint_remove_func() {
                static_key_slow_dec() {
                  __static_key_slow_dec() {

    <continued>

  __static_key_slow_dec_cpus_locked() {
    jump_label_update() {
      __jump_label_update()
        arch_jump_label_transform() {
          jump_label_transform() {
            __jump_label_transform() {
              text_poke_bp() {
                text_poke_bp_batch() {
                  text_poke() {
                    __text_poke() {

    <continued> (This is where you want to see)

  use_temporary_mm() {
    switch_mm_irqs_off() {
      load_new_mm_cr3() {
        write_cr3() <<--- Lock up!

The really strange part about this, is that this only crashes if we
filter the PIDs on an instance (and via trace-cmd, which does some
initial clean ups). But it works fine when doing from the top level
tracing buffer, or by doing this manually. I'm not sure how vmalloc
gets involved with this. Anyway, I tried the following patch, and it
fixes the lockup for both myself and Tzvetomir. But since I'm not 100%
sure what broke, I'm sending this out as an RFC.

Fixes: 763802b53a42 ("x86/mm: split vmalloc_sync_all()")
Reported-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@...il.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
---
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 7867dfb3963e..015dd30a2260 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -802,6 +802,8 @@ static void *__text_poke(void *addr, const void *opcode, size_t len)
 	 */
 	BUG_ON(!after_bootmem);

+	vmalloc_sync_mappings();
+
 	if (!core_kernel_text((unsigned long)addr)) {
 		pages[0] = vmalloc_to_page(addr);
 		if (cross_page_boundary)