Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

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>

authored by

Hyunwoo Kim and committed by
Mauro Carvalho Chehab
627bb528 4172385b

+78 -21
+63 -21
drivers/media/dvb-core/dvbdev.c
··· 27 27 #include <media/tuner.h> 28 28 29 29 static DEFINE_MUTEX(dvbdev_mutex); 30 + static LIST_HEAD(dvbdevfops_list); 30 31 static int dvbdev_debug; 31 32 32 33 module_param(dvbdev_debug, int, 0644); ··· 454 453 enum dvb_device_type type, int demux_sink_pads) 455 454 { 456 455 struct dvb_device *dvbdev; 457 - struct file_operations *dvbdevfops; 456 + struct file_operations *dvbdevfops = NULL; 457 + struct dvbdevfops_node *node = NULL, *new_node = NULL; 458 458 struct device *clsdev; 459 459 int minor; 460 460 int id, ret; 461 461 462 462 mutex_lock(&dvbdev_register_lock); 463 463 464 - if ((id = dvbdev_get_free_id (adap, type)) < 0){ 464 + if ((id = dvbdev_get_free_id (adap, type)) < 0) { 465 465 mutex_unlock(&dvbdev_register_lock); 466 466 *pdvbdev = NULL; 467 467 pr_err("%s: couldn't find free device id\n", __func__); ··· 470 468 } 471 469 472 470 *pdvbdev = dvbdev = kzalloc(sizeof(*dvbdev), GFP_KERNEL); 473 - 474 471 if (!dvbdev){ 475 472 mutex_unlock(&dvbdev_register_lock); 476 473 return -ENOMEM; 477 474 } 478 475 479 - dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL); 476 + /* 477 + * When a device of the same type is probe()d more than once, 478 + * the first allocated fops are used. This prevents memory leaks 479 + * that can occur when the same device is probe()d repeatedly. 480 + */ 481 + list_for_each_entry(node, &dvbdevfops_list, list_head) { 482 + if (node->fops->owner == adap->module && 483 + node->type == type && 484 + node->template == template) { 485 + dvbdevfops = node->fops; 486 + break; 487 + } 488 + } 480 489 481 - if (!dvbdevfops){ 482 - kfree (dvbdev); 483 - mutex_unlock(&dvbdev_register_lock); 484 - return -ENOMEM; 490 + if (dvbdevfops == NULL) { 491 + dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL); 492 + if (!dvbdevfops) { 493 + kfree(dvbdev); 494 + mutex_unlock(&dvbdev_register_lock); 495 + return -ENOMEM; 496 + } 497 + 498 + new_node = kzalloc(sizeof(struct dvbdevfops_node), GFP_KERNEL); 499 + if (!new_node) { 500 + kfree(dvbdevfops); 501 + kfree(dvbdev); 502 + mutex_unlock(&dvbdev_register_lock); 503 + return -ENOMEM; 504 + } 505 + 506 + new_node->fops = dvbdevfops; 507 + new_node->type = type; 508 + new_node->template = template; 509 + list_add_tail (&new_node->list_head, &dvbdevfops_list); 485 510 } 486 511 487 512 memcpy(dvbdev, template, sizeof(struct dvb_device)); ··· 519 490 dvbdev->priv = priv; 520 491 dvbdev->fops = dvbdevfops; 521 492 init_waitqueue_head (&dvbdev->wait_queue); 522 - 523 493 dvbdevfops->owner = adap->module; 524 - 525 494 list_add_tail (&dvbdev->list_head, &adap->device_list); 526 - 527 495 down_write(&minor_rwsem); 528 496 #ifdef CONFIG_DVB_DYNAMIC_MINORS 529 497 for (minor = 0; minor < MAX_DVB_MINORS; minor++) 530 498 if (dvb_minors[minor] == NULL) 531 499 break; 532 - 533 500 if (minor == MAX_DVB_MINORS) { 501 + if (new_node) { 502 + list_del (&new_node->list_head); 503 + kfree(dvbdevfops); 504 + kfree(new_node); 505 + } 534 506 list_del (&dvbdev->list_head); 535 - kfree(dvbdevfops); 536 507 kfree(dvbdev); 537 508 up_write(&minor_rwsem); 538 509 mutex_unlock(&dvbdev_register_lock); ··· 541 512 #else 542 513 minor = nums2minor(adap->num, type, id); 543 514 #endif 544 - 545 515 dvbdev->minor = minor; 546 516 dvb_minors[minor] = dvb_device_get(dvbdev); 547 517 up_write(&minor_rwsem); 548 - 549 518 ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads); 550 519 if (ret) { 551 520 pr_err("%s: dvb_register_media_device failed to create the mediagraph\n", 552 521 __func__); 553 - 522 + if (new_node) { 523 + list_del (&new_node->list_head); 524 + kfree(dvbdevfops); 525 + kfree(new_node); 526 + } 554 527 dvb_media_device_free(dvbdev); 555 528 list_del (&dvbdev->list_head); 556 - kfree(dvbdevfops); 557 529 kfree(dvbdev); 558 530 mutex_unlock(&dvbdev_register_lock); 559 531 return ret; 560 532 } 561 - 562 - mutex_unlock(&dvbdev_register_lock); 563 533 564 534 clsdev = device_create(dvb_class, adap->device, 565 535 MKDEV(DVB_MAJOR, minor), ··· 566 538 if (IS_ERR(clsdev)) { 567 539 pr_err("%s: failed to create device dvb%d.%s%d (%ld)\n", 568 540 __func__, adap->num, dnames[type], id, PTR_ERR(clsdev)); 541 + if (new_node) { 542 + list_del (&new_node->list_head); 543 + kfree(dvbdevfops); 544 + kfree(new_node); 545 + } 569 546 dvb_media_device_free(dvbdev); 570 547 list_del (&dvbdev->list_head); 571 - kfree(dvbdevfops); 572 548 kfree(dvbdev); 549 + mutex_unlock(&dvbdev_register_lock); 573 550 return PTR_ERR(clsdev); 574 551 } 552 + 575 553 dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n", 576 554 adap->num, dnames[type], id, minor, minor); 577 555 556 + mutex_unlock(&dvbdev_register_lock); 578 557 return 0; 579 558 } 580 559 EXPORT_SYMBOL(dvb_register_device); ··· 610 575 { 611 576 struct dvb_device *dvbdev = container_of(ref, struct dvb_device, ref); 612 577 613 - kfree (dvbdev->fops); 614 578 kfree (dvbdev); 615 579 } 616 580 ··· 1115 1081 1116 1082 static void __exit exit_dvbdev(void) 1117 1083 { 1084 + struct dvbdevfops_node *node, *next; 1085 + 1118 1086 class_destroy(dvb_class); 1119 1087 cdev_del(&dvb_device_cdev); 1120 1088 unregister_chrdev_region(MKDEV(DVB_MAJOR, 0), MAX_DVB_MINORS); 1089 + 1090 + list_for_each_entry_safe(node, next, &dvbdevfops_list, list_head) { 1091 + list_del (&node->list_head); 1092 + kfree(node->fops); 1093 + kfree(node); 1094 + } 1121 1095 } 1122 1096 1123 1097 subsys_initcall(init_dvbdev);
+15
include/media/dvbdev.h
··· 194 194 }; 195 195 196 196 /** 197 + * struct dvbdevfops_node - fops nodes registered in dvbdevfops_list 198 + * 199 + * @fops: Dynamically allocated fops for ->owner registration 200 + * @type: type of dvb_device 201 + * @template: dvb_device used for registration 202 + * @list_head: list_head for dvbdevfops_list 203 + */ 204 + struct dvbdevfops_node { 205 + struct file_operations *fops; 206 + enum dvb_device_type type; 207 + const struct dvb_device *template; 208 + struct list_head list_head; 209 + }; 210 + 211 + /** 197 212 * dvb_device_get - Increase dvb_device reference 198 213 * 199 214 * @dvbdev: pointer to struct dvb_device