media: dvb-core: Fix use-after-free due to race at dvb_register_device()

[ Upstream commit 627bb528b086b4136315c25d6a447a98ea9448d3 ]

dvb_register_device() dynamically allocates fops with kmemdup()
to set the fops->owner.
And these fops are registered in 'file->f_ops' using replace_fops()
in the dvb_device_open() process, and kfree()d in dvb_free_device().

However, it is not common to use dynamically allocated fops instead
of 'static const' fops as an argument of replace_fops(),
and UAF may occur.
These UAFs can occur on any dvb type using dvb_register_device(),
such as dvb_dvr, dvb_demux, dvb_frontend, dvb_net, etc.

So, instead of kfree() the fops dynamically allocated in
dvb_register_device() in dvb_free_device() called during the
.disconnect() process, kfree() it collectively in exit_dvbdev()
called when the dvbdev.c module is removed.

Link: https://lore.kernel.org/linux-media/20221117045925.14297-4-imv4bel@gmail.com
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index a31d52c..9f9a976 100644 (file)
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -27,6 +27,7 @@
 #include <media/tuner.h>
 
 static DEFINE_MUTEX(dvbdev_mutex);
+static LIST_HEAD(dvbdevfops_list);
 static int dvbdev_debug;
 
 module_param(dvbdev_debug, int, 0644);
@@ -452,14 +453,15 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
                        enum dvb_device_type type, int demux_sink_pads)
 {
        struct dvb_device *dvbdev;
-       struct file_operations *dvbdevfops;
+       struct file_operations *dvbdevfops = NULL;
+       struct dvbdevfops_node *node = NULL, *new_node = NULL;
        struct device *clsdev;
        int minor;
        int id, ret;
 
        mutex_lock(&dvbdev_register_lock);
 
-       if ((id = dvbdev_get_free_id (adap, type)) < 0){
+       if ((id = dvbdev_get_free_id (adap, type)) < 0) {
                mutex_unlock(&dvbdev_register_lock);
                *pdvbdev = NULL;
                pr_err("%s: couldn't find free device id\n", __func__);
@@ -467,18 +469,45 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
        }
 
        *pdvbdev = dvbdev = kzalloc(sizeof(*dvbdev), GFP_KERNEL);
-
        if (!dvbdev){
                mutex_unlock(&dvbdev_register_lock);
                return -ENOMEM;
        }
 
-       dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL);
+       /*
+        * When a device of the same type is probe()d more than once,
+        * the first allocated fops are used. This prevents memory leaks
+        * that can occur when the same device is probe()d repeatedly.
+        */
+       list_for_each_entry(node, &dvbdevfops_list, list_head) {
+               if (node->fops->owner == adap->module &&
+                               node->type == type &&
+                               node->template == template) {
+                       dvbdevfops = node->fops;
+                       break;
+               }
+       }
 
-       if (!dvbdevfops){
-               kfree (dvbdev);
-               mutex_unlock(&dvbdev_register_lock);
-               return -ENOMEM;
+       if (dvbdevfops == NULL) {
+               dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL);
+               if (!dvbdevfops) {
+                       kfree(dvbdev);
+                       mutex_unlock(&dvbdev_register_lock);
+                       return -ENOMEM;
+               }
+
+               new_node = kzalloc(sizeof(struct dvbdevfops_node), GFP_KERNEL);
+               if (!new_node) {
+                       kfree(dvbdevfops);
+                       kfree(dvbdev);
+                       mutex_unlock(&dvbdev_register_lock);
+                       return -ENOMEM;
+               }
+
+               new_node->fops = dvbdevfops;
+               new_node->type = type;
+               new_node->template = template;
+               list_add_tail (&new_node->list_head, &dvbdevfops_list);
        }
 
        memcpy(dvbdev, template, sizeof(struct dvb_device));
@@ -489,20 +518,20 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
        dvbdev->priv = priv;
        dvbdev->fops = dvbdevfops;
        init_waitqueue_head (&dvbdev->wait_queue);
-
        dvbdevfops->owner = adap->module;
-
        list_add_tail (&dvbdev->list_head, &adap->device_list);
-
        down_write(&minor_rwsem);
 #ifdef CONFIG_DVB_DYNAMIC_MINORS
        for (minor = 0; minor < MAX_DVB_MINORS; minor++)
                if (dvb_minors[minor] == NULL)
                        break;
-
        if (minor == MAX_DVB_MINORS) {
+               if (new_node) {
+                       list_del (&new_node->list_head);
+                       kfree(dvbdevfops);
+                       kfree(new_node);
+               }
                list_del (&dvbdev->list_head);
-               kfree(dvbdevfops);
                kfree(dvbdev);
                up_write(&minor_rwsem);
                mutex_unlock(&dvbdev_register_lock);
@@ -511,41 +540,47 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 #else
        minor = nums2minor(adap->num, type, id);
 #endif
-
        dvbdev->minor = minor;
        dvb_minors[minor] = dvb_device_get(dvbdev);
        up_write(&minor_rwsem);
-
        ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads);
        if (ret) {
                pr_err("%s: dvb_register_media_device failed to create the mediagraph\n",
                      __func__);
-
+               if (new_node) {
+                       list_del (&new_node->list_head);
+                       kfree(dvbdevfops);
+                       kfree(new_node);
+               }
                dvb_media_device_free(dvbdev);
                list_del (&dvbdev->list_head);
-               kfree(dvbdevfops);
                kfree(dvbdev);
                mutex_unlock(&dvbdev_register_lock);
                return ret;
        }
 
-       mutex_unlock(&dvbdev_register_lock);
-
        clsdev = device_create(dvb_class, adap->device,
                               MKDEV(DVB_MAJOR, minor),
                               dvbdev, "dvb%d.%s%d", adap->num, dnames[type], id);
        if (IS_ERR(clsdev)) {
                pr_err("%s: failed to create device dvb%d.%s%d (%ld)\n",
                       __func__, adap->num, dnames[type], id, PTR_ERR(clsdev));
+               if (new_node) {
+                       list_del (&new_node->list_head);
+                       kfree(dvbdevfops);
+                       kfree(new_node);
+               }
                dvb_media_device_free(dvbdev);
                list_del (&dvbdev->list_head);
-               kfree(dvbdevfops);
                kfree(dvbdev);
+               mutex_unlock(&dvbdev_register_lock);
                return PTR_ERR(clsdev);
        }
+
        dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
                adap->num, dnames[type], id, minor, minor);
 
