On Thu, Jan 07, 2021 at 11:08:39PM +0000, Satya Tangirala wrote:

quoted

 		error = sb->s_op->freeze_super(sb);
 	else

@@ -600,6 +602,7 @@ int thaw_bdev(struct block_device *bdev)
 	if (!sb)
 		goto out;
 
+	bdev->bd_fsfreeze_sb = NULL;

This causes bdev->bd_fsfreeze_sb to be set to NULL even if the call to
thaw_super right after this line fail. So if a caller tries to call
thaw_bdev() again after receiving such an error, that next call won't even
try to call thaw_super(). Is that what we want here?  (I don't know much
about this code, but from a cursory glance I think this difference is
visible to emergency_thaw_bdev() in fs/buffer.c)

Yes, that definitively is an issue.

I think the second difference (decrementing bd_fsfreeze_count when
get_active_super() returns NULL) doesn't change anything w.r.t the
use-after-free. It does however, change the behaviour of the function
slightly, and it might be caller visible (because from a cursory glance, it
looks like we're reading the bd_fsfreeze_count from some other places like
fs/super.c). Even before 040f04bd2e82, the code wouldn't decrement
bd_fsfreeze_count when get_active_super() returned NULL - so is this change
in behaviour intentional? And if so, maybe it should go in a separate
patch?

Yes, that would be a change in behavior.  And I'm not sure why we would
want to change it.  But if so we should do it in a separate patch that
documents the why, on top of the patch that already is in the block tree.