Messages in this thread Patch in this message |  | | From | Yoochan Lee <> | Subject | [PATCH] misc: hpilo: Fix use-after-free in ilo_open | Date | Sat, 31 Dec 2022 14:53:10 +0900 |
| |
A race condition may occur if the user physically removes the hpilo device while calling open().
This is a race condition between ilo_open() function and the ilo_remove() function, which may lead to Use-After-Free.
Therefore, add a refcount check to ilo_remove() function to free the "ilo_hwinfo" structure after the device is close()d.
---------------CPU 0--------------------CPU 1----------------- ilo_open() | ilo_remove() -------------------------------------------------------------- hw = container_of(ip->i_cdev, | struct ilo_hwinfo, cdev); -(1)| | struct ilo_hwinfo *ilo_hw = pci_get_drvdata(pdev); | ... | kfree(ilo_hw); -(2) spin_lock(&hw->open_lock);-(3)|
Signed-off-by: Yoochan Lee <yoochan1026@gmail.com> --- drivers/misc/hpilo.c | 53 ++++++++++++++++++++++++++------------------ drivers/misc/hpilo.h | 1 + 2 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/misc/hpilo.c b/drivers/misc/hpilo.c index 8d00df9243c4..b4a99676a642 100644 --- a/drivers/misc/hpilo.c +++ b/drivers/misc/hpilo.c @@ -532,6 +532,34 @@ static __poll_t ilo_poll(struct file *fp, poll_table *wait) return 0; } +static void ilo_delete(struct kref *kref) +{ + struct ilo_hwinfo *ilo_hw = container_of(kref, struct ilo_hwinfo, refcnt); + + clear_device(ilo_hw); + + minor = MINOR(ilo_hw->cdev.dev); + for (i = minor; i < minor + max_ccb; i++) + device_destroy(ilo_class, MKDEV(ilo_major, i)); + + cdev_del(&ilo_hw->cdev); + ilo_disable_interrupts(ilo_hw); + free_irq(pdev->irq, ilo_hw); + ilo_unmap_device(pdev, ilo_hw); + pci_release_regions(pdev); + /* + * pci_disable_device(pdev) used to be here. But this PCI device has + * two functions with interrupt lines connected to a single pin. The + * other one is a USB host controller. So when we disable the PIN here + * e.g. by rmmod hpilo, the controller stops working. It is because + * the interrupt link is disabled in ACPI since it is not refcounted + * yet. See acpi_pci_link_free_irq called from acpi_pci_irq_disable. + */ + kfree(ilo_hw); + ilo_hwdev[(minor / max_ccb)] = 0; + +} + static int ilo_close(struct inode *ip, struct file *fp) { int slot; @@ -558,6 +586,7 @@ static int ilo_close(struct inode *ip, struct file *fp) } else hw->ccb_alloc[slot]->ccb_cnt--; + kref_put(&hw->refcnt, ilo_delete); spin_unlock(&hw->open_lock); return 0; @@ -629,6 +658,7 @@ static int ilo_open(struct inode *ip, struct file *fp) } } out: + kref_get(&hw->refcnt); spin_unlock(&hw->open_lock); if (!error) @@ -748,27 +778,7 @@ static void ilo_remove(struct pci_dev *pdev) if (!ilo_hw) return; - clear_device(ilo_hw); - - minor = MINOR(ilo_hw->cdev.dev); - for (i = minor; i < minor + max_ccb; i++) - device_destroy(ilo_class, MKDEV(ilo_major, i)); - - cdev_del(&ilo_hw->cdev); - ilo_disable_interrupts(ilo_hw); - free_irq(pdev->irq, ilo_hw); - ilo_unmap_device(pdev, ilo_hw); - pci_release_regions(pdev); - /* - * pci_disable_device(pdev) used to be here. But this PCI device has - * two functions with interrupt lines connected to a single pin. The - * other one is a USB host controller. So when we disable the PIN here - * e.g. by rmmod hpilo, the controller stops working. It is because - * the interrupt link is disabled in ACPI since it is not refcounted - * yet. See acpi_pci_link_free_irq called from acpi_pci_irq_disable. - */ - kfree(ilo_hw); - ilo_hwdev[(minor / max_ccb)] = 0; + kref_put(&hw->refcnt, ilo_delete); } static int ilo_probe(struct pci_dev *pdev, @@ -810,6 +820,7 @@ static int ilo_probe(struct pci_dev *pdev, spin_lock_init(&ilo_hw->alloc_lock); spin_lock_init(&ilo_hw->fifo_lock); spin_lock_init(&ilo_hw->open_lock); + kref_init(&iol_hw->refcnt); error = pci_enable_device(pdev); if (error) diff --git a/drivers/misc/hpilo.h b/drivers/misc/hpilo.h index d57c34680b09..ebc677eb45ae 100644 --- a/drivers/misc/hpilo.h +++ b/drivers/misc/hpilo.h @@ -62,6 +62,7 @@ struct ilo_hwinfo { spinlock_t fifo_lock; struct cdev cdev; + struct kref refcnt; }; /* offset from mmio_vaddr for enabling doorbell interrupts */ -- 2.39.0
|  |