Re: [PATCH v2 1/5] md/md-bitmap: call md_bitmap_create,destroy in location_store

[Su Yue <l@damenly.org>]

On Mon 20 Apr 2026 at 13:21, Xiao Ni <xni@redhat.com> wrote:

> On Thu, Apr 16, 2026 at 10:09 PM Su Yue <l@damenly.org> wrote:
>>
>> On Wed 15 Apr 2026 at 18:34, Xiao Ni <xni@redhat.com> wrote:
>>
>> > On Tue, Apr 7, 2026 at 6:26 PM Su Yue <glass.su@suse.com> 
>> > wrote:
>> >>
>> >> If bitmap/location is present, mdadm will call
>> >> update_array_info()
>> >> while growing bitmap from none to internal via
>> >> location_store().
>> >> md_bitmap_create() is needed to set mddev->bitmap_ops 
>> >> otherwise
>> >> mddev->bitmap_ops->get_stats() in update_array_info() will
>> >> trigger
>> >> kernel NULL pointer dereference.
>> >
>> >
>> > Hi Su Yue
>> >
>> > How can bitmap/location be present when bitmap is none? Could
>> > you
>> > provide the test commands that reproduce this problem?
>> >
>> Sorry for the misleading commit message. It can only be 
>> reproduced
>> patch 3 is appiled.
>> I adjusted the sequence of this patch for easy review because
>> md_bitmap_create,destroy
>> are touched in patch1,2 and 3. Also if put the patch after 3rd
>> patch,
>> it will break ability to bisect.
>>
>> # mdadm --create --assume-clean /dev/md0 -f --bitmap=internal
>>   --raid-devices=2 --level=mirror --metadata=1.2 /dev/vdc 
>>   /dev/vdd
>> # mdadm --grow /dev/md0 --bitmap=none
>> # mdadm --grow /dev/md0 --bitmap=internal # step 3
>> # mdadm --grow /dev/md0 --bitmap=none # step 4
>> [1]    2325 killed     mdadm --grow /dev/md0 --bitmap=none
>>
>> When step 3 is called,
>> md_bitmap_destroy() is called in update_array_info() to set 
>> NULL
>> mddev->bitmap_ops
>> then in step 4 kernel Oops is triggered.
>>
>>
>> I am willing to amend commit message or move it after patch 3 
>> if
>> you would like.
>
> Hi Su
>
> Thanks for the detail explanation. After reading patch3, I 
> totoally
> understand. The sequence is good to me. And yes, it's better to
> explain that this is needed after patch3.
>
Sure. I will do it in next version.

--
Su
>
> Best Regards
> Xiao
>>
>> --
>> Su
>>
>> >
>> > mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 
>> > --bitmap=none
>> > (There
>> > is not bitmap/location, because bitmap directory is not 
>> > created)
>> > mdadm /dev/md0 --grow --bitmap=internal
>> > Grow.c md_set_array_info runs
>> >  451             array.state |= (1 << MD_SB_BITMAP_PRESENT);
>> >  452             rv = md_set_array_info(fd, &array);
>> > In kernel space, it runs
>> >  8125             rv = md_bitmap_create(mddev);
>> >  8126             if (!rv)
>> >  8127                 rv = mddev->bitmap_ops->load(mddev);
>> >
>> > Best Regards
>> > Xiao
>> >
>> >>
>> >> Fixes: fb8cc3b0d9db ("md/md-bitmap: delay registration of
>> >> bitmap_ops until creating bitmap")
>> >> Signed-off-by: Su Yue <glass.su@suse.com>
>> >> ---
>> >>  drivers/md/md-bitmap.c | 11 ++++++++---
>> >>  drivers/md/md.c        |  4 ++--
>> >>  drivers/md/md.h        |  2 ++
>> >>  3 files changed, 12 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> >> index 83378c033c72..2f24aae05552 100644
>> >> --- a/drivers/md/md-bitmap.c
>> >> +++ b/drivers/md/md-bitmap.c
>> >> @@ -2618,7 +2618,7 @@ location_store(struct mddev *mddev, 
>> >> const
>> >> char *buf, size_t len)
>> >>                         goto out;
>> >>                 }
>> >>
>> >> -               bitmap_destroy(mddev);
>> >> +               md_bitmap_destroy(mddev);
>> >>                 mddev->bitmap_info.offset = 0;
>> >>                 if (mddev->bitmap_info.file) {
>> >>                         struct file *f =
>> >>                         mddev->bitmap_info.file;
>> >> @@ -2653,15 +2653,20 @@ location_store(struct mddev *mddev,
>> >> const char *buf, size_t len)
>> >>                                 goto out;
>> >>                         }
>> >>
>> >> +                       /*
>> >> +                        * lockless bitmap shoudle have set
>> >> bitmap_id
>> >> +                        * using bitmap_type, so always
>> >> ID_BITMAP.
>> >> +                        */
>> >> +                       mddev->bitmap_id = ID_BITMAP;
>> >>                         mddev->bitmap_info.offset = offset;
>> >> -                       rv = bitmap_create(mddev);
>> >> +                       rv = md_bitmap_create(mddev);
>> >>                         if (rv)
>> >>                                 goto out;
>> >>
>> >>                         rv = bitmap_load(mddev);
>> >>                         if (rv) {
>> >>                                 mddev->bitmap_info.offset = 
>> >>                                 0;
>> >> -                               bitmap_destroy(mddev);
>> >> +                               md_bitmap_destroy(mddev);
>> >>                                 goto out;
>> >>                         }
>> >>                 }
>> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> >> index 3ce6f9e9d38e..8b1ecc370ad6 100644
>> >> --- a/drivers/md/md.c
>> >> +++ b/drivers/md/md.c
>> >> @@ -6447,7 +6447,7 @@ static void md_safemode_timeout(struct
>> >> timer_list *t)
>> >>
>> >>  static int start_dirty_degraded;
>> >>
>> >> -static int md_bitmap_create(struct mddev *mddev)
>> >> +int md_bitmap_create(struct mddev *mddev)
>> >>  {
>> >>         if (mddev->bitmap_id == ID_BITMAP_NONE)
>> >>                 return -EINVAL;
>> >> @@ -6458,7 +6458,7 @@ static int md_bitmap_create(struct 
>> >> mddev
>> >> *mddev)
>> >>         return mddev->bitmap_ops->create(mddev);
>> >>  }
>> >>
>> >> -static void md_bitmap_destroy(struct mddev *mddev)
>> >> +void md_bitmap_destroy(struct mddev *mddev)
>> >>  {
>> >>         if (!md_bitmap_registered(mddev))
>> >>                 return;
>> >> diff --git a/drivers/md/md.h b/drivers/md/md.h
>> >> index ac84289664cd..ed69244af00d 100644
>> >> --- a/drivers/md/md.h
>> >> +++ b/drivers/md/md.h
>> >> @@ -895,6 +895,8 @@ static inline void safe_put_page(struct
>> >> page *p)
>> >>
>> >>  int register_md_submodule(struct md_submodule_head *msh);
>> >>  void unregister_md_submodule(struct md_submodule_head 
>> >>  *msh);
>> >> +int md_bitmap_create(struct mddev *mddev);
>> >> +void md_bitmap_destroy(struct mddev *mddev);
>> >>
>> >>  extern struct md_thread *md_register_thread(
>> >>         void (*run)(struct md_thread *thread),
>> >> --
>> >> 2.53.0
>> >>
>>