Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a93ae93e authored by Hyunwoo Kim's avatar Hyunwoo Kim Committed by Greg Kroah-Hartman
Browse files

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

commit 627bb528b086b4136315c25d6a447a98ea9448d3 upstream.

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: default avatarHyunwoo Kim <imv4bel@gmail.com>
Reported-by: default avatarkernel test robot <lkp@intel.com>
Reported-by: default avatarDan Carpenter <error27@gmail.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: default avatarOvidiu Panait <ovidiu.panait@windriver.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 225bd8cc

drivers/media/dvb-core/dvbdev.c

+63 −21
Original line number Diff line number Diff line
@@ -37,6 +37,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);
@@ -462,7 +463,8 @@ 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;
@@ -477,20 +479,47 @@ 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 == 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));

	kref_init(&dvbdev->ref);

	dvbdev->type = type;
@@ -499,20 +528,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) {

		list_del (&dvbdev->list_head);

		if (new_node) {

			list_del (&new_node->list_head);

			kfree(dvbdevfops);

			kfree(new_node);

		}

		list_del (&dvbdev->list_head);

		kfree(dvbdev);

		up_write(&minor_rwsem);

		mutex_unlock(&dvbdev_register_lock);
@@ -521,41 +550,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);
@@ -584,7 +619,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);

}
@@ -1090,9 +1124,17 @@ static int __init init_dvbdev(void) 


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);

include/media/dvbdev.h

+15 −0
Original line number Diff line number Diff line
@@ -189,6 +189,21 @@ struct dvb_device { 
	void *priv;

};



/**

 * 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

 *