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]
Date:	Wed, 10 Aug 2011 14:52:13 -0500
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Jun'ichi Nomura <j-nomura@...jp.nec.com>
Cc:	jaxboe@...ionio.com, roland@...estorage.com,
	stern@...land.harvard.edu, linux-scsi@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	device-mapper development <dm-devel@...hat.com>,
	Kiyoshi Ueda <k-ueda@...jp.nec.com>
Subject: Re: [BUG] Oops when SCSI device under multipath is removed

On Wed, 2011-08-10 at 13:29 +0900, Jun'ichi Nomura wrote:
> Hi James,
> 
> With the attached shell script, blk_insert_cloned_request will cause
> oops with 3.1-rc1. (This is a regression introduced in 2.6.39)
> 
> The regression was introduced by 86cbfb5607d4b81b1a993ff689bbd2addd5d3a9b,
> "[SCSI] put stricter guards on queue dead checks".
> Part of the commit has moved scsi_free_queue(), which calls
> blk_cleanup_queue(), to scsi_remove_device(), which is called while
> the device is still open.
> The oops occurs because blk_insert_cloned_request() is called
> after blk_cleanup_queue() is called (which frees elevator
> and turns on QUEUE_FLAG_DEAD).
> 
> 2 patches have been proposed but neither of them included:
>   1) Add QUEUE_FLAG_DEAD check in blk_insert_cloned_request()
>      https://lkml.org/lkml/2011/7/8/457
>   2) SCSI to call blk_cleanup_queue() from device's ->release() callback
>      (before 2.6.39, it used to work like this)
>      https://lkml.org/lkml/2011/7/2/106

Well, they both have documented objections.  I asked why we destroy the
elevator in the del case and didn't get any traction, so let me show the
actual patch which should fix all of these issues.

Is there a good reason for not doing this as a bug fix now?

James

---
diff --git a/block/blk-core.c b/block/blk-core.c
index b627558..cc72b7d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -367,11 +367,6 @@ void blk_cleanup_queue(struct request_queue *q)
 	queue_flag_set_unlocked(QUEUE_FLAG_DEAD, q);
 	mutex_unlock(&q->sysfs_lock);
 
-	if (q->elevator)
-		elevator_exit(q->elevator);
-
-	blk_throtl_exit(q);
-
 	blk_put_queue(q);
 }
 EXPORT_SYMBOL(blk_cleanup_queue);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 0ee17b5..a5a756b 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -477,6 +477,11 @@ static void blk_release_queue(struct kobject *kobj)
 
 	blk_sync_queue(q);
 
+	if (q->elevator)
+		elevator_exit(q->elevator);
+
+	blk_throtl_exit(q);
+
 	if (rl->rq_pool)
 		mempool_destroy(rl->rq_pool);
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