Commit 6769a0b7 authored by Hyunwoo Kim's avatar Hyunwoo Kim Committed by Mauro Carvalho Chehab

media: dvb-core: Fix use-after-free on race condition at dvb_frontend

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.comSigned-off-by: default avatarHyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
parent ae11c0ef
...@@ -809,15 +809,26 @@ static void dvb_frontend_stop(struct dvb_frontend *fe) ...@@ -809,15 +809,26 @@ static void dvb_frontend_stop(struct dvb_frontend *fe)
dev_dbg(fe->dvb->device, "%s:\n", __func__); dev_dbg(fe->dvb->device, "%s:\n", __func__);
mutex_lock(&fe->remove_mutex);
if (fe->exit != DVB_FE_DEVICE_REMOVED) if (fe->exit != DVB_FE_DEVICE_REMOVED)
fe->exit = DVB_FE_NORMAL_EXIT; fe->exit = DVB_FE_NORMAL_EXIT;
mb(); mb();
if (!fepriv->thread) if (!fepriv->thread) {
mutex_unlock(&fe->remove_mutex);
return; return;
}
kthread_stop(fepriv->thread); 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); sema_init(&fepriv->sem, 1);
fepriv->state = FESTATE_IDLE; fepriv->state = FESTATE_IDLE;
...@@ -2761,9 +2772,13 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) ...@@ -2761,9 +2772,13 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
struct dvb_adapter *adapter = fe->dvb; struct dvb_adapter *adapter = fe->dvb;
int ret; int ret;
mutex_lock(&fe->remove_mutex);
dev_dbg(fe->dvb->device, "%s:\n", __func__); dev_dbg(fe->dvb->device, "%s:\n", __func__);
if (fe->exit == DVB_FE_DEVICE_REMOVED) if (fe->exit == DVB_FE_DEVICE_REMOVED) {
return -ENODEV; ret = -ENODEV;
goto err_remove_mutex;
}
if (adapter->mfe_shared == 2) { if (adapter->mfe_shared == 2) {
mutex_lock(&adapter->mfe_lock); mutex_lock(&adapter->mfe_lock);
...@@ -2771,7 +2786,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) ...@@ -2771,7 +2786,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
if (adapter->mfe_dvbdev && if (adapter->mfe_dvbdev &&
!adapter->mfe_dvbdev->writers) { !adapter->mfe_dvbdev->writers) {
mutex_unlock(&adapter->mfe_lock); mutex_unlock(&adapter->mfe_lock);
return -EBUSY; ret = -EBUSY;
goto err_remove_mutex;
} }
adapter->mfe_dvbdev = dvbdev; adapter->mfe_dvbdev = dvbdev;
} }
...@@ -2794,8 +2810,10 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) ...@@ -2794,8 +2810,10 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
while (mferetry-- && (mfedev->users != -1 || while (mferetry-- && (mfedev->users != -1 ||
mfepriv->thread)) { mfepriv->thread)) {
if (msleep_interruptible(500)) { if (msleep_interruptible(500)) {
if (signal_pending(current)) if (signal_pending(current)) {
return -EINTR; ret = -EINTR;
goto err_remove_mutex;
}
} }
} }
...@@ -2807,7 +2825,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) ...@@ -2807,7 +2825,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
if (mfedev->users != -1 || if (mfedev->users != -1 ||
mfepriv->thread) { mfepriv->thread) {
mutex_unlock(&adapter->mfe_lock); mutex_unlock(&adapter->mfe_lock);
return -EBUSY; ret = -EBUSY;
goto err_remove_mutex;
} }
adapter->mfe_dvbdev = dvbdev; adapter->mfe_dvbdev = dvbdev;
} }
...@@ -2866,6 +2885,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) ...@@ -2866,6 +2885,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
if (adapter->mfe_shared) if (adapter->mfe_shared)
mutex_unlock(&adapter->mfe_lock); mutex_unlock(&adapter->mfe_lock);
mutex_unlock(&fe->remove_mutex);
return ret; return ret;
err3: err3:
...@@ -2887,6 +2908,9 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) ...@@ -2887,6 +2908,9 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
err0: err0:
if (adapter->mfe_shared) if (adapter->mfe_shared)
mutex_unlock(&adapter->mfe_lock); mutex_unlock(&adapter->mfe_lock);
err_remove_mutex:
mutex_unlock(&fe->remove_mutex);
return ret; return ret;
} }
...@@ -2897,6 +2921,8 @@ static int dvb_frontend_release(struct inode *inode, struct file *file) ...@@ -2897,6 +2921,8 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
struct dvb_frontend_private *fepriv = fe->frontend_priv; struct dvb_frontend_private *fepriv = fe->frontend_priv;
int ret; int ret;
mutex_lock(&fe->remove_mutex);
dev_dbg(fe->dvb->device, "%s:\n", __func__); dev_dbg(fe->dvb->device, "%s:\n", __func__);
if ((file->f_flags & O_ACCMODE) != O_RDONLY) { if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
...@@ -2918,10 +2944,18 @@ static int dvb_frontend_release(struct inode *inode, struct file *file) ...@@ -2918,10 +2944,18 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
} }
mutex_unlock(&fe->dvb->mdev_lock); mutex_unlock(&fe->dvb->mdev_lock);
#endif #endif
if (fe->exit != DVB_FE_NO_EXIT)
wake_up(&dvbdev->wait_queue);
if (fe->ops.ts_bus_ctrl) if (fe->ops.ts_bus_ctrl)
fe->ops.ts_bus_ctrl(fe, 0); 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); dvb_frontend_put(fe);
...@@ -3022,6 +3056,7 @@ int dvb_register_frontend(struct dvb_adapter *dvb, ...@@ -3022,6 +3056,7 @@ int dvb_register_frontend(struct dvb_adapter *dvb,
fepriv = fe->frontend_priv; fepriv = fe->frontend_priv;
kref_init(&fe->refcount); kref_init(&fe->refcount);
mutex_init(&fe->remove_mutex);
/* /*
* After initialization, there need to be two references: one * After initialization, there need to be two references: one
......
...@@ -686,7 +686,10 @@ struct dtv_frontend_properties { ...@@ -686,7 +686,10 @@ struct dtv_frontend_properties {
* @id: Frontend ID * @id: Frontend ID
* @exit: Used to inform the DVB core that the frontend * @exit: Used to inform the DVB core that the frontend
* thread should exit (usually, means that the hardware * 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 { struct dvb_frontend {
...@@ -704,6 +707,7 @@ struct dvb_frontend { ...@@ -704,6 +707,7 @@ struct dvb_frontend {
int (*callback)(void *adapter_priv, int component, int cmd, int arg); int (*callback)(void *adapter_priv, int component, int cmd, int arg);
int id; int id;
unsigned int exit; unsigned int exit;
struct mutex remove_mutex;
}; };
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment