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

[Su Yue <l@damenly.org>]

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.

--
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
>>