linux-kernel - [PATCH] cdrom: gdrom: Fix use-after-free in gdrom

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>] [day] [month] [year] [list]

Message-Id: <20221231064754.2044518-1-yoochan1026@gmail.com>
Date:   Sat, 31 Dec 2022 15:47:54 +0900
From:   Yoochan Lee <yoochan1026@...il.com>
To:     axboe@...nel.dk
Cc:     hare@...e.de, linux-kernel@...r.kernel.org,
        Yoochan Lee <yoochan1026@...il.com>
Subject: [PATCH] cdrom: gdrom: Fix use-after-free in gdrom_open()

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

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

Therefore, add a refcount check to remove_gdrom() function
to free the "gdrom_unit" structure after the device is close()d.

---------------CPU 0--------------------CPU 1-----------------
         gdrom_open()         |      remove_gdrom()
   gdrom_preparedisk_cmd()    |
--------------------------------------------------------------
                              |     kfree(gd.cd_info); — (1)
gdrom_packetcommand(          |
gd.cd_info,                   |
spin_command); — (2)          |

Signed-off-by: Yoochan Lee <yoochan1026@...il.com>
---
 drivers/cdrom/gdrom.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
index ceded5772aac..f392370a16a3 100644
--- a/drivers/cdrom/gdrom.c
+++ b/drivers/cdrom/gdrom.c
@@ -102,6 +102,7 @@ static struct gdrom_unit {
 	struct gdromtoc *toc;
 	struct request_queue *gdrom_rq;
 	struct blk_mq_tag_set tag_set;
+	struct kref refcnt;
 } gd;
 
 struct gdrom_id {
@@ -120,6 +121,19 @@ static int gdrom_packetcommand(struct cdrom_device_info *cd_info,
 	struct packet_command *command);
 static int gdrom_hardreset(struct cdrom_device_info *cd_info);
 
+static void delete_gdrom(struct kref *kref)
+{
+	blk_mq_free_tag_set(&gd.tag_set);
+	free_irq(HW_EVENT_GDROM_CMD, &gd);
+	free_irq(HW_EVENT_GDROM_DMA, &gd);
+	del_gendisk(gd.disk);
+	if (gdrom_major)
+		unregister_blkdev(gdrom_major, GDROM_DEV_NAME);
+	unregister_cdrom(gd.cd_info);
+	kfree(gd.cd_info);
+	kfree(gd.toc);
+}
+
 static bool gdrom_is_busy(void)
 {
 	return (__raw_readb(GDROM_ALTSTATUS_REG) & 0x80) != 0;
@@ -353,6 +367,7 @@ static int gdrom_get_last_session(struct cdrom_device_info *cd_info,
 
 static int gdrom_open(struct cdrom_device_info *cd_info, int purpose)
 {
+	kref_get(&gd.refcnt);
 	/* spin up the disk */
 	return gdrom_preparedisk_cmd();
 }
@@ -360,6 +375,7 @@ static int gdrom_open(struct cdrom_device_info *cd_info, int purpose)
 /* this function is required even if empty */
 static void gdrom_release(struct cdrom_device_info *cd_info)
 {
+	kref_put(&gd.refcnt, delete_gdrom);
 }
 
 static int gdrom_drivestatus(struct cdrom_device_info *cd_info, int ignore)
@@ -808,7 +824,7 @@ static int probe_gdrom(struct platform_device *devptr)
 	err = add_disk(gd.disk);
 	if (err)
 		goto probe_fail_add_disk;
-
+	kref_init(&gd.refcnt);
 	return 0;
 
 probe_fail_add_disk:
@@ -831,16 +847,7 @@ static int probe_gdrom(struct platform_device *devptr)
 
 static int remove_gdrom(struct platform_device *devptr)
 {
-	blk_mq_free_tag_set(&gd.tag_set);
-	free_irq(HW_EVENT_GDROM_CMD, &gd);
-	free_irq(HW_EVENT_GDROM_DMA, &gd);
-	del_gendisk(gd.disk);
-	if (gdrom_major)
-		unregister_blkdev(gdrom_major, GDROM_DEV_NAME);
-	unregister_cdrom(gd.cd_info);
-	kfree(gd.cd_info);
-	kfree(gd.toc);
-
+	kref_put(&gd.refcnt, delete_gdrom);
 	return 0;
 }
 
-- 
2.39.0