Skip to content

Commit

Permalink
media: dvb-core: Fix use-after-free due to race at dvb_register_device()
Browse files Browse the repository at this point in the history 
[ Upstream commit 627bb52 ]

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>
  • Loading branch information
@V4bel @gregkh
V4bel authored and gregkh committed Jun 9, 2023
1 parent 93b5dfe commit 4a8ecfb
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 21 deletions.
84 changes: 63 additions & 21 deletions drivers/media/dvb-core/dvbdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -452,33 +453,61 @@ 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__);
return -ENFILE;
}

*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));
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -1080,9 +1114,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);
Expand Down
15 changes: 15 additions & 0 deletions include/media/dvbdev.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down

0 comments on commit 4a8ecfb

Please sign in to comment.