linux-kernel - [PATCH 4.19 56/86] dm: report suspended device during destroy

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: <20200310124533.817196498@linuxfoundation.org>
Date:   Tue, 10 Mar 2020 13:45:20 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Corey Marthaler <cmarthal@...hat.com>,
        Mikulas Patocka <mpatocka@...hat.com>,
        Mike Snitzer <snitzer@...hat.com>
Subject: [PATCH 4.19 56/86] dm: report suspended device during destroy

From: Mikulas Patocka <mpatocka@...hat.com>

commit adc0daad366b62ca1bce3e2958a40b0b71a8b8b3 upstream.

The function dm_suspended returns true if the target is suspended.
However, when the target is being suspended during unload, it returns
false.

An example where this is a problem: the test "!dm_suspended(wc->ti)" in
writecache_writeback is not sufficient, because dm_suspended returns
zero while writecache_suspend is in progress.  As is, without an
enhanced dm_suspended, simply switching from flush_workqueue to
drain_workqueue still emits warnings:
workqueue writecache-writeback: drain_workqueue() isn't complete after 10 tries
workqueue writecache-writeback: drain_workqueue() isn't complete after 100 tries
workqueue writecache-writeback: drain_workqueue() isn't complete after 200 tries
workqueue writecache-writeback: drain_workqueue() isn't complete after 300 tries
workqueue writecache-writeback: drain_workqueue() isn't complete after 400 tries

writecache_suspend calls flush_workqueue(wc->writeback_wq) - this function
flushes the current work. However, the workqueue may re-queue itself and
flush_workqueue doesn't wait for re-queued works to finish. Because of
this - the function writecache_writeback continues execution after the
device was suspended and then concurrently with writecache_dtr, causing
a crash in writecache_writeback.

We must use drain_workqueue - that waits until the work and all re-queued
works finish.

As a prereq for switching to drain_workqueue, this commit fixes
dm_suspended to return true after the presuspend hook and before the
postsuspend hook - just like during a normal suspend. It allows
simplifying the dm-integrity and dm-writecache targets so that they
don't have to maintain suspended flags on their own.

With this change use of drain_workqueue() can be used effectively.  This
change was tested with the lvm2 testsuite and cryptsetup testsuite and
the are no regressions.

Fixes: 48debafe4f2f ("dm: add writecache target")
Cc: stable@...r.kernel.org # 4.18+
Reported-by: Corey Marthaler <cmarthal@...hat.com>
Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
Signed-off-by: Mike Snitzer <snitzer@...hat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 drivers/md/dm-integrity.c  |   12 +++++-------
 drivers/md/dm-writecache.c |    2 +-
 drivers/md/dm.c            |    1 +
 3 files changed, 7 insertions(+), 8 deletions(-)

--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -186,12 +186,13 @@ struct dm_integrity_c {
 	__u8 sectors_per_block;

 	unsigned char mode;
-	int suspending;

 	int failed;

 	struct crypto_shash *internal_hash;

+	struct dm_target *ti;
+
 	/* these variables are locked with endio_wait.lock */
 	struct rb_root in_progress;
 	struct list_head wait_list;
@@ -2080,7 +2081,7 @@ static void integrity_writer(struct work
 	unsigned prev_free_sectors;

 	/* the following test is not needed, but it tests the replay code */
-	if (READ_ONCE(ic->suspending) && !ic->meta_dev)
+	if (unlikely(dm_suspended(ic->ti)) && !ic->meta_dev)
 		return;

 	spin_lock_irq(&ic->endio_wait.lock);
@@ -2139,7 +2140,7 @@ static void integrity_recalc(struct work

 next_chunk:

-	if (unlikely(READ_ONCE(ic->suspending)))
+	if (unlikely(dm_suspended(ic->ti)))
 		goto unlock_ret;

 	range.logical_sector = le64_to_cpu(ic->sb->recalc_sector);
@@ -2411,8 +2412,6 @@ static void dm_integrity_postsuspend(str

 	del_timer_sync(&ic->autocommit_timer);

-	WRITE_ONCE(ic->suspending, 1);
-
 	if (ic->recalc_wq)
 		drain_workqueue(ic->recalc_wq);

@@ -2426,8 +2425,6 @@ static void dm_integrity_postsuspend(str
 		dm_integrity_flush_buffers(ic);
 	}

-	WRITE_ONCE(ic->suspending, 0);
-
 	BUG_ON(!RB_EMPTY_ROOT(&ic->in_progress));

 	ic->journal_uptodate = true;
@@ -3116,6 +3113,7 @@ static int dm_integrity_ctr(struct dm_ta
 	}
 	ti->private = ic;
 	ti->per_io_data_size = sizeof(struct dm_integrity_io);
+	ic->ti = ti;

 	ic->in_progress = RB_ROOT;
 	INIT_LIST_HEAD(&ic->wait_list);
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -844,7 +844,7 @@ static void writecache_suspend(struct dm
 	}
 	wc_unlock(wc);

-	flush_workqueue(wc->writeback_wq);
+	drain_workqueue(wc->writeback_wq);

 	wc_lock(wc);
 	if (flush_on_suspend)
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2353,6 +2353,7 @@ static void __dm_destroy(struct mapped_d
 	map = dm_get_live_table(md, &srcu_idx);
 	if (!dm_suspended_md(md)) {
 		dm_table_presuspend_targets(map);
+		set_bit(DMF_SUSPENDED, &md->flags);
 		dm_table_postsuspend_targets(map);
 	}
 	/* dm_put_live_table must be before msleep, otherwise deadlock is possible */