[PATCH] char: xilinx_hwicap: xilinx_hwicap: Fix use-after-free in hwicap_open

From: Yoochan Lee
Date: Fri Dec 30 2022 - 22:28:40 EST


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@xxxxxxxxx>
---
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