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 <[email protected]>
---
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
Hi Yoochan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on soc/for-next]
[also build test ERROR on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.2-rc1 next-20221226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yoochan-Lee/char-xilinx_hwicap-xilinx_hwicap-Fix-use-after-free-in-hwicap_open/20221231-112957
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20221231032826.2034288-1-yoochan1026%40gmail.com
patch subject: [PATCH] char: xilinx_hwicap: xilinx_hwicap: Fix use-after-free in hwicap_open
config: microblaze-randconfig-r013-20221226
compiler: microblaze-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/85568c4b21398a7da51b017dbbf78424b59f1f2f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yoochan-Lee/char-xilinx_hwicap-xilinx_hwicap-Fix-use-after-free-in-hwicap_open/20221231-112957
git checkout 85568c4b21398a7da51b017dbbf78424b59f1f2f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/char/xilinx_hwicap/xilinx_hwicap.c: In function 'hwicap_release':
>> drivers/char/xilinx_hwicap/xilinx_hwicap.c:587:36: error: 'hwicap_delete' undeclared (first use in this function); did you mean 'hwicap_release'?
587 | kref_put(&drvdata->refcnt, hwicap_delete);
| ^~~~~~~~~~~~~
| hwicap_release
drivers/char/xilinx_hwicap/xilinx_hwicap.c:587:36: note: each undeclared identifier is reported only once for each function it appears in
drivers/char/xilinx_hwicap/xilinx_hwicap.c: In function 'hwicap_remove':
drivers/char/xilinx_hwicap/xilinx_hwicap.c:736:36: error: 'hwicap_delete' undeclared (first use in this function); did you mean 'hwicap_release'?
736 | kref_put(&drvdata->refcnt, hwicap_delete);
| ^~~~~~~~~~~~~
| hwicap_release
In file included from include/linux/fs.h:7,
from include/linux/huge_mm.h:8,
from include/linux/mm.h:737,
from arch/microblaze/include/asm/io.h:14,
from include/linux/io.h:13,
from include/linux/irq.h:20,
from include/asm-generic/hardirq.h:17,
from ./arch/microblaze/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:11,
from include/linux/interrupt.h:11,
from drivers/char/xilinx_hwicap/xilinx_hwicap.c:78:
drivers/char/xilinx_hwicap/xilinx_hwicap.c: In function 'hwicap_delete':
>> drivers/char/xilinx_hwicap/xilinx_hwicap.c:840:30: error: 'dev' undeclared (first use in this function); did you mean 'cdev'?
840 | probed_devices[MINOR(dev->devt)-XHWICAP_MINOR] = 0;
| ^~~
include/linux/kdev_t.h:11:43: note: in definition of macro 'MINOR'
11 | #define MINOR(dev) ((unsigned int) ((dev) & MINORMASK))
| ^~~
drivers/char/xilinx_hwicap/xilinx_hwicap.c: At top level:
drivers/char/xilinx_hwicap/xilinx_hwicap.c:829:13: warning: 'hwicap_delete' defined but not used [-Wunused-function]
829 | static void hwicap_delete(struct kref *kref)
| ^~~~~~~~~~~~~
vim +587 drivers/char/xilinx_hwicap/xilinx_hwicap.c
564
565 static int hwicap_release(struct inode *inode, struct file *file)
566 {
567 struct hwicap_drvdata *drvdata = file->private_data;
568 int i;
569 int status = 0;
570
571 mutex_lock(&drvdata->sem);
572
573 if (drvdata->write_buffer_in_use) {
574 /* Flush write buffer. */
575 for (i = drvdata->write_buffer_in_use; i < 4; i++)
576 drvdata->write_buffer[i] = 0;
577
578 status = drvdata->config->set_configuration(drvdata,
579 (u32 *) drvdata->write_buffer, 1);
580 if (status)
581 goto error;
582 }
583
584 status = hwicap_command_desync(drvdata);
585 if (status)
586 goto error;
> 587 kref_put(&drvdata->refcnt, hwicap_delete);
588
589 error:
590 drvdata->is_open = 0;
591 mutex_unlock(&drvdata->sem);
592 return status;
593 }
594
--
0-DAY CI Kernel Test Service
https://01.org/lkp
I fix an error in the previous patch.
From ac8e2271ddc68ff53279d6f8d7ae8c114f3cfbf7 Mon Sep 17 00:00:00 2001
From: Yoochan Lee <[email protected]>
Date: Sun, 1 Jan 2023 10:38:48 +0900
Subject: [PATCH 2/2] Fix errors in previous patch
---
drivers/char/xilinx_hwicap/xilinx_hwicap.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
index d93abd99bd37..b219ee3142dc 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
@@ -217,6 +217,7 @@ static const struct config_registers v6_config_registers = {
.CTL_1 = 24,
};
+static void hwicap_delete(struct kref *kref);
/**
* hwicap_command_desync - Send a DESYNC command to the ICAP port.
* @drvdata: a pointer to the drvdata.
@@ -733,6 +734,10 @@ static int hwicap_remove(struct device *dev)
if (!drvdata)
return 0;
+ mutex_lock(&icap_sem);
+ probed_devices[MINOR(dev->devt)-XHWICAP_MINOR] = 0;
+ mutex_unlock(&icap_sem);
+
kref_put(&drvdata->refcnt, hwicap_delete);
return 0; /* success */
@@ -835,10 +840,6 @@ static void hwicap_delete(struct kref *kref)
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
--
2.39.0
2023년 1월 1일 (일) 오전 3:26, kernel test robot <[email protected]>님이 작성:
>
> Hi Yoochan,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on soc/for-next]
> [also build test ERROR on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.2-rc1 next-20221226]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Yoochan-Lee/char-xilinx_hwicap-xilinx_hwicap-Fix-use-after-free-in-hwicap_open/20221231-112957
> base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
> patch link: https://lore.kernel.org/r/20221231032826.2034288-1-yoochan1026%40gmail.com
> patch subject: [PATCH] char: xilinx_hwicap: xilinx_hwicap: Fix use-after-free in hwicap_open
> config: microblaze-randconfig-r013-20221226
> compiler: microblaze-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/85568c4b21398a7da51b017dbbf78424b59f1f2f
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Yoochan-Lee/char-xilinx_hwicap-xilinx_hwicap-Fix-use-after-free-in-hwicap_open/20221231-112957
> git checkout 85568c4b21398a7da51b017dbbf78424b59f1f2f
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> drivers/char/xilinx_hwicap/xilinx_hwicap.c: In function 'hwicap_release':
> >> drivers/char/xilinx_hwicap/xilinx_hwicap.c:587:36: error: 'hwicap_delete' undeclared (first use in this function); did you mean 'hwicap_release'?
> 587 | kref_put(&drvdata->refcnt, hwicap_delete);
> | ^~~~~~~~~~~~~
> | hwicap_release
> drivers/char/xilinx_hwicap/xilinx_hwicap.c:587:36: note: each undeclared identifier is reported only once for each function it appears in
> drivers/char/xilinx_hwicap/xilinx_hwicap.c: In function 'hwicap_remove':
> drivers/char/xilinx_hwicap/xilinx_hwicap.c:736:36: error: 'hwicap_delete' undeclared (first use in this function); did you mean 'hwicap_release'?
> 736 | kref_put(&drvdata->refcnt, hwicap_delete);
> | ^~~~~~~~~~~~~
> | hwicap_release
> In file included from include/linux/fs.h:7,
> from include/linux/huge_mm.h:8,
> from include/linux/mm.h:737,
> from arch/microblaze/include/asm/io.h:14,
> from include/linux/io.h:13,
> from include/linux/irq.h:20,
> from include/asm-generic/hardirq.h:17,
> from ./arch/microblaze/include/generated/asm/hardirq.h:1,
> from include/linux/hardirq.h:11,
> from include/linux/interrupt.h:11,
> from drivers/char/xilinx_hwicap/xilinx_hwicap.c:78:
> drivers/char/xilinx_hwicap/xilinx_hwicap.c: In function 'hwicap_delete':
> >> drivers/char/xilinx_hwicap/xilinx_hwicap.c:840:30: error: 'dev' undeclared (first use in this function); did you mean 'cdev'?
> 840 | probed_devices[MINOR(dev->devt)-XHWICAP_MINOR] = 0;
> | ^~~
> include/linux/kdev_t.h:11:43: note: in definition of macro 'MINOR'
> 11 | #define MINOR(dev) ((unsigned int) ((dev) & MINORMASK))
> | ^~~
> drivers/char/xilinx_hwicap/xilinx_hwicap.c: At top level:
> drivers/char/xilinx_hwicap/xilinx_hwicap.c:829:13: warning: 'hwicap_delete' defined but not used [-Wunused-function]
> 829 | static void hwicap_delete(struct kref *kref)
> | ^~~~~~~~~~~~~
>
>
> vim +587 drivers/char/xilinx_hwicap/xilinx_hwicap.c
>
> 564
> 565 static int hwicap_release(struct inode *inode, struct file *file)
> 566 {
> 567 struct hwicap_drvdata *drvdata = file->private_data;
> 568 int i;
> 569 int status = 0;
> 570
> 571 mutex_lock(&drvdata->sem);
> 572
> 573 if (drvdata->write_buffer_in_use) {
> 574 /* Flush write buffer. */
> 575 for (i = drvdata->write_buffer_in_use; i < 4; i++)
> 576 drvdata->write_buffer[i] = 0;
> 577
> 578 status = drvdata->config->set_configuration(drvdata,
> 579 (u32 *) drvdata->write_buffer, 1);
> 580 if (status)
> 581 goto error;
> 582 }
> 583
> 584 status = hwicap_command_desync(drvdata);
> 585 if (status)
> 586 goto error;
> > 587 kref_put(&drvdata->refcnt, hwicap_delete);
> 588
> 589 error:
> 590 drvdata->is_open = 0;
> 591 mutex_unlock(&drvdata->sem);
> 592 return status;
> 593 }
> 594
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp