* [PATCH] md/bitmap: Fix bitmap chunk size overflow issues.
@ 2022-10-14 12:20 Jack Wang
2022-10-14 13:23 ` Paul Menzel
[not found] ` <202210171721.bveCQYrI-lkp@intel.com>
0 siblings, 2 replies; 4+ messages in thread
From: Jack Wang @ 2022-10-14 12:20 UTC (permalink / raw)
To: linux-raid; +Cc: song, Florian-Ewald Mueller, stable
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.
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>
---
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..b266711485a8 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 >= (1UL << (BITS_PER_BYTE *
+ sizeof(((bitmap_super_t *)0)->chunksize))))
+ return -EOVERFLOW;
mddev->bitmap_info.chunksize = csize;
return len;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] md/bitmap: Fix bitmap chunk size overflow issues.
2022-10-14 12:20 [PATCH] md/bitmap: Fix bitmap chunk size overflow issues Jack Wang
@ 2022-10-14 13:23 ` Paul Menzel
2022-10-14 13:39 ` Jinpu Wang
[not found] ` <202210171721.bveCQYrI-lkp@intel.com>
1 sibling, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2022-10-14 13:23 UTC (permalink / raw)
To: Jack Wang; +Cc: Florian-Ewald Mueller, song, linux-raid, stable
Dear Jack, dear Florian-Ewald,
Thank you for the patch.
Am 14.10.22 um 14:20 schrieb Jack Wang:
> 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.
Did you find this during code review or hit actual problems? If so, a
reproducer would be nice to have. (A small nit, should you resend, if
you removed the dot/period from the end of the commit message
summary/title, that’d be great.)
Kind regards,
Paul
> 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>
> ---
> 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..b266711485a8 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 >= (1UL << (BITS_PER_BYTE *
> + sizeof(((bitmap_super_t *)0)->chunksize))))
> + return -EOVERFLOW;
> mddev->bitmap_info.chunksize = csize;
> return len;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] md/bitmap: Fix bitmap chunk size overflow issues.
2022-10-14 13:23 ` Paul Menzel
@ 2022-10-14 13:39 ` Jinpu Wang
0 siblings, 0 replies; 4+ messages in thread
From: Jinpu Wang @ 2022-10-14 13:39 UTC (permalink / raw)
To: Paul Menzel; +Cc: Florian-Ewald Mueller, song, linux-raid, stable
On Fri, Oct 14, 2022 at 3:23 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Jack, dear Florian-Ewald,
>
>
> Thank you for the patch.
>
> Am 14.10.22 um 14:20 schrieb Jack Wang:
> > 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.
>
> Did you find this during code review or hit actual problems? If so, a
> reproducer would be nice to have. (A small nit, should you resend, if
> you removed the dot/period from the end of the commit message
> summary/title, that’d be great.)
Yes, we can reproduce it.
steps:
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_chunk will overflow.
Sure we can improve the commit message.
>
> Kind regards,
>
> Paul
Thanks for checking!
>
>
> > 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>
> > ---
> > 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..b266711485a8 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 >= (1UL << (BITS_PER_BYTE *
> > + sizeof(((bitmap_super_t *)0)->chunksize))))
> > + return -EOVERFLOW;
> > mddev->bitmap_info.chunksize = csize;
> > return len;
> > }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] md/bitmap: Fix bitmap chunk size overflow issues.
[not found] ` <202210171721.bveCQYrI-lkp@intel.com>
@ 2022-10-17 10:10 ` Jinpu Wang
0 siblings, 0 replies; 4+ messages in thread
From: Jinpu Wang @ 2022-10-17 10:10 UTC (permalink / raw)
To: kernel test robot
Cc: linux-raid, kbuild-all, song, Florian-Ewald Mueller, stable
On Mon, Oct 17, 2022 at 11:17 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Jack,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on song-md/md-next]
> [also build test WARNING on linus/master v6.1-rc1 next-20221017]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jack-Wang/md-bitmap-Fix-bitmap-chunk-size-overflow-issues/20221017-103245
> base: git://git.kernel.org/pub/scm/linux/kernel/git/song/md.git md-next
> patch link: https://lore.kernel.org/r/20221014122032.47784-1-jinpu.wang%40ionos.com
> patch subject: [PATCH] md/bitmap: Fix bitmap chunk size overflow issues.
> config: microblaze-randconfig-r032-20221017
> compiler: microblaze-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/68927a99dc7197ea076a3b40d876337073f70a58
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jack-Wang/md-bitmap-Fix-bitmap-chunk-size-overflow-issues/20221017-103245
> git checkout 68927a99dc7197ea076a3b40d876337073f70a58
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze SHELL=/bin/bash drivers/md/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/md/md-bitmap.c: In function 'chunksize_store':
> >> drivers/md/md-bitmap.c:2538:27: warning: left shift count >= width of type [-Wshift-count-overflow]
> 2538 | if (csize >= (1UL << (BITS_PER_BYTE *
> | ^~
>
Thanks will fix in v2.
>
> vim +2538 drivers/md/md-bitmap.c
>
> 2523
> 2524 static ssize_t
> 2525 chunksize_store(struct mddev *mddev, const char *buf, size_t len)
> 2526 {
> 2527 /* Can only be changed when no bitmap is active */
> 2528 int rv;
> 2529 unsigned long csize;
> 2530 if (mddev->bitmap)
> 2531 return -EBUSY;
> 2532 rv = kstrtoul(buf, 10, &csize);
> 2533 if (rv)
> 2534 return rv;
> 2535 if (csize < 512 ||
> 2536 !is_power_of_2(csize))
> 2537 return -EINVAL;
> > 2538 if (csize >= (1UL << (BITS_PER_BYTE *
> 2539 sizeof(((bitmap_super_t *)0)->chunksize))))
> 2540 return -EOVERFLOW;
> 2541 mddev->bitmap_info.chunksize = csize;
> 2542 return len;
> 2543 }
> 2544
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-17 10:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-14 12:20 [PATCH] md/bitmap: Fix bitmap chunk size overflow issues Jack Wang
2022-10-14 13:23 ` Paul Menzel
2022-10-14 13:39 ` Jinpu Wang
[not found] ` <202210171721.bveCQYrI-lkp@intel.com>
2022-10-17 10:10 ` Jinpu Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox