diff mbox series

[PATCHv2] md/bitmap: Fix bitmap chunk size overflow issues

Message ID 20221017100951.22727-1-jinpu.wang@ionos.com (mailing list archive)
State Accepted, archived
Headers show
Series [PATCHv2] md/bitmap: Fix bitmap chunk size overflow issues | expand

Commit Message

Jinpu Wang Oct. 17, 2022, 10:09 a.m. UTC
From: Florian-Ewald Mueller <florian-ewald.mueller@ionos.com>

- limit bitmap chunk size internal u64 variable to values not overflowing
  the u32 bitmap superblock structure variable stored on persistent media
- assign bitmap chunk size internal u64 variable from unsigned values to
  avoid possible sign extension artifacts when assigning from a s32 value

The bug has been there since at least kernel 4.0.
Steps to reproduce it:
1: mdadm -C /dev/mdx -l 1 --bitmap=internal --bitmap-chunk=256M -e 1.2
-n2 /dev/rnbd1 /dev/rnbd2
2 resize member device rnbd1 and rnbd2 to 8 TB
3 mdadm --grow /dev/mdx --size=max

The bitmap_chunksize will overflow without patch.

Cc: stable@vger.kernel.org

Signed-off-by: Florian-Ewald Mueller <florian-ewald.mueller@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
v1->v2: extend commit message for steps to reproduce the problem.
        fix the warning reported by buildbot

 drivers/md/md-bitmap.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Song Liu Oct. 20, 2022, 7:16 p.m. UTC | #1
On Mon, Oct 17, 2022 at 3:09 AM Jack Wang <jinpu.wang@ionos.com> wrote:
>
> From: Florian-Ewald Mueller <florian-ewald.mueller@ionos.com>
>
> - limit bitmap chunk size internal u64 variable to values not overflowing
>   the u32 bitmap superblock structure variable stored on persistent media
> - assign bitmap chunk size internal u64 variable from unsigned values to
>   avoid possible sign extension artifacts when assigning from a s32 value
>
> The bug has been there since at least kernel 4.0.
> Steps to reproduce it:
> 1: mdadm -C /dev/mdx -l 1 --bitmap=internal --bitmap-chunk=256M -e 1.2
> -n2 /dev/rnbd1 /dev/rnbd2
> 2 resize member device rnbd1 and rnbd2 to 8 TB
> 3 mdadm --grow /dev/mdx --size=max
>
> The bitmap_chunksize will overflow without patch.
>
> Cc: stable@vger.kernel.org
>
> Signed-off-by: Florian-Ewald Mueller <florian-ewald.mueller@ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>

Applied to md-next. Thanks!

Song


