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:     arnd@arndb.de
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Yoochan Lee <yoochan1026@gmail.com>
Subject: [PATCH] char: xilinx_hwicap: xilinx_hwicap: Fix use-after-free in hwicap_open
Date:   Sat, 31 Dec 2022 12:28:26 +0900
Message-Id: <20221231032826.2034288-1-yoochan1026@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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

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

Therefore, add a refcount check to hwicap_remove() to free the
"hwicap_drvdata" structure after the device is closed.

Signed-off-by: Yoochan Lee <yoochan1026@gmail.com>
---
 drivers/char/xilinx_hwicap/xilinx_hwicap.c | 27 +++++++++++++++-------
 drivers/char/xilinx_hwicap/xilinx_hwicap.h |  1 +
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
index 74a4928aea1d..d93abd99bd37 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
@@ -553,6 +553,7 @@ static int hwicap_open(struct inode *inode, struct file *file)
 	drvdata->write_buffer_in_use = 0;
 	drvdata->read_buffer_in_use = 0;
 	drvdata->is_open = 1;
+	kref_get(&drvdata->refcnt);
 
  error:
 	mutex_unlock(&drvdata->sem);
@@ -583,6 +584,7 @@ static int hwicap_release(struct inode *inode, struct file *file)
 	status = hwicap_command_desync(drvdata);
 	if (status)
 		goto error;
+	kref_put(&drvdata->refcnt, hwicap_delete);
 
  error:
 	drvdata->is_open = 0;
@@ -672,6 +674,7 @@ static int hwicap_setup(struct device *dev, int id,
 	drvdata->config_regs = config_regs;
 
 	mutex_init(&drvdata->sem);
+	kref_init(&drvdata->refcnt);
 	drvdata->is_open = 0;
 
 	dev_info(dev, "ioremap %llx to %p with size %llx\n",
@@ -730,15 +733,8 @@ static int hwicap_remove(struct device *dev)
 	if (!drvdata)
 		return 0;
 
-	device_destroy(icap_class, drvdata->devt);
-	cdev_del(&drvdata->cdev);
-	iounmap(drvdata->base_address);
-	release_mem_region(drvdata->mem_start, drvdata->mem_size);
-	kfree(drvdata);
+	kref_put(&drvdata->refcnt, hwicap_delete);
 
-	mutex_lock(&icap_sem);
-	probed_devices[MINOR(dev->devt)-XHWICAP_MINOR] = 0;
-	mutex_unlock(&icap_sem);
 	return 0;		/* success */
 }
 
@@ -830,6 +826,21 @@ static int hwicap_drv_remove(struct platform_device *pdev)
 	return hwicap_remove(&pdev->dev);
 }
 
+static void hwicap_delete(struct kref *kref)
+{
+	struct hwicap_drvdata *drvdata = container_of(kref, struct hwicap_drvdata, refcnt);
+
+	device_destroy(icap_class, drvdata->devt);
+	cdev_del(&drvdata->cdev);
+	iounmap(drvdata->base_address);
+	release_mem_region(drvdata->mem_start, drvdata->mem_size);
+	kfree(drvdata);
+
+	mutex_lock(&icap_sem);
+	probed_devices[MINOR(dev->devt)-XHWICAP_MINOR] = 0;
+	mutex_unlock(&icap_sem);
+}
+
 #ifdef CONFIG_OF
 /* Match table for device tree binding */
 static const struct of_device_id hwicap_of_match[] = {
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
index 6b963d1c8ba3..9ea3a98ea600 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
@@ -58,6 +58,7 @@ struct hwicap_drvdata {
 	void *private_data;
 	bool is_open;
 	struct mutex sem;
+	struct kref refcnt;
 };
 
 struct hwicap_driver_config {
-- 
2.39.0