linux-kernel - Re: paride/pf.c: blk-mq use-after-free (kernel v5.0)

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

Message-ID: <23f48266-0d12-1891-1373-711b63f4f589@kernel.dk>
Date:   Sat, 16 Mar 2019 13:31:54 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Randy Dunlap <rdunlap@...radead.org>,
        linux-block <linux-block@...r.kernel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, Tim Waugh <tim@...erelk.net>,
        linux-parport@...ts.infradead.org
Subject: Re: paride/pf.c: blk-mq use-after-free (kernel v5.0)

On 3/15/19 6:32 PM, Randy Dunlap wrote:
> On 3/15/19 9:33 AM, Jens Axboe wrote:
>> On 3/14/19 5:49 PM, Randy Dunlap wrote:
>>> On 3/14/19 4:43 PM, Jens Axboe wrote:
>>>> On 3/13/19 5:09 PM, Randy Dunlap wrote:
>>>>> On 3/11/19 6:34 PM, Randy Dunlap wrote:
>>>>>> On 3/11/19 6:25 PM, Randy Dunlap wrote:
>>>>>>> [Has this already been addressed/fixed?]>>
>>>>>>
>>>>>> Same bug occurs with paride/pcd.c driver.
>>>>>
>>>>> This still happens (in blk-mq) in v5.0-11053-gebc551f2b8f9 of Mar. 12, 2019,
>>>>> around 4pm PT.  [caused by paride: pf.c and pcd.c)
>>>>
>>>> I'll take a look at this, been busy with other stuff. How are you
>>>> reproducing this? I'm assuming you don't actually have any hardware :-)
>>>
>>> Right.  I just load the module (pf or pcd), unload it, and
>>> then load it again.
>>
>> Does this work?
>>
> 
> No.  Just loading the pf module gives this:

Missing clear of the queue. This one should be more complete.

To be fair, this was utterly broken since forever. It's just now apparent
since we complain about it. But pf/pcd was one big leak fest.


diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
index 96670eefaeb2..377a694dc228 100644
--- a/drivers/block/paride/pcd.c
+++ b/drivers/block/paride/pcd.c
@@ -749,8 +749,12 @@ static int pcd_detect(void)
 		return 0;
 
 	printk("%s: No CD-ROM drive found\n", name);
-	for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++)
+	for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++) {
+		blk_cleanup_queue(cd->disk->queue);
+		cd->disk->queue = NULL;
+		blk_mq_free_tag_set(&cd->tag_set);
 		put_disk(cd->disk);
+	}
 	pi_unregister_driver(par_drv);
 	return -1;
 }
diff --git a/drivers/block/paride/pf.c b/drivers/block/paride/pf.c
index e92e7a8eeeb2..103b617cdc31 100644
--- a/drivers/block/paride/pf.c
+++ b/drivers/block/paride/pf.c
@@ -761,8 +761,12 @@ static int pf_detect(void)
 		return 0;
 
 	printk("%s: No ATAPI disk detected\n", name);
-	for (pf = units, unit = 0; unit < PF_UNITS; pf++, unit++)
+	for (pf = units, unit = 0; unit < PF_UNITS; pf++, unit++) {
+		blk_cleanup_queue(pf->disk->queue);
+		pf->disk->queue = NULL;
+		blk_mq_free_tag_set(&pf->tag_set);
 		put_disk(pf->disk);
+	}
 	pi_unregister_driver(par_drv);
 	return -1;
 }
@@ -1047,13 +1051,15 @@ static void __exit pf_exit(void)
 	int unit;
 	unregister_blkdev(major, name);
 	for (pf = units, unit = 0; unit < PF_UNITS; pf++, unit++) {
-		if (!pf->present)
-			continue;
-		del_gendisk(pf->disk);
+		if (pf->present)
+			del_gendisk(pf->disk);
+
 		blk_cleanup_queue(pf->disk->queue);
 		blk_mq_free_tag_set(&pf->tag_set);
 		put_disk(pf->disk);
-		pi_release(pf->pi);
+
+		if (pf->present)
+			pi_release(pf->pi);
 	}
 }
 

-- 
Jens Axboe