Thread (12 messages) 12 messages, 5 authors, 2021-01-08

Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb

From: Christoph Hellwig <hch@lst.de>
Date: 2021-01-07 16:21:05
Also in: lkml

Can someone pick this up?  Maybe through Jens' block tree as that is
where my commit this is fixing up came from.

For reference:


Reviewed-by: Christoph Hellwig <hch@lst.de>

On Thu, Dec 24, 2020 at 04:49:54AM +0000, Satya Tangirala wrote:
quoted hunk ↗ jump to hunk
freeze/thaw_bdev() currently use bdev->bd_fsfreeze_count to infer
whether or not bdev->bd_fsfreeze_sb is valid (it's valid iff
bd_fsfreeze_count is non-zero). thaw_bdev() doesn't nullify
bd_fsfreeze_sb.

But this means a freeze_bdev() call followed by a thaw_bdev() call can
leave bd_fsfreeze_sb with a non-null value, while bd_fsfreeze_count is
zero. If freeze_bdev() is called again, and this time
get_active_super() returns NULL (e.g. because the FS is unmounted),
we'll end up with bd_fsfreeze_count > 0, but bd_fsfreeze_sb is
*untouched* - it stays the same (now garbage) value. A subsequent
thaw_bdev() will decide that the bd_fsfreeze_sb value is legitimate
(since bd_fsfreeze_count > 0), and attempt to use it.

Fix this by always setting bd_fsfreeze_sb to NULL when
bd_fsfreeze_count is successfully decremented to 0 in thaw_sb().
Alternatively, we could set bd_fsfreeze_sb to whatever
get_active_super() returns in freeze_bdev() whenever bd_fsfreeze_count
is successfully incremented to 1 from 0 (which can be achieved cleanly
by moving the line currently setting bd_fsfreeze_sb to immediately
after the "sync:" label, but it might be a little too subtle/easily
overlooked in future).

This fixes the currently panicking xfstests generic/085.

Fixes: 040f04bd2e82 ("fs: simplify freeze_bdev/thaw_bdev")
Signed-off-by: Satya Tangirala <redacted>
---
 fs/block_dev.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9e56ee1f2652..12a811a9ae4b 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -606,6 +606,8 @@ int thaw_bdev(struct block_device *bdev)
 		error = thaw_super(sb);
 	if (error)
 		bdev->bd_fsfreeze_count++;
+	else
+		bdev->bd_fsfreeze_sb = NULL;
 out:
 	mutex_unlock(&bdev->bd_fsfreeze_mutex);
 	return error;
-- 
2.29.2.729.g45daf8777d-goog
---end quoted text---
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help