From: Hyunwoo Kim imv4bel@gmail.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@gmail.com Signed-off-by: Mauro Carvalho Chehab mchehab@kernel.org Signed-off-by: Sasha Levin sashal@kernel.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; };
/**