linux-kernel - [PATCH v2 1/2] KVM: eventfd: Fix NULL deref irqbypass producer

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: <20230802051700.52321-2-likexu@tencent.com>
Date:   Wed,  2 Aug 2023 13:16:59 +0800
From:   Like Xu <like.xu.linux@...il.com>
To:     Alex Williamson <alex.williamson@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: [PATCH v2 1/2] KVM: eventfd: Fix NULL deref irqbypass producer

From: Like Xu <likexu@...cent.com>

Adding guard logic to make irq_bypass_register/unregister_producer()
looks for the producer entry based on producer pointer itself instead
of pure token matching.

As was attempted commit 4f3dbdf47e15 ("KVM: eventfd: fix NULL deref
irqbypass consumer"), two different producers may occasionally have two
identical eventfd's. In this case, the later producer may unregister
the previous one after the registration fails (since they share the same
token), then NULL deref incurres in the path of deleting producer from
the producers list.

Registration should also fail if a registered producer changes its
token and registers again via the same producer pointer.

Cc: Alex Williamson <alex.williamson@...hat.com>
Signed-off-by: Like Xu <likexu@...cent.com>
Reviewed-by: Alex Williamson <alex.williamson@...hat.com>
---
 virt/lib/irqbypass.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
index 28fda42e471b..e0aabbbf27ec 100644
--- a/virt/lib/irqbypass.c
+++ b/virt/lib/irqbypass.c
@@ -98,7 +98,7 @@ int irq_bypass_register_producer(struct irq_bypass_producer *producer)
 	mutex_lock(&lock);

 	list_for_each_entry(tmp, &producers, node) {
-		if (tmp->token == producer->token) {
+		if (tmp->token == producer->token || tmp == producer) {
 			ret = -EBUSY;
 			goto out_err;
 		}
@@ -148,7 +148,7 @@ void irq_bypass_unregister_producer(struct irq_bypass_producer *producer)
 	mutex_lock(&lock);

 	list_for_each_entry(tmp, &producers, node) {
-		if (tmp->token != producer->token)
+		if (tmp != producer)
 			continue;

 		list_for_each_entry(consumer, &consumers, node) {
-- 
2.41.0