media: dvbdev: adopts refcnt to avoid UAF

dvb_unregister_device() is known that prone to use-after-free.
That is, the cleanup from dvb_unregister_device() releases the dvb_device
even if there are pointers stored in file->private_data still refer to it.

This patch adds a reference counter into struct dvb_device and delays its
deallocation until no pointer refers to the object.

Link: https://lore.kernel.org/linux-media/20220807145952.10368-1-linma@zju.edu.cn
Signed-off-by: Lin Ma <linma@zju.edu.cn>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 15a08d8c69ef8ec8da87627fde3fb7e411692548..c2d2792227f865ff934f0effa3eda98ecef7802d 100644 (file)
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -157,7 +157,7 @@ static void dvb_ca_private_free(struct dvb_ca_private *ca)
 {
        unsigned int i;
 
-       dvb_free_device(ca->dvbdev);
+       dvb_device_put(ca->dvbdev);
        for (i = 0; i < ca->slot_count; i++)
                vfree(ca->slot_info[i].rx_buffer.data);
 

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 2a98082c605e152199f2c2615b629c89bbf6b815..7ce4785c8d88df5e158cce5e3e5300748af7c400 100644 (file)
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -136,7 +136,7 @@ static void __dvb_frontend_free(struct dvb_frontend *fe)
        struct dvb_frontend_private *fepriv = fe->frontend_priv;
 
        if (fepriv)
-               dvb_free_device(fepriv->dvbdev);
+               dvb_device_put(fepriv->dvbdev);
 
        dvb_frontend_invoke_release(fe, fe->ops.release);
 

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 5b275a9395c12cc9c90c16e7dd14630354c56c06..d45673cb3ce177ad2113c0a472ad7192f8581d00 100644 (file)
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -97,7 +97,7 @@ static int dvb_device_open(struct inode *inode, struct file *file)
                new_fops = fops_get(dvbdev->fops);
                if (!new_fops)
                        goto fail;
-               file->private_data = dvbdev;
+               file->private_data = dvb_device_get(dvbdev);
                replace_fops(file, new_fops);
                if (file->f_op->open)
                        err = file->f_op->open(inode, file);
@@ -161,6 +161,9 @@ int dvb_generic_release(struct inode *inode, struct file *file)
        }
 
        dvbdev->users++;
+
+       dvb_device_put(dvbdev);
+
        return 0;
 }
 EXPORT_SYMBOL(dvb_generic_release);
@@ -479,6 +482,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
                return -ENOMEM;
        }
 
+       kref_init(&dvbdev->ref);
        memcpy(dvbdev, template, sizeof(struct dvb_device));
        dvbdev->type = type;
        dvbdev->id = id;
@@ -510,7 +514,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 #endif
 
        dvbdev->minor = minor;
-       dvb_minors[minor] = dvbdev;
+       dvb_minors[minor] = dvb_device_get(dvbdev);
        up_write(&minor_rwsem);
 
        ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads);
@@ -555,6 +559,7 @@ void dvb_remove_device(struct dvb_device *dvbdev)
 
        down_write(&minor_rwsem);
        dvb_minors[dvbdev->minor] = NULL;
+       dvb_device_put(dvbdev);
        up_write(&minor_rwsem);
 
        dvb_media_device_free(dvbdev);
@@ -566,21 +571,34 @@ void dvb_remove_device(struct dvb_device *dvbdev)
 EXPORT_SYMBOL(dvb_remove_device);
 
 
-void dvb_free_device(struct dvb_device *dvbdev)
+static void dvb_free_device(struct kref *ref)
 {
-       if (!dvbdev)
-               return;
+       struct dvb_device *dvbdev = container_of(ref, struct dvb_device, ref);
 
        kfree (dvbdev->fops);
        kfree (dvbdev);
 }
-EXPORT_SYMBOL(dvb_free_device);
+
+
+struct dvb_device *dvb_device_get(struct dvb_device *dvbdev)
+{
+       kref_get(&dvbdev->ref);
+       return dvbdev;
+}
+EXPORT_SYMBOL(dvb_device_get);
+
+
+void dvb_device_put(struct dvb_device *dvbdev)
+{
+       if (dvbdev)
+               kref_put(&dvbdev->ref, dvb_free_device);
+}
 
 
 void dvb_unregister_device(struct dvb_device *dvbdev)
 {
        dvb_remove_device(dvbdev);
-       dvb_free_device(dvbdev);
+       dvb_device_put(dvbdev);
 }
 EXPORT_SYMBOL(dvb_unregister_device);
 

diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h
index 6ccff7c6fa6b309a0931d9cc8ed09b6a471989f7..fad9871157e2af779ed99c8c7242c363afac4ecc 100644 (file)
--- a/include/media/dvbdev.h
+++ b/include/media/dvbdev.h
@@ -160,6 +160,7 @@ struct dvb_adapter {
  */
 struct dvb_device {
        struct list_head list_head;
+       struct kref ref;
        const struct file_operations *fops;
        struct dvb_adapter *adapter;
        enum dvb_device_type type;
@@ -191,6 +192,20 @@ struct dvb_device {
        void *priv;
 };
 
+/**
+ * dvb_device_get - Increase dvb_device reference
+ *
+ * @dvbdev:    pointer to struct dvb_device
+ */
+struct dvb_device *dvb_device_get(struct dvb_device *dvbdev);
+
+/**
+ * dvb_device_get - Decrease dvb_device reference
+ *
+ * @dvbdev:    pointer to struct dvb_device
+ */
+void dvb_device_put(struct dvb_device *dvbdev);
+
 /**
  * dvb_register_adapter - Registers a new DVB adapter
  *
@@ -235,29 +250,17 @@ int dvb_register_device(struct dvb_adapter *adap,
 /**
  * dvb_remove_device - Remove a registered DVB device
  *
- * This does not free memory.  To do that, call dvb_free_device().
+ * This does not free memory. dvb_free_device() will do that when
+ * reference counter is empty
  *
  * @dvbdev:    pointer to struct dvb_device
  */
 void dvb_remove_device(struct dvb_device *dvbdev);
 
-/**
- * dvb_free_device - Free memory occupied by a DVB device.
- *
- * Call dvb_unregister_device() before calling this function.
- *
- * @dvbdev:    pointer to struct dvb_device
- */
-void dvb_free_device(struct dvb_device *dvbdev);
 
 /**
  * dvb_unregister_device - Unregisters a DVB device
  *
- * This is a combination of dvb_remove_device() and dvb_free_device().
- * Using this function is usually a mistake, and is often an indicator
- * for a use-after-free bug (when a userspace process keeps a file
- * handle to a detached device).
- *
  * @dvbdev:    pointer to struct dvb_device
  */
 void dvb_unregister_device(struct dvb_device *dvbdev);