| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 31 Dec 2022 18:31:02 +0800
From: kernel test robot <lkp@...el.com>
To: Yoochan Lee <yoochan1026@...il.com>, matt.hsiao@....com
Cc: oe-kbuild-all@...ts.linux.dev, arnd@...db.de,
gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
Yoochan Lee <yoochan1026@...il.com>
Subject: Re: [PATCH] misc: hpilo: Fix use-after-free in ilo_open
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 <lkp@...el.com>
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
View attachment "config" of type "text/plain" (171966 bytes)
Powered by blists - more mailing lists