[PATCH] misc: phantom: Fix use-after-free in phantom_open

From: Yoochan Lee
Date: Sat Dec 31 2022 - 01:05:23 EST


A race condition may occur if the user physically removes the
phantom device while calling open().

This is a race condition between phantom_open() function and
the phantom_remove() function, which may lead to Use-After-Free.

Therefore, add a refcount check to phantom_remove() function
to free the "phantom_device" structure after the device is close()d.

---------------CPU 0--------------------CPU 1-----------------
phantom_open() | phantom_remove()
--------------------------------------------------------------
struct phantom_device *dev = |
container_of(inode->i_cdev, |
struct phantom_device, cdev); — (1) |
| struct phantom_device *pht
| = pci_get_drvdata(pdev);
| ...
| kfree(pht); — (2)
if (dev->opened) { — (3) |

Signed-off-by: Yoochan Lee <yoochan1026@xxxxxxxxx>
---
drivers/misc/phantom.c | 50 ++++++++++++++++++++++++++----------------
1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/misc/phantom.c b/drivers/misc/phantom.c
index ce72e46a2e73..ee75aa1f56ae 100644
--- a/drivers/misc/phantom.c
+++ b/drivers/misc/phantom.c
@@ -55,6 +55,7 @@ struct phantom_device {
/* used in NOT_OH mode */
struct phm_regs oregs;
u32 ctl_reg;
+ struct kref refcnt;
};

static unsigned char phantom_devices[PHANTOM_MAX_MINORS];
@@ -78,6 +79,32 @@ static int phantom_status(struct phantom_device *dev, unsigned long newstat)
return 0;
}

+static void phantom_delete(struct kref *kref)
+{
+ struct phantom_device *pht = container_of(kref, struct phantom_device, refcnt);
+
+ device_destroy(phantom_class, MKDEV(phantom_major, minor));
+
+ cdev_del(&pht->cdev);
+
+ iowrite32(0, pht->caddr + PHN_IRQCTL);
+ ioread32(pht->caddr + PHN_IRQCTL); /* PCI posting */
+ free_irq(pdev->irq, pht);
+
+ pci_iounmap(pdev, pht->oaddr);
+ pci_iounmap(pdev, pht->iaddr);
+ pci_iounmap(pdev, pht->caddr);
+
+ kfree(pht);
+
+ pci_release_regions(pdev);
+
+ phantom_devices[minor] = 0;
+
+ pci_disable_device(pdev);
+
+}
+
/*
* File ops
*/
@@ -232,6 +259,7 @@ static int phantom_open(struct inode *inode, struct file *file)

atomic_set(&dev->counter, 0);
dev->opened++;
+ kref_get(&dev->refcnt);
mutex_unlock(&dev->open_lock);
mutex_unlock(&phantom_mutex);
return 0;
@@ -247,6 +275,7 @@ static int phantom_release(struct inode *inode, struct file *file)
phantom_status(dev, dev->status & ~PHB_RUNNING);
dev->status &= ~PHB_NOT_OH;

+ kref_put(&dev->refcnt, phantom_delete);
mutex_unlock(&dev->open_lock);

return 0;
@@ -384,6 +413,7 @@ static int phantom_probe(struct pci_dev *pdev,

mutex_init(&pht->open_lock);
spin_lock_init(&pht->regs_lock);
+ kref_init(&pht->refcnt);
init_waitqueue_head(&pht->wait);
cdev_init(&pht->cdev, &phantom_file_ops);
pht->cdev.owner = THIS_MODULE;
@@ -436,25 +466,7 @@ static void phantom_remove(struct pci_dev *pdev)
struct phantom_device *pht = pci_get_drvdata(pdev);
unsigned int minor = MINOR(pht->cdev.dev);

- device_destroy(phantom_class, MKDEV(phantom_major, minor));
-
- cdev_del(&pht->cdev);
-
- iowrite32(0, pht->caddr + PHN_IRQCTL);
- ioread32(pht->caddr + PHN_IRQCTL); /* PCI posting */
- free_irq(pdev->irq, pht);
-
- pci_iounmap(pdev, pht->oaddr);
- pci_iounmap(pdev, pht->iaddr);
- pci_iounmap(pdev, pht->caddr);
-
- kfree(pht);
-
- pci_release_regions(pdev);
-
- phantom_devices[minor] = 0;
-
- pci_disable_device(pdev);
+ kref_put(&pht->refcnt, phantom_delete);
}

static int __maybe_unused phantom_suspend(struct device *dev_d)
--
2.39.0