bugfix for CVE-2022-45884
Dinghao Liu (1): media: dvbdev: Fix memleak in dvb_register_device
Fuqian Huang (1): media: media/dvb: Use kmemdup rather than duplicating its implementation
Hyunwoo Kim (1): media: dvb-core: Fix use-after-free due to race at dvb_register_device()
drivers/media/dvb-core/dvbdev.c | 86 ++++++++++++++++----- drivers/media/dvb-frontends/drx39xyj/drxj.c | 5 +- include/media/dvbdev.h | 15 ++++ 3 files changed, 82 insertions(+), 24 deletions(-)
From: Dinghao Liu dinghao.liu@zju.edu.cn
mainline inclusion from mainline-v5.11-rc1 commit 167faadfcf9339088910e9e85a1b711fcbbef8e9 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I635IG CVE: CVE-2022-45884
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
---------------------------
When device_create() fails, dvbdev and dvbdevfops should be freed just like when dvb_register_media_device() fails.
Signed-off-by: Dinghao Liu dinghao.liu@zju.edu.cn Signed-off-by: Sean Young sean@mess.org Signed-off-by: Mauro Carvalho Chehab mchehab+huawei@kernel.org Signed-off-by: liwei liwei728@huawei.com --- drivers/media/dvb-core/dvbdev.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c index 04dc2f4bc7aa..a652b6b1eb1f 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -541,6 +541,9 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev, 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)); + dvb_media_device_free(dvbdev); + kfree(dvbdevfops); + kfree(dvbdev); return PTR_ERR(clsdev); } dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
From: Fuqian Huang huangfq.daxian@gmail.com
mainline inclusion from mainline-v5.4-rc1 commit f6af820ef1be58c2e4b81aa479b9f109eb6344ce category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I635IG CVE: CVE-2022-45884
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
---------------------------
kmemdup is introduced to duplicate a region of memory in a neat way. Rather than kmalloc/kzalloc + memcpy, which the programmer needs to write the size twice (sometimes lead to mistakes), kmemdup improves readability, leads to smaller code and also reduce the chances of mistakes. Suggestion to use kmemdup rather than using kmalloc/kzalloc + memcpy.
Signed-off-by: Fuqian Huang huangfq.daxian@gmail.com Signed-off-by: Sean Young sean@mess.org Signed-off-by: Mauro Carvalho Chehab mchehab+samsung@kernel.org Signed-off-by: liwei liwei728@huawei.com --- drivers/media/dvb-core/dvbdev.c | 3 +-- drivers/media/dvb-frontends/drx39xyj/drxj.c | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c index a652b6b1eb1f..6daacce17341 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -478,7 +478,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev, return -ENOMEM; }
- dvbdevfops = kzalloc(sizeof(struct file_operations), GFP_KERNEL); + dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL);
if (!dvbdevfops){ kfree (dvbdev); @@ -494,7 +494,6 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev, dvbdev->fops = dvbdevfops; init_waitqueue_head (&dvbdev->wait_queue);
- memcpy(dvbdevfops, template->fops, sizeof(struct file_operations)); dvbdevfops->owner = adap->module;
list_add_tail (&dvbdev->list_head, &adap->device_list); diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 9628d4067fe1..4dae309fdb00 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -12287,7 +12287,8 @@ struct dvb_frontend *drx39xxj_attach(struct i2c_adapter *i2c) if (state == NULL) goto error;
- demod = kmalloc(sizeof(struct drx_demod_instance), GFP_KERNEL); + demod = kmemdup(&drxj_default_demod_g, + sizeof(struct drx_demod_instance), GFP_KERNEL); if (demod == NULL) goto error;
@@ -12311,8 +12312,6 @@ struct dvb_frontend *drx39xxj_attach(struct i2c_adapter *i2c) state->demod = demod;
/* setup the demod data */ - memcpy(demod, &drxj_default_demod_g, sizeof(struct drx_demod_instance)); - demod->my_i2c_dev_addr = demod_addr; demod->my_common_attr = demod_comm_attr; demod->my_i2c_dev_addr->user_data = state;
From: Hyunwoo Kim imv4bel@gmail.com
mainline inclusion from mainline-v6.4-rc4 commit 627bb528b086b4136315c25d6a447a98ea9448d3 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I635IG CVE: CVE-2022-45884
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
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
Conflicts: drivers/media/dvb-core/dvbdev.c include/media/dvbdev.h Signed-off-by: liwei liwei728@huawei.com --- drivers/media/dvb-core/dvbdev.c | 84 ++++++++++++++++++++++++--------- include/media/dvbdev.h | 15 ++++++ 2 files changed, 78 insertions(+), 21 deletions(-)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c index 6daacce17341..a7d57ff7c88b 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -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); @@ -457,14 +458,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__); @@ -472,18 +474,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)); @@ -493,19 +522,19 @@ 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) { - kfree(dvbdevfops); + if (new_node) { + list_del (&new_node->list_head); + kfree(dvbdevfops); + kfree(new_node); + } kfree(dvbdev); up_write(&minor_rwsem); mutex_unlock(&dvbdev_register_lock); @@ -514,40 +543,46 @@ 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] = 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); - kfree(dvbdevfops); kfree(dvbdev); up_write(&minor_rwsem); 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); - 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); @@ -576,7 +611,6 @@ void dvb_free_device(struct dvb_device *dvbdev) if (!dvbdev) return;
- kfree (dvbdev->fops); kfree (dvbdev); } EXPORT_SYMBOL(dvb_free_device); @@ -1058,9 +1092,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); diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h index 881ca461b7bb..8c629938ca44 100644 --- a/include/media/dvbdev.h +++ b/include/media/dvbdev.h @@ -187,6 +187,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_register_adapter - Registers a new DVB adapter *
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/2809 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/7...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/2809 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/7...