Takashi Iwai 52c897
From 29b0589a865b6f66d141d79b2dd1373e4e50fe17 Mon Sep 17 00:00:00 2001
Takashi Iwai 52c897
From: Duoming Zhou <duoming@zju.edu.cn>
Takashi Iwai 52c897
Date: Tue, 24 Jan 2023 08:55:33 +0100
Takashi Iwai 52c897
Subject: [PATCH] media: rc: Fix use-after-free bugs caused by ene_tx_irqsim()
Takashi Iwai 52c897
Git-commit: 29b0589a865b6f66d141d79b2dd1373e4e50fe17
Takashi Iwai 52c897
Patch-mainline: v6.3-rc1
Takashi Iwai 52c897
References: CVE-2023-1118 bsc#1208837
Takashi Iwai 52c897
Takashi Iwai 52c897
When the ene device is detaching, function ene_remove() will
Takashi Iwai 52c897
be called. But there is no function to cancel tx_sim_timer
Takashi Iwai 52c897
in ene_remove(), the timer handler ene_tx_irqsim() could race
Takashi Iwai 52c897
with ene_remove(). As a result, the UAF bugs could happen,
Takashi Iwai 52c897
the process is shown below.
Takashi Iwai 52c897
Takashi Iwai 52c897
    (cleanup routine)          |        (timer routine)
Takashi Iwai 52c897
                               | mod_timer(&dev->tx_sim_timer, ..)
Takashi Iwai 52c897
ene_remove()                   | (wait a time)
Takashi Iwai 52c897
                               | ene_tx_irqsim()
Takashi Iwai 52c897
                               |   dev->hw_lock //USE
Takashi Iwai 52c897
                               |   ene_tx_sample(dev) //USE
Takashi Iwai 52c897
Takashi Iwai 52c897
Fix by adding del_timer_sync(&dev->tx_sim_timer) in ene_remove(),
Takashi Iwai 52c897
The tx_sim_timer could stop before ene device is deallocated.
Takashi Iwai 52c897
Takashi Iwai 52c897
What's more, The rc_unregister_device() and del_timer_sync()
Takashi Iwai 52c897
should be called first in ene_remove() and the deallocated
Takashi Iwai 52c897
functions such as free_irq(), release_region() and so on
Takashi Iwai 52c897
should be called behind them. Because the rc_unregister_device()
Takashi Iwai 52c897
is well synchronized. Otherwise, race conditions may happen. The
Takashi Iwai 52c897
situations that may lead to race conditions are shown below.
Takashi Iwai 52c897
Takashi Iwai 52c897
Firstly, the rx receiver is disabled with ene_rx_disable()
Takashi Iwai 52c897
before rc_unregister_device() in ene_remove(), which means it
Takashi Iwai 52c897
can be enabled again if a process opens /dev/lirc0 between
Takashi Iwai 52c897
ene_rx_disable() and rc_unregister_device().
Takashi Iwai 52c897
Takashi Iwai 52c897
Secondly, the irqaction descriptor is freed by free_irq()
Takashi Iwai 52c897
before the rc device is unregistered, which means irqaction
Takashi Iwai 52c897
descriptor may be accessed again after it is deallocated.
Takashi Iwai 52c897
Takashi Iwai 52c897
Thirdly, the timer can call ene_tx_sample() that can write
Takashi Iwai 52c897
to the io ports, which means the io ports could be accessed
Takashi Iwai 52c897
again after they are deallocated by release_region().
Takashi Iwai 52c897
Takashi Iwai 52c897
Therefore, the rc_unregister_device() and del_timer_sync()
Takashi Iwai 52c897
should be called first in ene_remove().
Takashi Iwai 52c897
Takashi Iwai 52c897
Suggested by: Sean Young <sean@mess.org>
Takashi Iwai 52c897
Takashi Iwai 52c897
Fixes: 9ea53b74df9c ("V4L/DVB: STAGING: remove lirc_ene0100 driver")
Takashi Iwai 52c897
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
Takashi Iwai 52c897
Signed-off-by: Sean Young <sean@mess.org>
Takashi Iwai 52c897
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Takashi Iwai 52c897
Acked-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai 52c897
Takashi Iwai 52c897
---
Takashi Iwai 52c897
 drivers/media/rc/ene_ir.c | 3 ++-
Takashi Iwai 52c897
 1 file changed, 2 insertions(+), 1 deletion(-)
Takashi Iwai 52c897
Takashi Iwai 52c897
diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
Takashi Iwai 52c897
index e09270916fbc..11ee21a7db8f 100644
Takashi Iwai 52c897
--- a/drivers/media/rc/ene_ir.c
Takashi Iwai 52c897
+++ b/drivers/media/rc/ene_ir.c
Takashi Iwai 52c897
@@ -1106,6 +1106,8 @@ static void ene_remove(struct pnp_dev *pnp_dev)
Takashi Iwai 52c897
 	struct ene_device *dev = pnp_get_drvdata(pnp_dev);
Takashi Iwai 52c897
 	unsigned long flags;
Takashi Iwai 52c897
 
Takashi Iwai 52c897
+	rc_unregister_device(dev->rdev);
Takashi Iwai 52c897
+	del_timer_sync(&dev->tx_sim_timer);
Takashi Iwai 52c897
 	spin_lock_irqsave(&dev->hw_lock, flags);
Takashi Iwai 52c897
 	ene_rx_disable(dev);
Takashi Iwai 52c897
 	ene_rx_restore_hw_buffer(dev);
Takashi Iwai 52c897
@@ -1113,7 +1115,6 @@ static void ene_remove(struct pnp_dev *pnp_dev)
Takashi Iwai 52c897
 
Takashi Iwai 52c897
 	free_irq(dev->irq, dev);
Takashi Iwai 52c897
 	release_region(dev->hw_io, ENE_IO_SIZE);
Takashi Iwai 52c897
-	rc_unregister_device(dev->rdev);
Takashi Iwai 52c897
 	kfree(dev);
Takashi Iwai 52c897
 }
Takashi Iwai 52c897
 
Takashi Iwai 52c897
-- 
Takashi Iwai 52c897
2.35.3
Takashi Iwai 52c897