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:     axboe@kernel.dk
Cc:     hare@suse.de, linux-kernel@vger.kernel.org,
        Yoochan Lee <yoochan1026@gmail.com>
Subject: [PATCH] cdrom: gdrom: Fix use-after-free in gdrom_open()
Date:   Sat, 31 Dec 2022 15:47:54 +0900
Message-Id: <20221231064754.2044518-1-yoochan1026@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@gmail.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