[PATCH] buffer: warn when a buffer is attached to a second mmb

[Aurelien DESBRIERES]

mmb_mark_buffer_dirty() tracks a buffer on one mapping_metadata_bhs at
most. The per-mmb lock does not protect a buffer that changes lists,
and mmb_sync() clears b_mmb for a moment under its own lock, so a
filesystem that attaches a buffer shared by several inodes to each
inode's list in turn corrupts b_assoc_buffers eventually. With
CONFIG_DEBUG_LIST the list_del is refused and mmb_sync() spins on a
list that never empties, holding mmb->lock: seen with beamfs under
dbench (xfstests generic/241) as list_del corruption, a soft lockup and
a node lost to the OOM killer, with nothing naming the filesystem.

Warn once when a buffer already tracked by another mmb is marked dirty
through a second one, so that the misuse is named at its first
occurrence rather than at the corruption it leads to. A corrupted
volume in which two inodes claim the same block reaches this as well;
the cmpxchg below keeps the lists sound in that case, and the warning
then says which filesystem believed its block map.

Suggested-by: Jan Kara <jack@xxxxxxx>
Link: https://lore.kernel.org/linux-fsdevel/q472nkbfcfdumbxm3cebfnnmueqyl6uhhnper5t6e2cxajpvtu@7prjjo7k6ia3/
Assisted-by: LLM
Signed-off-by: Aurelien DESBRIERES <aurelien@xxxxxxxxxxxx>
---
Tested on vfs.all with beamfs, x86_64 guest, 4 CPUs:

- the module as it was before our fix, attaching the shared bitmap
  block to each allocating inode's list in turn: the warning fires at
  boot, 8.6 s in, on the first truncate that frees blocks of a file
  another inode had allocated, long before the dbench run that used
  to corrupt the list:

    WARNING: fs/buffer.c:639 at mmb_mark_buffer_dirty+0x46/0xd0, CPU#3: S01hostname-cmd/111
     beamfs_write_bitmap_block+0x191/0x1c0
     beamfs_inline_free_blocks_from+0x64/0x390
     beamfs_inline_setattr+0x127/0x4c0
     notify_change+0x33b/0x520
     do_truncate+0xbf/0xf0
     path_openat+0x8c1/0xaa0

- the module with the shared buffers on one superblock list: no
  warning at boot or under the filesystem's own tests.

 fs/buffer.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/buffer.c b/fs/buffer.c
index 1dc933ba6925..e503a7ed9c37 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -627,6 +627,16 @@ void write_boundary_block(struct block_device *bdev,
 void mmb_mark_buffer_dirty(struct buffer_head *bh,
 			   struct mapping_metadata_bhs *mmb)
 {
+	struct mapping_metadata_bhs *cur = READ_ONCE(bh->b_mmb);
+
+	/*
+	 * A buffer is tracked by one mmb at most. The per-mmb lock does
+	 * not cover a buffer that changes lists, and mmb_sync() clears
+	 * b_mmb for a moment under its own lock, so a filesystem that
+	 * attaches a buffer to a second mmb corrupts b_assoc_buffers
+	 * sooner or later. Name it here, before the lists do.
+	 */
+	WARN_ON_ONCE(cur && cur != mmb);
 	mark_buffer_dirty(bh);
 	if (!bh->b_mmb) {
 		spin_lock(&mmb->lock);

base-commit: 2c3ba1f1cb416b81d617b92a24a4bcaa48d574e3
-- 
2.55.0