linux-kernel - [PATCH] misc: hpilo: Fix use after free bug in ilo_remove due to race condition with ilo

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230417165246.467723-1-zyytlz.wz@163.com>
Date:   Tue, 18 Apr 2023 00:52:46 +0800
From:   Zheng Wang <zyytlz.wz@....com>
To:     matt.hsiao@....com
Cc:     arnd@...db.de, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, hackerzheng666@...il.com,
        1395428693sheep@...il.com, alex000young@...il.com,
        Zheng Wang <zyytlz.wz@....com>
Subject: [PATCH] misc: hpilo: Fix use after free bug in ilo_remove due  to race condition with ilo_open

A race condition may occur if the user physically removes the ilo device 
while calling ilo_open.

If we remove the driver which will call ilo_remove to make cleanup, there 
is no guarantee to make sure there's no more ilo_open invoking.

The possible sequence is as follows:

Fix it by adding a refcount check to ilo_remove() function to free the 
"ilo_hwinfo " structure after the last file is closed.

CPU0                  CPU1

                    |ilo_open
ilo_remove          |
kfree(ilo_hw)       |
//free	            |
                    |hw = container_of(ip->i_cdev,
	   	     |struct ilo_hwinfo, cdev);
                    |hw->ccb_alloc[slot]
                    |//use
Fixes: 89bcb05d9bbf ("HP iLO driver")
Signed-off-by: Zheng Wang <zyytlz.wz@....com>
---
 drivers/misc/hpilo.c | 16 +++++++++++++++-
 drivers/misc/hpilo.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/hpilo.c b/drivers/misc/hpilo.c
index 8d00df9243c4..2e9af39e91a4 100644
--- a/drivers/misc/hpilo.c
+++ b/drivers/misc/hpilo.c
@@ -37,6 +37,8 @@ static const struct pci_device_id ilo_blacklist[] = {
 	{}
 };
 
+static void ilo_delete(struct kref *kref);
+
 static inline int get_entry_id(int entry)
 {
 	return (entry & ENTRY_MASK_DESCRIPTOR) >> ENTRY_BITPOS_DESCRIPTOR;
@@ -559,6 +561,7 @@ static int ilo_close(struct inode *ip, struct file *fp)
 		hw->ccb_alloc[slot]->ccb_cnt--;
 
 	spin_unlock(&hw->open_lock);
+	kref_put(&hw->refcnt, ilo_delete);
 
 	return 0;
 }
@@ -578,6 +581,7 @@ static int ilo_open(struct inode *ip, struct file *fp)
 	if (!data)
 		return -ENOMEM;
 
+	kref_get(&hw->refcnt);
 	spin_lock(&hw->open_lock);
 
 	/* each fd private_data holds sw/hw view of ccb */
@@ -633,6 +637,8 @@ static int ilo_open(struct inode *ip, struct file *fp)
 
 	if (!error)
 		fp->private_data = hw->ccb_alloc[slot];
+	else
+		kref_put(&hw->refcnt, ilo_delete);
 
 	return error;
 }
@@ -742,8 +748,15 @@ static int ilo_map_device(struct pci_dev *pdev, struct ilo_hwinfo *hw)
 
 static void ilo_remove(struct pci_dev *pdev)
 {
-	int i, minor;
 	struct ilo_hwinfo *ilo_hw = pci_get_drvdata(pdev);
+	kref_put(&ilo_hw->refcnt, ilo_delete);
+}
+
+static void ilo_delete(struct kref *kref)
+{
+	int i, minor;
+	struct ilo_hwinfo *ilo_hw = container_of(kref, struct ilo_hwinfo, refcnt);
+	struct pci_dev *pdev = ilo_hw->ilo_dev;
 
 	if (!ilo_hw)
 		return;
@@ -807,6 +820,7 @@ static int ilo_probe(struct pci_dev *pdev,
 		goto out;
 
 	ilo_hw->ilo_dev = pdev;
+	kref_init(&ilo_hw->refcnt);
 	spin_lock_init(&ilo_hw->alloc_lock);
 	spin_lock_init(&ilo_hw->fifo_lock);
 	spin_lock_init(&ilo_hw->open_lock);
diff --git a/drivers/misc/hpilo.h b/drivers/misc/hpilo.h
index d57c34680b09..ebc677eb45ae 100644
--- a/drivers/misc/hpilo.h
+++ b/drivers/misc/hpilo.h
@@ -62,6 +62,7 @@ struct ilo_hwinfo {
 	spinlock_t fifo_lock;
 
 	struct cdev cdev;
+	struct kref refcnt;
 };
 
 /* offset from mmio_vaddr for enabling doorbell interrupts */
-- 
2.25.1