Skip to content

Commit

Permalink
md/md-bitmap: Synchronize bitmap_get_stats() with bitmap lifetime
Browse files Browse the repository at this point in the history
After commit ec6bb29 ("md/md-bitmap: add 'sync_size' into struct
md_bitmap_stats"), following panic is reported:

Oops: general protection fault, probably for non-canonical address
RIP: 0010:bitmap_get_stats+0x2b/0xa0
Call Trace:
 <TASK>
 md_seq_show+0x2d2/0x5b0
 seq_read_iter+0x2b9/0x470
 seq_read+0x12f/0x180
 proc_reg_read+0x57/0xb0
 vfs_read+0xf6/0x380
 ksys_read+0x6c/0xf0
 do_syscall_64+0x82/0x170
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Root cause is that bitmap_get_stats() can be called at anytime if mddev
is still there, even if bitmap is destroyed, or not fully initialized.
Deferenceing bitmap in this case can crash the kernel. Meanwhile, the
above commit start to deferencing bitmap->storage, make the problem
easier to trigger.

Fix the problem by protecting bitmap_get_stats() with bitmap_info.mutex.

Cc: stable@vger.kernel.org # v6.12+
Fixes: 32a7627 ("[PATCH] md: optimised resync using Bitmap based intent logging")
Reported-and-tested-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Closes: https://lore.kernel.org/linux-raid/ca3a91a2-50ae-4f68-b317-abd9889f3907@oracle.com/T/#m6e5086c95201135e4941fe38f9efa76daf9666c5
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Link: https://lore.kernel.org/r/20250124092055.4050195-1-yukuai1@huaweicloud.com
Signed-off-by: Song Liu <song@kernel.org>
  • Loading branch information
Yu Kuai authored and Song Liu committed Jan 24, 2025
1 parent a9ae6fe commit 8d28d0d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
5 changes: 4 additions & 1 deletion drivers/md/md-bitmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -2355,7 +2355,10 @@ static int bitmap_get_stats(void *data, struct md_bitmap_stats *stats)

if (!bitmap)
return -ENOENT;

if (bitmap->mddev->bitmap_info.external)
return -ENOENT;
if (!bitmap->storage.sb_page) /* no superblock */
return -EINVAL;
sb = kmap_local_page(bitmap->storage.sb_page);
stats->sync_size = le64_to_cpu(sb->sync_size);
kunmap_local(sb);
Expand Down
5 changes: 5 additions & 0 deletions drivers/md/md.c
Original file line number Diff line number Diff line change
Expand Up @@ -8376,6 +8376,10 @@ static int md_seq_show(struct seq_file *seq, void *v)
return 0;

spin_unlock(&all_mddevs_lock);

/* prevent bitmap to be freed after checking */
mutex_lock(&mddev->bitmap_info.mutex);

spin_lock(&mddev->lock);
if (mddev->pers || mddev->raid_disks || !list_empty(&mddev->disks)) {
seq_printf(seq, "%s : ", mdname(mddev));
Expand Down Expand Up @@ -8451,6 +8455,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "\n");
}
spin_unlock(&mddev->lock);
mutex_unlock(&mddev->bitmap_info.mutex);
spin_lock(&all_mddevs_lock);

if (mddev == list_last_entry(&all_mddevs, struct mddev, all_mddevs))
Expand Down

0 comments on commit 8d28d0d

Please sign in to comment.