linux-kernel - [PATCH UPDATED 2/3 v2.6.39-rc7] block: make disk_block_events() properly wait for work cancellation

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: <20110517154748.GS20624@htj.dyndns.org>
Date:	Tue, 17 May 2011 17:47:48 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Jens Axboe <axboe@...nel.dk>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Sitsofe Wheeler <sitsofe@...oo.com>,
	Borislav Petkov <bp@...en8.de>, Meelis Roos <mroos@...ux.ee>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kay Sievers <kay.sievers@...y.org>,
	linux-kernel@...r.kernel.org
Subject: [PATCH UPDATED 2/3 v2.6.39-rc7] block: make disk_block_events()
 properly wait for work cancellation

disk_block_events() should guarantee that the event work is not in
flight on return and once blocked it shouldn't issue further
cancellations.

Because there was no synchronization between the first blocker doing
cancel_delayed_work_sync() and the following blockers, the following
blockers could finish before cancellation was complete, which broke
both guarantees - event work could be in flight and cancellation could
happen after return.

This bug triggered WARN_ON_ONCE() in disk_clear_events() reported in
bug#34662.

  https://bugzilla.kernel.org/show_bug.cgi?id=34662

Fix it by adding an outer mutex which protects both block count
manipulation and work cancellation.

-v2: Use outer mutex instead of bit waitqueue per Linus.

Signed-off-by: Tejun Heo <tj@...nel.org>
Tested-by: Sitsofe Wheeler <sitsofe@...oo.com>
Reported-by: Sitsofe Wheeler <sitsofe@...oo.com>
Reported-by: Borislav Petkov <bp@...en8.de>
Reported-by: Meelis Roos <mroos@...ux.ee>
Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Jens Axboe <axboe@...nel.dk>
Cc: Kay Sievers <kay.sievers@...y.org>
---
Yeap, it's so much simpler this way.  Thanks.

 block/genhd.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: work/block/genhd.c
===================================================================
--- work.orig/block/genhd.c
+++ work/block/genhd.c
@@ -1371,6 +1371,7 @@ struct disk_events {
 	struct gendisk		*disk;		/* the associated disk */
 	spinlock_t		lock;
 
+	struct mutex		block_mutex;	/* protects blocking */
 	int			block;		/* event blocking depth */
 	unsigned int		pending;	/* events already sent out */
 	unsigned int		clearing;	/* events being cleared */
@@ -1438,12 +1439,20 @@ void disk_block_events(struct gendisk *d
 	if (!ev)
 		return;
 
+	/*
+	 * Outer mutex ensures that the first blocker completes canceling
+	 * the event work before further blockers are allowed to finish.
+	 */
+	mutex_lock(&ev->block_mutex);
+
 	spin_lock_irqsave(&ev->lock, flags);
 	cancel = !ev->block++;
 	spin_unlock_irqrestore(&ev->lock, flags);
 
 	if (cancel)
 		cancel_delayed_work_sync(&disk->ev->dwork);
+
+	mutex_unlock(&ev->block_mutex);
 }
 
 static void __disk_unblock_events(struct gendisk *disk, bool check_now)
@@ -1751,6 +1760,7 @@ static void disk_add_events(struct gendi
 	INIT_LIST_HEAD(&ev->node);
 	ev->disk = disk;
 	spin_lock_init(&ev->lock);
+	mutex_init(&ev->block_mutex);
 	ev->block = 1;
 	ev->poll_msecs = -1;
 	INIT_DELAYED_WORK(&ev->dwork, disk_events_workfn);
--
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/