[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230525183144.1717540-26-sashal@kernel.org>
Date: Thu, 25 May 2023 14:31:03 -0400
From: Sasha Levin <sashal@...nel.org>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc: Hyunwoo Kim <imv4bel@...il.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Sasha Levin <sashal@...nel.org>, robert_s@....net,
chenzhongjin@...wei.com, tiwai@...e.de, linma@....edu.cn,
linux-media@...r.kernel.org
Subject: [PATCH AUTOSEL 6.3 26/67] media: dvb-core: Fix use-after-free on race condition at dvb_frontend
From: Hyunwoo Kim <imv4bel@...il.com>
[ Upstream commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f ]
If the device node of dvb_frontend is open() and the device is
disconnected, many kinds of UAFs may occur when calling close()
on the device node.
The root cause of this is that wake_up() for dvbdev->wait_queue
is implemented in the dvb_frontend_release() function, but
wait_event() is not implemented in the dvb_frontend_stop() function.
So, implement wait_event() function in dvb_frontend_stop() and
add 'remove_mutex' which prevents race condition for 'fe->exit'.
[mchehab: fix a couple of checkpatch warnings and some mistakes at the error handling logic]
Link: https://lore.kernel.org/linux-media/20221117045925.14297-2-imv4bel@gmail.com
Signed-off-by: Hyunwoo Kim <imv4bel@...il.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@...nel.org>
Signed-off-by: Sasha Levin <sashal@...nel.org>
---
drivers/media/dvb-core/dvb_frontend.c | 53 ++++++++++++++++++++++-----
include/media/dvb_frontend.h | 6 ++-
2 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index cc0a789f09ae5..947b61959b2b8 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -809,15 +809,26 @@ static void dvb_frontend_stop(struct dvb_frontend *fe)
dev_dbg(fe->dvb->device, "%s:\n", __func__);
+ mutex_lock(&fe->remove_mutex);
+
if (fe->exit != DVB_FE_DEVICE_REMOVED)
fe->exit = DVB_FE_NORMAL_EXIT;
mb();
- if (!fepriv->thread)
+ if (!fepriv->thread) {
+ mutex_unlock(&fe->remove_mutex);
return;
+ }
kthread_stop(fepriv->thread);
+ mutex_unlock(&fe->remove_mutex);
+
+ if (fepriv->dvbdev->users < -1) {
+ wait_event(fepriv->dvbdev->wait_queue,
+ fepriv->dvbdev->users == -1);
+ }
+
sema_init(&fepriv->sem, 1);
fepriv->state = FESTATE_IDLE;
@@ -2761,9 +2772,13 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
struct dvb_adapter *adapter = fe->dvb;
int ret;
+ mutex_lock(&fe->remove_mutex);
+
dev_dbg(fe->dvb->device, "%s:\n", __func__);
- if (fe->exit == DVB_FE_DEVICE_REMOVED)
- return -ENODEV;
+ if (fe->exit == DVB_FE_DEVICE_REMOVED) {
+ ret = -ENODEV;
+ goto err_remove_mutex;
+ }
if (adapter->mfe_shared == 2) {
mutex_lock(&adapter->mfe_lock);
@@ -2771,7 +2786,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
if (adapter->mfe_dvbdev &&
!adapter->mfe_dvbdev->writers) {
mutex_unlock(&adapter->mfe_lock);
- return -EBUSY;
+ ret = -EBUSY;
+ goto err_remove_mutex;
}
adapter->mfe_dvbdev = dvbdev;
}
@@ -2794,8 +2810,10 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
while (mferetry-- && (mfedev->users != -1 ||
mfepriv->thread)) {
if (msleep_interruptible(500)) {
- if (signal_pending(current))
- return -EINTR;
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto err_remove_mutex;
+ }
}
}
@@ -2807,7 +2825,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
if (mfedev->users != -1 ||
mfepriv->thread) {
mutex_unlock(&adapter->mfe_lock);
- return -EBUSY;
+ ret = -EBUSY;
+ goto err_remove_mutex;
}
adapter->mfe_dvbdev = dvbdev;
}
@@ -2866,6 +2885,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
if (adapter->mfe_shared)
mutex_unlock(&adapter->mfe_lock);
+
+ mutex_unlock(&fe->remove_mutex);
return ret;
err3:
@@ -2887,6 +2908,9 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
err0:
if (adapter->mfe_shared)
mutex_unlock(&adapter->mfe_lock);
+
+err_remove_mutex:
+ mutex_unlock(&fe->remove_mutex);
return ret;
}
@@ -2897,6 +2921,8 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
struct dvb_frontend_private *fepriv = fe->frontend_priv;
int ret;
+ mutex_lock(&fe->remove_mutex);
+
dev_dbg(fe->dvb->device, "%s:\n", __func__);
if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
@@ -2918,10 +2944,18 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
}
mutex_unlock(&fe->dvb->mdev_lock);
#endif
- if (fe->exit != DVB_FE_NO_EXIT)
- wake_up(&dvbdev->wait_queue);
if (fe->ops.ts_bus_ctrl)
fe->ops.ts_bus_ctrl(fe, 0);
+
+ if (fe->exit != DVB_FE_NO_EXIT) {
+ mutex_unlock(&fe->remove_mutex);
+ wake_up(&dvbdev->wait_queue);
+ } else {
+ mutex_unlock(&fe->remove_mutex);
+ }
+
+ } else {
+ mutex_unlock(&fe->remove_mutex);
}
dvb_frontend_put(fe);
@@ -3022,6 +3056,7 @@ int dvb_register_frontend(struct dvb_adapter *dvb,
fepriv = fe->frontend_priv;
kref_init(&fe->refcount);
+ mutex_init(&fe->remove_mutex);
/*
* After initialization, there need to be two references: one
diff --git a/include/media/dvb_frontend.h b/include/media/dvb_frontend.h
index e7c44870f20de..367d5381217b5 100644
--- a/include/media/dvb_frontend.h
+++ b/include/media/dvb_frontend.h
@@ -686,7 +686,10 @@ struct dtv_frontend_properties {
* @id: Frontend ID
* @exit: Used to inform the DVB core that the frontend
* thread should exit (usually, means that the hardware
- * got disconnected.
+ * got disconnected).
+ * @remove_mutex: mutex that avoids a race condition between a callback
+ * called when the hardware is disconnected and the
+ * file_operations of dvb_frontend.
*/
struct dvb_frontend {
@@ -704,6 +707,7 @@ struct dvb_frontend {
int (*callback)(void *adapter_priv, int component, int cmd, int arg);
int id;
unsigned int exit;
+ struct mutex remove_mutex;
};
/**
--
2.39.2
Powered by blists - more mailing lists