+       mutex_unlock(&dvbdev_register_lock);
        return 0;
 }
 EXPORT_SYMBOL(dvb_register_device);
@@ -574,7 +609,6 @@ static void dvb_free_device(struct kref *ref)
 {
        struct dvb_device *dvbdev = container_of(ref, struct dvb_device, ref);
 
-       kfree (dvbdev->fops);
        kfree (dvbdev);
 }
 
@@ -1080,9 +1114,17 @@ error:
 
 static void __exit exit_dvbdev(void)
 {
+       struct dvbdevfops_node *node, *next;
+
        class_destroy(dvb_class);
        cdev_del(&dvb_device_cdev);
        unregister_chrdev_region(MKDEV(DVB_MAJOR, 0), MAX_DVB_MINORS);
+
+       list_for_each_entry_safe(node, next, &dvbdevfops_list, list_head) {
+               list_del (&node->list_head);
+               kfree(node->fops);
+               kfree(node);
+       }
 }
 
 subsys_initcall(init_dvbdev);

diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h
index ac60c9f..34b01eb 100644 (file)
--- a/include/media/dvbdev.h
+++ b/include/media/dvbdev.h
@@ -190,6 +190,21 @@ struct dvb_device {
 };
 
 /**
+ * struct dvbdevfops_node - fops nodes registered in dvbdevfops_list
+ *
+ * @fops:              Dynamically allocated fops for ->owner registration
+ * @type:              type of dvb_device
+ * @template:          dvb_device used for registration
+ * @list_head:         list_head for dvbdevfops_list
+ */
+struct dvbdevfops_node {
+       struct file_operations *fops;
+       enum dvb_device_type type;
+       const struct dvb_device *template;
+       struct list_head list_head;
+};
+
+/**
  * dvb_device_get - Increase dvb_device reference
  *
  * @dvbdev:    pointer to struct dvb_device