linux-kernel - [PATCH 4.16 48/64] x86/apic/vector: Prevent hlist corruption and leaks

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20180624142746.713587355@linuxfoundation.org>
Date:   Sun, 24 Jun 2018 23:22:29 +0800
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Dmitry Safonov <0x7f454c46@...il.com>,
        Tariq Toukan <tariqt@...lanox.com>,
        Song Liu <liu.song.a23@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Song Liu <songliubraving@...com>,
        Joerg Roedel <jroedel@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Mike Travis <mike.travis@....com>,
        Borislav Petkov <bp@...en8.de>
Subject: [PATCH 4.16 48/64] x86/apic/vector: Prevent hlist corruption and leaks

4.16-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Thomas Gleixner <tglx@...utronix.de>

commit 80ae7b1a918e78b0bae88b0c0ad413d3fdced968 upstream.

Several people observed the WARN_ON() in irq_matrix_free() which triggers
when the caller tries to free an vector which is not in the allocation
range. Song provided the trace information which allowed to decode the root
cause.

The rework of the vector allocation mechanism failed to preserve a sanity
check, which prevents setting a new target vector/CPU when the previous
affinity change has not fully completed.

As a result a half finished affinity change can be overwritten, which can
cause the leak of a irq descriptor pointer on the previous target CPU and
double enqueue of the hlist head into the cleanup lists of two or more
CPUs. After one CPU cleaned up its vector the next CPU will invoke the
cleanup handler with vector 0, which triggers the out of range warning in
the matrix allocator.

Prevent this by checking the apic_data of the interrupt whether the
move_in_progress flag is false and the hlist node is not hashed. Return
-EBUSY if not.

This prevents the damage and restores the behaviour before the vector
allocation rework, but due to other changes in that area it also widens the
chance that user space can observe -EBUSY. In theory this should be fine,
but actually not all user space tools handle -EBUSY correctly. Addressing
that is not part of this fix, but will be addressed in follow up patches.

Fixes: 69cde0004a4b ("x86/vector: Use matrix allocator for vector assignment")
Reported-by: Dmitry Safonov <0x7f454c46@...il.com>
Reported-by: Tariq Toukan <tariqt@...lanox.com>
Reported-by: Song Liu <liu.song.a23@...il.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Tested-by: Song Liu <songliubraving@...com>
Cc: Joerg Roedel <jroedel@...e.de>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: stable@...r.kernel.org
Cc: Mike Travis <mike.travis@....com>
Cc: Borislav Petkov <bp@...en8.de>
Link: https://lkml.kernel.org/r/20180604162224.303870257@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 arch/x86/kernel/apic/vector.c |    9 +++++++++
 1 file changed, 9 insertions(+)

--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -235,6 +235,15 @@ static int allocate_vector(struct irq_da
 	if (vector && cpu_online(cpu) && cpumask_test_cpu(cpu, dest))
 		return 0;

+	/*
+	 * Careful here. @apicd might either have move_in_progress set or
+	 * be enqueued for cleanup. Assigning a new vector would either
+	 * leave a stale vector on some CPU around or in case of a pending
+	 * cleanup corrupt the hlist.
+	 */
+	if (apicd->move_in_progress || !hlist_unhashed(&apicd->clist))
+		return -EBUSY;
+
 	vector = irq_matrix_alloc(vector_matrix, dest, resvd, &cpu);
 	if (vector > 0)
 		apic_update_vector(irqd, vector, cpu);