linux-kernel - [tip:x86/apic] x86/vector: Use correct per cpu variable in free_moved

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] [day] [month] [year] [list]

Date:   Tue, 17 Oct 2017 07:49:28 -0700
From:   tip-bot for Thomas Gleixner <tipbot@...or.com>
To:     linux-tip-commits@...r.kernel.org
Cc:     boris.ostrovsky@...cle.com, yu.c.chen@...el.com, hch@....de,
        peterz@...radead.org, tony.luck@...el.com, lenb@...nel.org,
        marc.zyngier@....com, petri.latvala@...el.com, joro@...tes.org,
        tglx@...utronix.de, hpa@...or.com, rui.zhang@...el.com,
        rjw@...ysocki.net, pbonzini@...hat.com, akataria@...are.com,
        bp@...en8.de, mingo@...nel.org, kys@...rosoft.com, jgross@...e.com,
        linux-kernel@...r.kernel.org, arjan@...ux.intel.com,
        dan.j.williams@...el.com, rostedt@...dmis.org
Subject: [tip:x86/apic] x86/vector: Use correct per cpu variable in
 free_moved_vector()

Commit-ID:  0696d059f23c05f2dbc3b19ef50e5bdd175b782b
Gitweb:     https://git.kernel.org/tip/0696d059f23c05f2dbc3b19ef50e5bdd175b782b
Author:     Thomas Gleixner <tglx@...utronix.de>
AuthorDate: Mon, 16 Oct 2017 16:16:19 +0200
Committer:  Thomas Gleixner <tglx@...utronix.de>
CommitDate: Tue, 17 Oct 2017 16:45:09 +0200

x86/vector: Use correct per cpu variable in free_moved_vector()

free_moved_vector() accesses the per cpu vector array with this_cpu_write()
to clear the vector. The function has two call sites:

 1) The vector cleanup IPI
 2) The force_complete_move() code path

For #1 this_cpu_write() is correct as it runs on the CPU on which the
vector needs to be freed.

For #2 this_cpu_write() is wrong because the function is called from an
outgoing CPU which is not necessarily the CPU on which the previous vector
needs to be freed. As a result it sets the vector on the outgoing CPU to
NULL, which is pointless as that CPU does not handle interrupts
anymore. What's worse is that it leaves the vector on the previous target
CPU in place which later on triggers the BUG_ON(vector) in the vector
allocation code when the vector gets reused. That's possible because the
bitmap allocator entry of that CPU is freed correctly.

Always use the CPU to which the vector was associated and clear the vector
entry on that CPU. Fixup the tracepoint as well so it tracks on which CPU
the vector gets removed.

Fixes: 69cde0004a4b ("x86/vector: Use matrix allocator for vector assignment")
Reported-by: Petri Latvala <petri.latvala@...el.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Cc: Juergen Gross <jgross@...e.com>
Cc: Tony Luck <tony.luck@...el.com>
Cc: Len Brown <lenb@...nel.org>
Cc: Marc Zyngier <marc.zyngier@....com>
Cc: Joerg Roedel <joro@...tes.org>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Christoph Hellwig <hch@....de>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Rui Zhang <rui.zhang@...el.com>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Paolo Bonzini <pbonzini@...hat.com>
Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc: "K. Y. Srinivasan" <kys@...rosoft.com>
Cc: Arjan van de Ven <arjan@...ux.intel.com>
Cc: Alok Kataria <akataria@...are.com>
Cc: Dan Williams <dan.j.williams@...el.com>
Cc: Yu Chen <yu.c.chen@...el.com>
Link: https://lkml.kernel.org/r/alpine.DEB.2.20.1710161614430.1973@nanos
---
 arch/x86/include/asm/trace/irq_vectors.h | 12 ++++++++----
 arch/x86/kernel/apic/vector.c            |  4 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/trace/irq_vectors.h b/arch/x86/include/asm/trace/irq_vectors.h
index bc09c5c..bfd480b 100644
--- a/arch/x86/include/asm/trace/irq_vectors.h
+++ b/arch/x86/include/asm/trace/irq_vectors.h
@@ -360,24 +360,28 @@ TRACE_EVENT(vector_setup,
 
 TRACE_EVENT(vector_free_moved,
 
-	TP_PROTO(unsigned int irq, unsigned int vector, bool is_managed),
+	TP_PROTO(unsigned int irq, unsigned int cpu, unsigned int vector,
+		 bool is_managed),
 
-	TP_ARGS(irq, vector, is_managed),
+	TP_ARGS(irq, cpu, vector, is_managed),
 
 	TP_STRUCT__entry(
 		__field(	unsigned int,	irq		)
+		__field(	unsigned int,	cpu		)
 		__field(	unsigned int,	vector		)
 		__field(	bool,		is_managed	)
 	),
 
 	TP_fast_assign(
 		__entry->irq		= irq;
+		__entry->cpu		= cpu;
 		__entry->vector		= vector;
 		__entry->is_managed	= is_managed;
 	),
 
-	TP_printk("irq=%u vector=%u is_managed=%d",
-		  __entry->irq, __entry->vector, __entry->is_managed)
+	TP_printk("irq=%u cpu=%u vector=%u is_managed=%d",
+		  __entry->irq, __entry->cpu, __entry->vector,
+		  __entry->is_managed)
 );
 
 
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 573538e..05c85e6 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -797,9 +797,9 @@ static void free_moved_vector(struct apic_chip_data *apicd)
 	 */
 	WARN_ON_ONCE(managed);
 
-	trace_vector_free_moved(apicd->irq, vector, managed);
+	trace_vector_free_moved(apicd->irq, cpu, vector, managed);
 	irq_matrix_free(vector_matrix, cpu, vector, managed);
-	__this_cpu_write(vector_irq[vector], VECTOR_UNUSED);
+	per_cpu(vector_irq, cpu)[vector] = VECTOR_UNUSED;
 	hlist_del_init(&apicd->clist);
 	apicd->prev_vector = 0;
 	apicd->move_in_progress = 0;