Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Yoochan Lee <yoochan1026@gmail.com>
To:     jirislaby@kernel.org
Cc:     arnd@arndb.de, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org, Yoochan Lee <yoochan1026@gmail.com>
Subject: [PATCH] misc: phantom: Fix use-after-free in phantom_open
Date:   Sat, 31 Dec 2022 15:04:59 +0900
Message-Id: <20221231060459.2041173-1-yoochan1026@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@gmail.com>
---
 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