linux-kernel - [PATCH] percpu: Sync vmalloc mappings in pcpu_alloc() and free

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]

Date:   Mon, 4 May 2020 17:12:36 +0200
From:   Joerg Roedel <jroedel@...e.de>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        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: [PATCH] percpu: Sync vmalloc mappings in pcpu_alloc() and
 free_percpu()

On Thu, Apr 30, 2020 at 10:39:19PM -0400, Steven Rostedt wrote:
> What's so damn special about alloc_percpu()? It's definitely not a fast
> path. And it's not used often.

Okay, I fixed it in the percpu code. It is definitly not a nice
solution, but having to call vmalloc_sync_mappings/unmappings() is not a
nice solution at any place in the code. Here is the patch which fixes
this issue for me. I am also not sure what to put in the Fixes tag, as
it is related to tracing code accessing per-cpu data from the page-fault
handler, not sure when this got introduced. Maybe someone else can
provide a meaningful Fixes- or stable tag.

I also have an idea in mind how to make this all more robust and get rid
of the vmalloc_sync_mappings/unmappings() interface, will show more when
I know it works the way I think it does.

Regards,

	Joerg

>From c616a9a09499f9c9d682775767d3de7db81fb2ed Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@...e.de>
Date: Mon, 4 May 2020 17:11:41 +0200
Subject: [PATCH] percpu: Sync vmalloc mappings in pcpu_alloc() and
 free_percpu()

Sync the vmalloc mappings for all page-tables in the system when
allocating and freeing per-cpu memory. This is necessary for
architectures which use page-faults on vmalloc areas.

The page-fault handlers accesses per-cpu data when tracing is enabled,
and fauling again in the page-fault handler on a vmalloc'ed per-cpu area
will result in a recursive fault.

Signed-off-by: Joerg Roedel <jroedel@...e.de>
---
 mm/percpu.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/mm/percpu.c b/mm/percpu.c
index d7e3bc649f4e..6ab035bc6977 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1710,6 +1710,20 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
 	trace_percpu_alloc_percpu(reserved, is_atomic, size, align,
 			chunk->base_addr, off, ptr);
 
+	/*
+	 * The per-cpu buffers might be allocated in the vmalloc area of the
+	 * address space. When the architecture allows faulting on the vmalloc
+	 * area and the memory allocated here is accessed in the page-fault
+	 * handler, the vmalloc area fault may be recursive and could never be
+	 * resolved.
+	 * This happens for example in the tracing code which allocates per-cpu
+	 * and accesses them when tracing page-faults.
+	 * To prevent this, make sure the per-cpu buffers allocated here are
+	 * mapped in all PGDs so that the page-fault handler will never fault
+	 * again on them.
+	 */
+	vmalloc_sync_mappings();
+
 	return ptr;
 
 fail_unlock:
@@ -1958,6 +1972,12 @@ void free_percpu(void __percpu *ptr)
 
 	trace_percpu_free_percpu(chunk->base_addr, off, ptr);
 
+	/*
+	 * See comment at the vmalloc_sync_mappings() call in pcpu_alloc() for
+	 * why this is necessary.
+	 */
+	vmalloc_sync_unmappings();
+
 	spin_unlock_irqrestore(&pcpu_lock, flags);
 
 	if (need_balance)
-- 
2.12.3