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.com
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index cc0a789..947b619 100644 (file)
--- 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 @@ err1:
 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 e7c4487..367d538 100644 (file)
--- 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;
 };
 
 /**