> ---
> v1->v2: extend commit message for steps to reproduce the problem.
>         fix the warning reported by buildbot
>
>  drivers/md/md-bitmap.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index bf6dffadbe6f..d4bf7f603ef6 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -486,7 +486,7 @@ void md_bitmap_print_sb(struct bitmap *bitmap)
>         sb = kmap_atomic(bitmap->storage.sb_page);
>         pr_debug("%s: bitmap file superblock:\n", bmname(bitmap));
>         pr_debug("         magic: %08x\n", le32_to_cpu(sb->magic));
> -       pr_debug("       version: %d\n", le32_to_cpu(sb->version));
> +       pr_debug("       version: %u\n", le32_to_cpu(sb->version));
>         pr_debug("          uuid: %08x.%08x.%08x.%08x\n",
>                  le32_to_cpu(*(__le32 *)(sb->uuid+0)),
>                  le32_to_cpu(*(__le32 *)(sb->uuid+4)),
> @@ -497,11 +497,11 @@ void md_bitmap_print_sb(struct bitmap *bitmap)
>         pr_debug("events cleared: %llu\n",
>                  (unsigned long long) le64_to_cpu(sb->events_cleared));
>         pr_debug("         state: %08x\n", le32_to_cpu(sb->state));
> -       pr_debug("     chunksize: %d B\n", le32_to_cpu(sb->chunksize));
> -       pr_debug("  daemon sleep: %ds\n", le32_to_cpu(sb->daemon_sleep));
> +       pr_debug("     chunksize: %u B\n", le32_to_cpu(sb->chunksize));
> +       pr_debug("  daemon sleep: %us\n", le32_to_cpu(sb->daemon_sleep));
>         pr_debug("     sync size: %llu KB\n",
>                  (unsigned long long)le64_to_cpu(sb->sync_size)/2);
> -       pr_debug("max write behind: %d\n", le32_to_cpu(sb->write_behind));
> +       pr_debug("max write behind: %u\n", le32_to_cpu(sb->write_behind));
>         kunmap_atomic(sb);
>  }
>
> @@ -2105,7 +2105,8 @@ int md_bitmap_resize(struct bitmap *bitmap, sector_t blocks,
>                         bytes = DIV_ROUND_UP(chunks, 8);
>                         if (!bitmap->mddev->bitmap_info.external)
>                                 bytes += sizeof(bitmap_super_t);
> -               } while (bytes > (space << 9));
> +               } while (bytes > (space << 9) && (chunkshift + BITMAP_BLOCK_SHIFT) <
> +                       (BITS_PER_BYTE * sizeof(((bitmap_super_t *)0)->chunksize) - 1));
>         } else
>                 chunkshift = ffz(~chunksize) - BITMAP_BLOCK_SHIFT;
>
> @@ -2150,7 +2151,7 @@ int md_bitmap_resize(struct bitmap *bitmap, sector_t blocks,
>         bitmap->counts.missing_pages = pages;
>         bitmap->counts.chunkshift = chunkshift;
>         bitmap->counts.chunks = chunks;
> -       bitmap->mddev->bitmap_info.chunksize = 1 << (chunkshift +
> +       bitmap->mddev->bitmap_info.chunksize = 1UL << (chunkshift +
>                                                      BITMAP_BLOCK_SHIFT);
>
>         blocks = min(old_counts.chunks << old_counts.chunkshift,
> @@ -2176,8 +2177,8 @@ int md_bitmap_resize(struct bitmap *bitmap, sector_t blocks,
>                                 bitmap->counts.missing_pages = old_counts.pages;
>                                 bitmap->counts.chunkshift = old_counts.chunkshift;
>                                 bitmap->counts.chunks = old_counts.chunks;
> -                               bitmap->mddev->bitmap_info.chunksize = 1 << (old_counts.chunkshift +
> -                                                                            BITMAP_BLOCK_SHIFT);
> +                               bitmap->mddev->bitmap_info.chunksize =
> +                                       1UL << (old_counts.chunkshift + BITMAP_BLOCK_SHIFT);
>                                 blocks = old_counts.chunks << old_counts.chunkshift;
>                                 pr_warn("Could not pre-allocate in-memory bitmap for cluster raid\n");
>                                 break;
> @@ -2534,6 +2535,9 @@ chunksize_store(struct mddev *mddev, const char *buf, size_t len)
>         if (csize < 512 ||
>             !is_power_of_2(csize))
>                 return -EINVAL;
> +       if (csize >= (1ULL << (BITS_PER_BYTE *
> +               sizeof(((bitmap_super_t *)0)->chunksize))))
> +               return -EOVERFLOW;
>         mddev->bitmap_info.chunksize = csize;
>         return len;
>  }
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index bf6dffadbe6f..d4bf7f603ef6 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -486,7 +486,7 @@  void md_bitmap_print_sb(struct bitmap *bitmap)
 	sb = kmap_atomic(bitmap->storage.sb_page);
 	pr_debug("%s: bitmap file superblock:\n", bmname(bitmap));
 	pr_debug("         magic: %08x\n", le32_to_cpu(sb->magic));
-	pr_debug("       version: %d\n", le32_to_cpu(sb->version));
+	pr_debug("       version: %u\n", le32_to_cpu(sb->version));
 	pr_debug("          uuid: %08x.%08x.%08x.%08x\n",
 		 le32_to_cpu(*(__le32 *)(sb->uuid+0)),
 		 le32_to_cpu(*(__le32 *)(sb->uuid+4)),
@@ -497,11 +497,11 @@  void md_bitmap_print_sb(struct bitmap *bitmap)
 	pr_debug("events cleared: %llu\n",
 		 (unsigned long long) le64_to_cpu(sb->events_cleared));
 	pr_debug("         state: %08x\n", le32_to_cpu(sb->state));
-	pr_debug("     chunksize: %d B\n", le32_to_cpu(sb->chunksize));
-	pr_debug("  daemon sleep: %ds\n", le32_to_cpu(sb->daemon_sleep));
+	pr_debug("     chunksize: %u B\n", le32_to_cpu(sb->chunksize));
+	pr_debug("  daemon sleep: %us\n", le32_to_cpu(sb->daemon_sleep));
 	pr_debug("     sync size: %llu KB\n",
 		 (unsigned long long)le64_to_cpu(sb->sync_size)/2);
-	pr_debug("max write behind: %d\n", le32_to_cpu(sb->write_behind));
+	pr_debug("max write behind: %u\n", le32_to_cpu(sb->write_behind));
 	kunmap_atomic(sb);
 }
 
@@ -2105,7 +2105,8 @@  int md_bitmap_resize(struct bitmap *bitmap, sector_t blocks,
 			bytes = DIV_ROUND_UP(chunks, 8);
 			if (!bitmap->mddev->bitmap_info.external)
 				bytes += sizeof(bitmap_super_t);
-		} while (bytes > (space << 9));
+		} while (bytes > (space << 9) && (chunkshift + BITMAP_BLOCK_SHIFT) <
+			(BITS_PER_BYTE * sizeof(((bitmap_super_t *)0)->chunksize) - 1));
 	} else
 		chunkshift = ffz(~chunksize) - BITMAP_BLOCK_SHIFT;
 
@@ -2150,7 +2151,7 @@  int md_bitmap_resize(struct bitmap *bitmap, sector_t blocks,
 	bitmap->counts.missing_pages = pages;
 	bitmap->counts.chunkshift = chunkshift;
 	bitmap->counts.chunks = chunks;
-	bitmap->mddev->bitmap_info.chunksize = 1 << (chunkshift +
+	bitmap->mddev->bitmap_info.chunksize = 1UL << (chunkshift +
 						     BITMAP_BLOCK_SHIFT);
 
 	blocks = min(old_counts.chunks << old_counts.chunkshift,
@@ -2176,8 +2177,8 @@  int md_bitmap_resize(struct bitmap *bitmap, sector_t blocks,
 				bitmap->counts.missing_pages = old_counts.pages;
 				bitmap->counts.chunkshift = old_counts.chunkshift;
 				bitmap->counts.chunks = old_counts.chunks;
-				bitmap->mddev->bitmap_info.chunksize = 1 << (old_counts.chunkshift +
-									     BITMAP_BLOCK_SHIFT);
+				bitmap->mddev->bitmap_info.chunksize =
+					1UL << (old_counts.chunkshift + BITMAP_BLOCK_SHIFT);
 				blocks = old_counts.chunks << old_counts.chunkshift;
 				pr_warn("Could not pre-allocate in-memory bitmap for cluster raid\n");
 				break;
@@ -2534,6 +2535,9 @@  chunksize_store(struct mddev *mddev, const char *buf, size_t len)
 	if (csize < 512 ||
 	    !is_power_of_2(csize))
 		return -EINVAL;
+	if (csize >= (1ULL << (BITS_PER_BYTE *
+		sizeof(((bitmap_super_t *)0)->chunksize))))
+		return -EOVERFLOW;
 	mddev->bitmap_info.chunksize = csize;
 	return len;
 }