Hi Yoochan, Thank you for the patch! Yet something to improve: [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.2-rc1 next-20221226] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yoochan-Lee/misc-hpilo-Fix-use-after-free-in-ilo_open/20221231-135458 patch link: https://lore.kernel.org/r/20221231055310.2040648-1-yoochan1026%40gmail.com patch subject: [PATCH] misc: hpilo: Fix use-after-free in ilo_open config: x86_64-rhel-8.3-kselftests compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/aca13e7e71e5c2b68742270a834fd67929850ef9 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yoochan-Lee/misc-hpilo-Fix-use-after-free-in-ilo_open/20221231-135458 git checkout aca13e7e71e5c2b68742270a834fd67929850ef9 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=x86_64 olddefconfig make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/misc/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All error/warnings (new ones prefixed by >>): drivers/misc/hpilo.c: In function 'ilo_delete': >> drivers/misc/hpilo.c:541:9: error: 'minor' undeclared (first use in this function); did you mean 'iminor'? 541 | minor = MINOR(ilo_hw->cdev.dev); | ^~~~~ | iminor drivers/misc/hpilo.c:541:9: note: each undeclared identifier is reported only once for each function it appears in >> drivers/misc/hpilo.c:542:14: error: 'i' undeclared (first use in this function) 542 | for (i = minor; i < minor + max_ccb; i++) | ^ >> drivers/misc/hpilo.c:547:18: error: 'pdev' undeclared (first use in this function); did you mean 'cdev'? 547 | free_irq(pdev->irq, ilo_hw); | ^~~~ | cdev >> drivers/misc/hpilo.c:548:9: error: implicit declaration of function 'ilo_unmap_device' [-Werror=implicit-function-declaration] 548 | ilo_unmap_device(pdev, ilo_hw); | ^~~~~~~~~~~~~~~~ drivers/misc/hpilo.c: At top level: >> drivers/misc/hpilo.c:715:13: warning: conflicting types for 'ilo_unmap_device'; have 'void(struct pci_dev *, struct ilo_hwinfo *)' 715 | static void ilo_unmap_device(struct pci_dev *pdev, struct ilo_hwinfo *hw) | ^~~~~~~~~~~~~~~~ >> drivers/misc/hpilo.c:715:13: error: static declaration of 'ilo_unmap_device' follows non-static declaration drivers/misc/hpilo.c:548:9: note: previous implicit declaration of 'ilo_unmap_device' with type 'void(struct pci_dev *, struct ilo_hwinfo *)' 548 | ilo_unmap_device(pdev, ilo_hw); | ^~~~~~~~~~~~~~~~ drivers/misc/hpilo.c: In function 'ilo_remove': >> drivers/misc/hpilo.c:781:19: error: 'hw' undeclared (first use in this function) 781 | kref_put(&hw->refcnt, ilo_delete); | ^~ >> drivers/misc/hpilo.c:775:16: warning: unused variable 'minor' [-Wunused-variable] 775 | int i, minor; | ^~~~~ >> drivers/misc/hpilo.c:775:13: warning: unused variable 'i' [-Wunused-variable] 775 | int i, minor; | ^ drivers/misc/hpilo.c: In function 'ilo_probe': >> drivers/misc/hpilo.c:823:20: error: 'iol_hw' undeclared (first use in this function); did you mean 'ilo_hw'? 823 | kref_init(&iol_hw->refcnt); | ^~~~~~ | ilo_hw cc1: some warnings being treated as errors vim +541 drivers/misc/hpilo.c 534 535 static void ilo_delete(struct kref *kref) 536 { 537 struct ilo_hwinfo *ilo_hw = container_of(kref, struct ilo_hwinfo, refcnt); 538 539 clear_device(ilo_hw); 540 > 541 minor = MINOR(ilo_hw->cdev.dev); > 542 for (i = minor; i < minor + max_ccb; i++) 543 device_destroy(ilo_class, MKDEV(ilo_major, i)); 544 545 cdev_del(&ilo_hw->cdev); 546 ilo_disable_interrupts(ilo_hw); > 547 free_irq(pdev->irq, ilo_hw); > 548 ilo_unmap_device(pdev, ilo_hw); 549 pci_release_regions(pdev); 550 /* 551 * pci_disable_device(pdev) used to be here. But this PCI device has 552 * two functions with interrupt lines connected to a single pin. The 553 * other one is a USB host controller. So when we disable the PIN here 554 * e.g. by rmmod hpilo, the controller stops working. It is because 555 * the interrupt link is disabled in ACPI since it is not refcounted 556 * yet. See acpi_pci_link_free_irq called from acpi_pci_irq_disable. 557 */ 558 kfree(ilo_hw); 559 ilo_hwdev[(minor / max_ccb)] = 0; 560 561 } 562 563 static int ilo_close(struct inode *ip, struct file *fp) 564 { 565 int slot; 566 struct ccb_data *data; 567 struct ilo_hwinfo *hw; 568 unsigned long flags; 569 570 slot = iminor(ip) % max_ccb; 571 hw = container_of(ip->i_cdev, struct ilo_hwinfo, cdev); 572 573 spin_lock(&hw->open_lock); 574 575 if (hw->ccb_alloc[slot]->ccb_cnt == 1) { 576 577 data = fp->private_data; 578 579 spin_lock_irqsave(&hw->alloc_lock, flags); 580 hw->ccb_alloc[slot] = NULL; 581 spin_unlock_irqrestore(&hw->alloc_lock, flags); 582 583 ilo_ccb_close(hw->ilo_dev, data); 584 585 kfree(data); 586 } else 587 hw->ccb_alloc[slot]->ccb_cnt--; 588 589 kref_put(&hw->refcnt, ilo_delete); 590 spin_unlock(&hw->open_lock); 591 592 return 0; 593 } 594 595 static int ilo_open(struct inode *ip, struct file *fp) 596 { 597 int slot, error; 598 struct ccb_data *data; 599 struct ilo_hwinfo *hw; 600 unsigned long flags; 601 602 slot = iminor(ip) % max_ccb; 603 hw = container_of(ip->i_cdev, struct ilo_hwinfo, cdev); 604 605 /* new ccb allocation */ 606 data = kzalloc(sizeof(*data), GFP_KERNEL); 607 if (!data) 608 return -ENOMEM; 609 610 spin_lock(&hw->open_lock); 611 612 /* each fd private_data holds sw/hw view of ccb */ 613 if (hw->ccb_alloc[slot] == NULL) { 614 /* create a channel control block for this minor */ 615 error = ilo_ccb_setup(hw, data, slot); 616 if (error) { 617 kfree(data); 618 goto out; 619 } 620 621 data->ccb_cnt = 1; 622 data->ccb_excl = fp->f_flags & O_EXCL; 623 data->ilo_hw = hw; 624 init_waitqueue_head(&data->ccb_waitq); 625 626 /* write the ccb to hw */ 627 spin_lock_irqsave(&hw->alloc_lock, flags); 628 ilo_ccb_open(hw, data, slot); 629 hw->ccb_alloc[slot] = data; 630 spin_unlock_irqrestore(&hw->alloc_lock, flags); 631 632 /* make sure the channel is functional */ 633 error = ilo_ccb_verify(hw, data); 634 if (error) { 635 636 spin_lock_irqsave(&hw->alloc_lock, flags); 637 hw->ccb_alloc[slot] = NULL; 638 spin_unlock_irqrestore(&hw->alloc_lock, flags); 639 640 ilo_ccb_close(hw->ilo_dev, data); 641 642 kfree(data); 643 goto out; 644 } 645 646 } else { 647 kfree(data); 648 if (fp->f_flags & O_EXCL || hw->ccb_alloc[slot]->ccb_excl) { 649 /* 650 * The channel exists, and either this open 651 * or a previous open of this channel wants 652 * exclusive access. 653 */ 654 error = -EBUSY; 655 } else { 656 hw->ccb_alloc[slot]->ccb_cnt++; 657 error = 0; 658 } 659 } 660 out: 661 kref_get(&hw->refcnt); 662 spin_unlock(&hw->open_lock); 663 664 if (!error) 665 fp->private_data = hw->ccb_alloc[slot]; 666 667 return error; 668 } 669 670 static const struct file_operations ilo_fops = { 671 .owner = THIS_MODULE, 672 .read = ilo_read, 673 .write = ilo_write, 674 .poll = ilo_poll, 675 .open = ilo_open, 676 .release = ilo_close, 677 .llseek = noop_llseek, 678 }; 679 680 static irqreturn_t ilo_isr(int irq, void *data) 681 { 682 struct ilo_hwinfo *hw = data; 683 int pending, i; 684 685 spin_lock(&hw->alloc_lock); 686 687 /* check for ccbs which have data */ 688 pending = get_device_outbound(hw); 689 if (!pending) { 690 spin_unlock(&hw->alloc_lock); 691 return IRQ_NONE; 692 } 693 694 if (is_db_reset(pending)) { 695 /* wake up all ccbs if the device was reset */ 696 pending = -1; 697 ilo_set_reset(hw); 698 } 699 700 for (i = 0; i < max_ccb; i++) { 701 if (!hw->ccb_alloc[i]) 702 continue; 703 if (pending & (1 << i)) 704 wake_up_interruptible(&hw->ccb_alloc[i]->ccb_waitq); 705 } 706 707 /* clear the device of the channels that have been handled */ 708 clear_pending_db(hw, pending); 709 710 spin_unlock(&hw->alloc_lock); 711 712 return IRQ_HANDLED; 713 } 714 > 715 static void ilo_unmap_device(struct pci_dev *pdev, struct ilo_hwinfo *hw) 716 { 717 pci_iounmap(pdev, hw->db_vaddr); 718 pci_iounmap(pdev, hw->ram_vaddr); 719 pci_iounmap(pdev, hw->mmio_vaddr); 720 } 721 -- 0-DAY CI Kernel Test Service https://01.org/lkp