[PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Zhihao Cheng]

Concurrent processes md_stop and IO submitting could trigger a
null-ptr-deref of 'mddev->private':

 BUG: kernel NULL pointer dereference, address: 0000000000000070
 RIP: 0010:_wait_barrier+0x2f/0x250
 Call Trace:
  raid1_make_request+0x150/0xf50
  md_handle_request+0x104/0x530
  md_submit_bio+0x76/0x130
  submit_bio+0xdd/0x250
  submit_bio_wait+0x1f/0x40
  __blkdev_direct_IO_simple+0x1f6/0x370
  blkdev_write_iter+0x3b2/0x520
  ksys_write+0x7d/0x190

              P1
  fd = open(/dev/md0, O_RDWR)
	                      P2 (forked from P1, fd' <= fd)
  write(fd)
   submit_bio
    md_handle_request
     raid1_make_request
      raid1_write_request
                                  ioctl(fd, STOP_ARRAY)
		                   mddev_set_closing_and_sync_blockdev
				   // check passed, mddev->openers = 1,
				   // because md_open() is only called
				   // once in P1->open
				   do_md_stop
				    __md_stop
				     mddev->private = NULL

       conf = mddev->private // NULL
        wait_barrier(conf, sector) // null-ptr-deref !

It is a common problem for raid0/1/10/5, and __md_stop could be triggered
by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
The caller dm_table_destroy() is guaranteed being invoked with device
suspended, so raid_dtr() could keep using mddev_lock_nointr().
The caller array_state_store() is guaranteed by the check
mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
/dev/mdx, write something and close /dev/mdx, it won't trigger the
problem, all dirty pages can be flushed before mddev->openers decrement.
Besides, fail the submitting IO in md_handle_request() if the
'mddev->pers' becomes NULL.

Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 v1->v2:
  1. Add 'mddev->pers != NULL' check before make_request
  2. Delete dm-raid caller(->dtr) modifications
  3. Move memalloc_noio_restore after mddev_unlock_and_resume
 v2->v3:
  1. Remove modifications in array_state_store()
  2. update commit msg
 drivers/md/md.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..e73338c26d53 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
 		if (!percpu_ref_tryget_live(&mddev->active_io))
 			goto check_suspended;
 	}
+	if (!mddev->pers) {
+		/*
+		 * The __md_stop() sets 'mddev->private' to NULL during
+		 * the IO submitting, check 'mddev->pers' before the IO
+		 * being processed by specific driver to avoid the
+		 * null-ptr-deref of 'mddev-><member>'. The check is
+		 * safe because the IO has got the 'mddev->active_io'
+		 * reference, and all __md_stop() callers will wait for
+		 * the reference to be zero.
+		 */
+		bio_io_error(bio);
+		percpu_ref_put(&mddev->active_io);
+		return true;
+	}
 	if (!mddev->pers->make_request(mddev, bio)) {
 		percpu_ref_put(&mddev->active_io);
 		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
@@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
 	case HOT_REMOVE_DISK:
 	case SET_BITMAP_FILE:
 	case SET_ARRAY_INFO:
+	case STOP_ARRAY:
 		return true;
 	default:
 		return false;
-- 
2.52.0

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Abd-Alrhman Masalkhi]

Hi Zhihao,

On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
> Concurrent processes md_stop and IO submitting could trigger a
> null-ptr-deref of 'mddev->private':
>
>  BUG: kernel NULL pointer dereference, address: 0000000000000070
>  RIP: 0010:_wait_barrier+0x2f/0x250
>  Call Trace:
>   raid1_make_request+0x150/0xf50
>   md_handle_request+0x104/0x530
>   md_submit_bio+0x76/0x130
>   submit_bio+0xdd/0x250
>   submit_bio_wait+0x1f/0x40
>   __blkdev_direct_IO_simple+0x1f6/0x370
>   blkdev_write_iter+0x3b2/0x520
>   ksys_write+0x7d/0x190
>
>               P1
>   fd = open(/dev/md0, O_RDWR)
> 	                      P2 (forked from P1, fd' <= fd)
>   write(fd)
>    submit_bio
>     md_handle_request
>      raid1_make_request
>       raid1_write_request
>                                   ioctl(fd, STOP_ARRAY)
> 		                   mddev_set_closing_and_sync_blockdev
> 				   // check passed, mddev->openers = 1,
> 				   // because md_open() is only called
> 				   // once in P1->open
> 				   do_md_stop
> 				    __md_stop
> 				     mddev->private = NULL
>
>        conf = mddev->private // NULL
>         wait_barrier(conf, sector) // null-ptr-deref !
>
> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
> The caller dm_table_destroy() is guaranteed being invoked with device
> suspended, so raid_dtr() could keep using mddev_lock_nointr().
> The caller array_state_store() is guaranteed by the check
> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
> /dev/mdx, write something and close /dev/mdx, it won't trigger the
> problem, all dirty pages can be flushed before mddev->openers decrement.
> Besides, fail the submitting IO in md_handle_request() if the
> 'mddev->pers' becomes NULL.
>
> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>  v1->v2:
>   1. Add 'mddev->pers != NULL' check before make_request
>   2. Delete dm-raid caller(->dtr) modifications
>   3. Move memalloc_noio_restore after mddev_unlock_and_resume
>  v2->v3:
>   1. Remove modifications in array_state_store()
>   2. update commit msg
>  drivers/md/md.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3..e73338c26d53 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>  		if (!percpu_ref_tryget_live(&mddev->active_io))
>  			goto check_suspended;
>  	}
> +	if (!mddev->pers) {
Isn't this case already handled by md_submit_bio(). take a look in
md_submit_bio()

> +		/*
> +		 * The __md_stop() sets 'mddev->private' to NULL during
> +		 * the IO submitting, check 'mddev->pers' before the IO
> +		 * being processed by specific driver to avoid the
> +		 * null-ptr-deref of 'mddev-><member>'. The check is
> +		 * safe because the IO has got the 'mddev->active_io'
> +		 * reference, and all __md_stop() callers will wait for
> +		 * the reference to be zero.
> +		 */
> +		bio_io_error(bio);
> +		percpu_ref_put(&mddev->active_io);
> +		return true;
> +	}
>  	if (!mddev->pers->make_request(mddev, bio)) {
>  		percpu_ref_put(&mddev->active_io);
>  		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>  	case HOT_REMOVE_DISK:
>  	case SET_BITMAP_FILE:
>  	case SET_ARRAY_INFO:
> +	case STOP_ARRAY:
>  		return true;
>  	default:
>  		return false;
> -- 
> 2.52.0
>

-- 
Best Regards,
Abd-Alrhman

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Zhihao Cheng]

在 2026/9/18 15:29, Abd-Alrhman Masalkhi 写道:
> 
> Hi Zhihao,
> 
> On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
>> Concurrent processes md_stop and IO submitting could trigger a
>> null-ptr-deref of 'mddev->private':
>>
>>   BUG: kernel NULL pointer dereference, address: 0000000000000070
>>   RIP: 0010:_wait_barrier+0x2f/0x250
>>   Call Trace:
>>    raid1_make_request+0x150/0xf50
>>    md_handle_request+0x104/0x530
>>    md_submit_bio+0x76/0x130
>>    submit_bio+0xdd/0x250
>>    submit_bio_wait+0x1f/0x40
>>    __blkdev_direct_IO_simple+0x1f6/0x370
>>    blkdev_write_iter+0x3b2/0x520
>>    ksys_write+0x7d/0x190
>>
>>                P1
>>    fd = open(/dev/md0, O_RDWR)
>> 	                      P2 (forked from P1, fd' <= fd)
>>    write(fd)
>>     submit_bio
>>      md_handle_request
>>       raid1_make_request
>>        raid1_write_request
>>                                    ioctl(fd, STOP_ARRAY)
>> 		                   mddev_set_closing_and_sync_blockdev
>> 				   // check passed, mddev->openers = 1,
>> 				   // because md_open() is only called
>> 				   // once in P1->open
>> 				   do_md_stop
>> 				    __md_stop
>> 				     mddev->private = NULL
>>
>>         conf = mddev->private // NULL
>>          wait_barrier(conf, sector) // null-ptr-deref !
>>
>> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
>> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
>> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
>> The caller dm_table_destroy() is guaranteed being invoked with device
>> suspended, so raid_dtr() could keep using mddev_lock_nointr().
>> The caller array_state_store() is guaranteed by the check
>> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
>> /dev/mdx, write something and close /dev/mdx, it won't trigger the
>> problem, all dirty pages can be flushed before mddev->openers decrement.
>> Besides, fail the submitting IO in md_handle_request() if the
>> 'mddev->pers' becomes NULL.
>>
>> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>> ---
>>   v1->v2:
>>    1. Add 'mddev->pers != NULL' check before make_request
>>    2. Delete dm-raid caller(->dtr) modifications
>>    3. Move memalloc_noio_restore after mddev_unlock_and_resume
>>   v2->v3:
>>    1. Remove modifications in array_state_store()
>>    2. update commit msg
>>   drivers/md/md.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 680b34a63cb3..e73338c26d53 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>>   		if (!percpu_ref_tryget_live(&mddev->active_io))
>>   			goto check_suspended;
>>   	}
>> +	if (!mddev->pers) {
> Isn't this case already handled by md_submit_bio(). take a look in
> md_submit_bio()
Hi,Abd-Alrhman
I understand that 'mddev == NULL || mddev->pers == NULL' in 
md_submit_bio() is a qiuck check, there still exists a small race window:
      P1                     P2
md_submit_bio
  if (mddev == NULL || mddev->pers == NULL)
                         md_ioctl
                          mddev_suspend_and_lock
                          do_md_stop->__md_stop
                          // set mddev->pers/private as NULL
                          mddev_unlock_and_resume
  md_handle_request
   if (is_suspended(mddev, bio))
   percpu_ref_tryget_live(&mddev->active_io)
   mddev->pers // null-ptr-deref
> 
>> +		/*
>> +		 * The __md_stop() sets 'mddev->private' to NULL during
>> +		 * the IO submitting, check 'mddev->pers' before the IO
>> +		 * being processed by specific driver to avoid the
>> +		 * null-ptr-deref of 'mddev-><member>'. The check is
>> +		 * safe because the IO has got the 'mddev->active_io'
>> +		 * reference, and all __md_stop() callers will wait for
>> +		 * the reference to be zero.
>> +		 */
>> +		bio_io_error(bio);
>> +		percpu_ref_put(&mddev->active_io);
>> +		return true;
>> +	}
>>   	if (!mddev->pers->make_request(mddev, bio)) {
>>   		percpu_ref_put(&mddev->active_io);
>>   		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
>> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>>   	case HOT_REMOVE_DISK:
>>   	case SET_BITMAP_FILE:
>>   	case SET_ARRAY_INFO:
>> +	case STOP_ARRAY:
>>   		return true;
>>   	default:
>>   		return false;
>> -- 
>> 2.52.0
>>
> 

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Abd-Alrhman Masalkhi]

On Fri, Sep 18, 2026 at 15:38 +0800, Zhihao Cheng wrote:
> 在 2026/9/18 15:29, Abd-Alrhman Masalkhi 写道:
>> 
>> Hi Zhihao,
>> 
>> On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
>>> Concurrent processes md_stop and IO submitting could trigger a
>>> null-ptr-deref of 'mddev->private':
>>>
>>>   BUG: kernel NULL pointer dereference, address: 0000000000000070
>>>   RIP: 0010:_wait_barrier+0x2f/0x250
>>>   Call Trace:
>>>    raid1_make_request+0x150/0xf50
>>>    md_handle_request+0x104/0x530
>>>    md_submit_bio+0x76/0x130
>>>    submit_bio+0xdd/0x250
>>>    submit_bio_wait+0x1f/0x40
>>>    __blkdev_direct_IO_simple+0x1f6/0x370
>>>    blkdev_write_iter+0x3b2/0x520
>>>    ksys_write+0x7d/0x190
>>>
>>>                P1
>>>    fd = open(/dev/md0, O_RDWR)
>>> 	                      P2 (forked from P1, fd' <= fd)
>>>    write(fd)
>>>     submit_bio
>>>      md_handle_request
>>>       raid1_make_request
>>>        raid1_write_request
>>>                                    ioctl(fd, STOP_ARRAY)
>>> 		                   mddev_set_closing_and_sync_blockdev
>>> 				   // check passed, mddev->openers = 1,
>>> 				   // because md_open() is only called
>>> 				   // once in P1->open
>>> 				   do_md_stop
>>> 				    __md_stop
>>> 				     mddev->private = NULL
>>>
>>>         conf = mddev->private // NULL
>>>          wait_barrier(conf, sector) // null-ptr-deref !
>>>
>>> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
>>> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
>>> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
>>> The caller dm_table_destroy() is guaranteed being invoked with device
>>> suspended, so raid_dtr() could keep using mddev_lock_nointr().
>>> The caller array_state_store() is guaranteed by the check
>>> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
>>> /dev/mdx, write something and close /dev/mdx, it won't trigger the
>>> problem, all dirty pages can be flushed before mddev->openers decrement.
>>> Besides, fail the submitting IO in md_handle_request() if the
>>> 'mddev->pers' becomes NULL.
>>>
>>> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
>>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>>> ---
>>>   v1->v2:
>>>    1. Add 'mddev->pers != NULL' check before make_request
>>>    2. Delete dm-raid caller(->dtr) modifications
>>>    3. Move memalloc_noio_restore after mddev_unlock_and_resume
>>>   v2->v3:
>>>    1. Remove modifications in array_state_store()
>>>    2. update commit msg
>>>   drivers/md/md.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>> index 680b34a63cb3..e73338c26d53 100644
>>> --- a/drivers/md/md.c
>>> +++ b/drivers/md/md.c
>>> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>>>   		if (!percpu_ref_tryget_live(&mddev->active_io))
>>>   			goto check_suspended;
>>>   	}
>>> +	if (!mddev->pers) {
>> Isn't this case already handled by md_submit_bio(). take a look in
>> md_submit_bio()
> Hi,Abd-Alrhman
> I understand that 'mddev == NULL || mddev->pers == NULL' in 
> md_submit_bio() is a qiuck check, there still exists a small race window:
>       P1                     P2
> md_submit_bio
>   if (mddev == NULL || mddev->pers == NULL)
>                          md_ioctl
>                           mddev_suspend_and_lock
>                           do_md_stop->__md_stop
>                           // set mddev->pers/private as NULL
>                           mddev_unlock_and_resume
>   md_handle_request
>    if (is_suspended(mddev, bio))
>    percpu_ref_tryget_live(&mddev->active_io)
>    mddev->pers // null-ptr-deref

It makes sense. It would solve only the null pointer reference, but I
see another issue. Look at the comment before calling
mddev_set_closing_and_sync_blockdev() in md_ioctl(). it says "Need to
flush page cache, and ensure no-one else opens and writes". The
suspending happens after flushing the page cache, in this case,
there might be other writes in-flight.

>> 
>>> +		/*
>>> +		 * The __md_stop() sets 'mddev->private' to NULL during
>>> +		 * the IO submitting, check 'mddev->pers' before the IO
>>> +		 * being processed by specific driver to avoid the
>>> +		 * null-ptr-deref of 'mddev-><member>'. The check is
>>> +		 * safe because the IO has got the 'mddev->active_io'
>>> +		 * reference, and all __md_stop() callers will wait for
>>> +		 * the reference to be zero.
>>> +		 */
>>> +		bio_io_error(bio);
>>> +		percpu_ref_put(&mddev->active_io);
>>> +		return true;
>>> +	}
>>>   	if (!mddev->pers->make_request(mddev, bio)) {
>>>   		percpu_ref_put(&mddev->active_io);
>>>   		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
>>> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>>>   	case HOT_REMOVE_DISK:
>>>   	case SET_BITMAP_FILE:
>>>   	case SET_ARRAY_INFO:
>>> +	case STOP_ARRAY:
>>>   		return true;
>>>   	default:
>>>   		return false;
>>> -- 
>>> 2.52.0
>>>
>> 
>

-- 
Best Regards,
Abd-Alrhman

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Zhihao Cheng]

在 2026/9/18 16:20, Abd-Alrhman Masalkhi 写道:
> On Fri, Sep 18, 2026 at 15:38 +0800, Zhihao Cheng wrote:
>> 在 2026/9/18 15:29, Abd-Alrhman Masalkhi 写道:
>>>
>>> Hi Zhihao,
>>>
>>> On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
>>>> Concurrent processes md_stop and IO submitting could trigger a
>>>> null-ptr-deref of 'mddev->private':
>>>>
>>>>    BUG: kernel NULL pointer dereference, address: 0000000000000070
>>>>    RIP: 0010:_wait_barrier+0x2f/0x250
>>>>    Call Trace:
>>>>     raid1_make_request+0x150/0xf50
>>>>     md_handle_request+0x104/0x530
>>>>     md_submit_bio+0x76/0x130
>>>>     submit_bio+0xdd/0x250
>>>>     submit_bio_wait+0x1f/0x40
>>>>     __blkdev_direct_IO_simple+0x1f6/0x370
>>>>     blkdev_write_iter+0x3b2/0x520
>>>>     ksys_write+0x7d/0x190
>>>>
>>>>                 P1
>>>>     fd = open(/dev/md0, O_RDWR)
>>>> 	                      P2 (forked from P1, fd' <= fd)
>>>>     write(fd)
>>>>      submit_bio
>>>>       md_handle_request
>>>>        raid1_make_request
>>>>         raid1_write_request
>>>>                                     ioctl(fd, STOP_ARRAY)
>>>> 		                   mddev_set_closing_and_sync_blockdev
>>>> 				   // check passed, mddev->openers = 1,
>>>> 				   // because md_open() is only called
>>>> 				   // once in P1->open
>>>> 				   do_md_stop
>>>> 				    __md_stop
>>>> 				     mddev->private = NULL
>>>>
>>>>          conf = mddev->private // NULL
>>>>           wait_barrier(conf, sector) // null-ptr-deref !
>>>>
>>>> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
>>>> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
>>>> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
>>>> The caller dm_table_destroy() is guaranteed being invoked with device
>>>> suspended, so raid_dtr() could keep using mddev_lock_nointr().
>>>> The caller array_state_store() is guaranteed by the check
>>>> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
>>>> /dev/mdx, write something and close /dev/mdx, it won't trigger the
>>>> problem, all dirty pages can be flushed before mddev->openers decrement.
>>>> Besides, fail the submitting IO in md_handle_request() if the
>>>> 'mddev->pers' becomes NULL.
>>>>
>>>> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>>>>
>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
>>>> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
>>>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>>>> ---
>>>>    v1->v2:
>>>>     1. Add 'mddev->pers != NULL' check before make_request
>>>>     2. Delete dm-raid caller(->dtr) modifications
>>>>     3. Move memalloc_noio_restore after mddev_unlock_and_resume
>>>>    v2->v3:
>>>>     1. Remove modifications in array_state_store()
>>>>     2. update commit msg
>>>>    drivers/md/md.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>> index 680b34a63cb3..e73338c26d53 100644
>>>> --- a/drivers/md/md.c
>>>> +++ b/drivers/md/md.c
>>>> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>>>>    		if (!percpu_ref_tryget_live(&mddev->active_io))
>>>>    			goto check_suspended;
>>>>    	}
>>>> +	if (!mddev->pers) {
>>> Isn't this case already handled by md_submit_bio(). take a look in
>>> md_submit_bio()
>> Hi,Abd-Alrhman
>> I understand that 'mddev == NULL || mddev->pers == NULL' in
>> md_submit_bio() is a qiuck check, there still exists a small race window:
>>        P1                     P2
>> md_submit_bio
>>    if (mddev == NULL || mddev->pers == NULL)
>>                           md_ioctl
>>                            mddev_suspend_and_lock
>>                            do_md_stop->__md_stop
>>                            // set mddev->pers/private as NULL
>>                            mddev_unlock_and_resume
>>    md_handle_request
>>     if (is_suspended(mddev, bio))
>>     percpu_ref_tryget_live(&mddev->active_io)
>>     mddev->pers // null-ptr-deref
> 
> It makes sense. It would solve only the null pointer reference, but I
> see another issue. Look at the comment before calling
> mddev_set_closing_and_sync_blockdev() in md_ioctl(). it says "Need to
> flush page cache, and ensure no-one else opens and writes". The
> suspending happens after flushing the page cache, in this case,
> there might be other writes in-flight.

At present, the current implementation does not align with the expected 
annotations. Yu Kai privately gave me a feasible suggestion, which is to 
have md_ioctl operate on a character device(like /dev/mapper/control) 
instead of an md block device. This fix patch will serve as a temporary 
solution before the official implementation is merged. By the way, I 
don't have much time to implment it, do you have time to deal with the 
official solution?
> 
>>>
>>>> +		/*
>>>> +		 * The __md_stop() sets 'mddev->private' to NULL during
>>>> +		 * the IO submitting, check 'mddev->pers' before the IO
>>>> +		 * being processed by specific driver to avoid the
>>>> +		 * null-ptr-deref of 'mddev-><member>'. The check is
>>>> +		 * safe because the IO has got the 'mddev->active_io'
>>>> +		 * reference, and all __md_stop() callers will wait for
>>>> +		 * the reference to be zero.
>>>> +		 */
>>>> +		bio_io_error(bio);
>>>> +		percpu_ref_put(&mddev->active_io);
>>>> +		return true;
>>>> +	}
>>>>    	if (!mddev->pers->make_request(mddev, bio)) {
>>>>    		percpu_ref_put(&mddev->active_io);
>>>>    		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
>>>> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>>>>    	case HOT_REMOVE_DISK:
>>>>    	case SET_BITMAP_FILE:
>>>>    	case SET_ARRAY_INFO:
>>>> +	case STOP_ARRAY:
>>>>    		return true;
>>>>    	default:
>>>>    		return false;
>>>> -- 
>>>> 2.52.0
>>>>
>>>
>>
> 

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Abd-Alrhman Masalkhi]

Hi Zhihao,

On Fri, Sep 18, 2026 at 16:34 +0800, Zhihao Cheng wrote:
> 在 2026/9/18 16:20, Abd-Alrhman Masalkhi 写道:
>> On Fri, Sep 18, 2026 at 15:38 +0800, Zhihao Cheng wrote:
>>> 在 2026/9/18 15:29, Abd-Alrhman Masalkhi 写道:
>>>>
>>>> Hi Zhihao,
>>>>
>>>> On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
>>>>> Concurrent processes md_stop and IO submitting could trigger a
>>>>> null-ptr-deref of 'mddev->private':
>>>>>
>>>>>    BUG: kernel NULL pointer dereference, address: 0000000000000070
>>>>>    RIP: 0010:_wait_barrier+0x2f/0x250
>>>>>    Call Trace:
>>>>>     raid1_make_request+0x150/0xf50
>>>>>     md_handle_request+0x104/0x530
>>>>>     md_submit_bio+0x76/0x130
>>>>>     submit_bio+0xdd/0x250
>>>>>     submit_bio_wait+0x1f/0x40
>>>>>     __blkdev_direct_IO_simple+0x1f6/0x370
>>>>>     blkdev_write_iter+0x3b2/0x520
>>>>>     ksys_write+0x7d/0x190
>>>>>
>>>>>                 P1
>>>>>     fd = open(/dev/md0, O_RDWR)
>>>>> 	                      P2 (forked from P1, fd' <= fd)
>>>>>     write(fd)
>>>>>      submit_bio
>>>>>       md_handle_request
>>>>>        raid1_make_request
>>>>>         raid1_write_request
>>>>>                                     ioctl(fd, STOP_ARRAY)
>>>>> 		                   mddev_set_closing_and_sync_blockdev
>>>>> 				   // check passed, mddev->openers = 1,
>>>>> 				   // because md_open() is only called
>>>>> 				   // once in P1->open
>>>>> 				   do_md_stop
>>>>> 				    __md_stop
>>>>> 				     mddev->private = NULL
>>>>>
>>>>>          conf = mddev->private // NULL
>>>>>           wait_barrier(conf, sector) // null-ptr-deref !
>>>>>
>>>>> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
>>>>> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
>>>>> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
>>>>> The caller dm_table_destroy() is guaranteed being invoked with device
>>>>> suspended, so raid_dtr() could keep using mddev_lock_nointr().
>>>>> The caller array_state_store() is guaranteed by the check
>>>>> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
>>>>> /dev/mdx, write something and close /dev/mdx, it won't trigger the
>>>>> problem, all dirty pages can be flushed before mddev->openers decrement.
>>>>> Besides, fail the submitting IO in md_handle_request() if the
>>>>> 'mddev->pers' becomes NULL.
>>>>>
>>>>> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>>>>>
>>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>>> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
>>>>> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
>>>>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>>>>> ---
>>>>>    v1->v2:
>>>>>     1. Add 'mddev->pers != NULL' check before make_request
>>>>>     2. Delete dm-raid caller(->dtr) modifications
>>>>>     3. Move memalloc_noio_restore after mddev_unlock_and_resume
>>>>>    v2->v3:
>>>>>     1. Remove modifications in array_state_store()
>>>>>     2. update commit msg
>>>>>    drivers/md/md.c | 15 +++++++++++++++
>>>>>    1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>>> index 680b34a63cb3..e73338c26d53 100644
>>>>> --- a/drivers/md/md.c
>>>>> +++ b/drivers/md/md.c
>>>>> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>>>>>    		if (!percpu_ref_tryget_live(&mddev->active_io))
>>>>>    			goto check_suspended;
>>>>>    	}
>>>>> +	if (!mddev->pers) {
>>>> Isn't this case already handled by md_submit_bio(). take a look in
>>>> md_submit_bio()
>>> Hi,Abd-Alrhman
>>> I understand that 'mddev == NULL || mddev->pers == NULL' in
>>> md_submit_bio() is a qiuck check, there still exists a small race window:
>>>        P1                     P2
>>> md_submit_bio
>>>    if (mddev == NULL || mddev->pers == NULL)
>>>                           md_ioctl
>>>                            mddev_suspend_and_lock
>>>                            do_md_stop->__md_stop
>>>                            // set mddev->pers/private as NULL
>>>                            mddev_unlock_and_resume
>>>    md_handle_request
>>>     if (is_suspended(mddev, bio))
>>>     percpu_ref_tryget_live(&mddev->active_io)
>>>     mddev->pers // null-ptr-deref
>> 
>> It makes sense. It would solve only the null pointer reference, but I
>> see another issue. Look at the comment before calling
>> mddev_set_closing_and_sync_blockdev() in md_ioctl(). it says "Need to
>> flush page cache, and ensure no-one else opens and writes". The
>> suspending happens after flushing the page cache, in this case,
>> there might be other writes in-flight.
>
> At present, the current implementation does not align with the expected 
> annotations. Yu Kai privately gave me a feasible suggestion, which is to 
> have md_ioctl operate on a character device(like /dev/mapper/control) 
> instead of an md block device. This fix patch will serve as a temporary 
> solution before the official implementation is merged. By the way, I 
> don't have much time to implment it, do you have time to deal with the 
> official solution?

Thanks for the summary. I can take on the official implementation.
Before I get started, could we align on the interface details and any
other relevant considerations?

>> 
>>>>
>>>>> +		/*
>>>>> +		 * The __md_stop() sets 'mddev->private' to NULL during
>>>>> +		 * the IO submitting, check 'mddev->pers' before the IO
>>>>> +		 * being processed by specific driver to avoid the
>>>>> +		 * null-ptr-deref of 'mddev-><member>'. The check is
>>>>> +		 * safe because the IO has got the 'mddev->active_io'
>>>>> +		 * reference, and all __md_stop() callers will wait for
>>>>> +		 * the reference to be zero.
>>>>> +		 */
>>>>> +		bio_io_error(bio);
>>>>> +		percpu_ref_put(&mddev->active_io);
>>>>> +		return true;
>>>>> +	}
>>>>>    	if (!mddev->pers->make_request(mddev, bio)) {
>>>>>    		percpu_ref_put(&mddev->active_io);
>>>>>    		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
>>>>> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>>>>>    	case HOT_REMOVE_DISK:
>>>>>    	case SET_BITMAP_FILE:
>>>>>    	case SET_ARRAY_INFO:
>>>>> +	case STOP_ARRAY:
>>>>>    		return true;
>>>>>    	default:
>>>>>    		return false;
>>>>> -- 
>>>>> 2.52.0
>>>>>
>>>>
>>>
>> 
>

-- 
Best Regards,
Abd-Alrhman

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[yu kuai]

Hi,

在 2026/9/18 16:54, Abd-Alrhman Masalkhi 写道:
> Hi Zhihao,
>
> On Fri, Sep 18, 2026 at 16:34 +0800, Zhihao Cheng wrote:
>> 在 2026/9/18 16:20, Abd-Alrhman Masalkhi 写道:
>>> On Fri, Sep 18, 2026 at 15:38 +0800, Zhihao Cheng wrote:
>>>> 在 2026/9/18 15:29, Abd-Alrhman Masalkhi 写道:
>>>>> Hi Zhihao,
>>>>>
>>>>> On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
>>>>>> Concurrent processes md_stop and IO submitting could trigger a
>>>>>> null-ptr-deref of 'mddev->private':
>>>>>>
>>>>>>     BUG: kernel NULL pointer dereference, address: 0000000000000070
>>>>>>     RIP: 0010:_wait_barrier+0x2f/0x250
>>>>>>     Call Trace:
>>>>>>      raid1_make_request+0x150/0xf50
>>>>>>      md_handle_request+0x104/0x530
>>>>>>      md_submit_bio+0x76/0x130
>>>>>>      submit_bio+0xdd/0x250
>>>>>>      submit_bio_wait+0x1f/0x40
>>>>>>      __blkdev_direct_IO_simple+0x1f6/0x370
>>>>>>      blkdev_write_iter+0x3b2/0x520
>>>>>>      ksys_write+0x7d/0x190
>>>>>>
>>>>>>                  P1
>>>>>>      fd = open(/dev/md0, O_RDWR)
>>>>>> 	                      P2 (forked from P1, fd' <= fd)
>>>>>>      write(fd)
>>>>>>       submit_bio
>>>>>>        md_handle_request
>>>>>>         raid1_make_request
>>>>>>          raid1_write_request
>>>>>>                                      ioctl(fd, STOP_ARRAY)
>>>>>> 		                   mddev_set_closing_and_sync_blockdev
>>>>>> 				   // check passed, mddev->openers = 1,
>>>>>> 				   // because md_open() is only called
>>>>>> 				   // once in P1->open
>>>>>> 				   do_md_stop
>>>>>> 				    __md_stop
>>>>>> 				     mddev->private = NULL
>>>>>>
>>>>>>           conf = mddev->private // NULL
>>>>>>            wait_barrier(conf, sector) // null-ptr-deref !
>>>>>>
>>>>>> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
>>>>>> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
>>>>>> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
>>>>>> The caller dm_table_destroy() is guaranteed being invoked with device
>>>>>> suspended, so raid_dtr() could keep using mddev_lock_nointr().
>>>>>> The caller array_state_store() is guaranteed by the check
>>>>>> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
>>>>>> /dev/mdx, write something and close /dev/mdx, it won't trigger the
>>>>>> problem, all dirty pages can be flushed before mddev->openers decrement.
>>>>>> Besides, fail the submitting IO in md_handle_request() if the
>>>>>> 'mddev->pers' becomes NULL.
>>>>>>
>>>>>> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>>>>>>
>>>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>>>> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
>>>>>> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
>>>>>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>>>>>> ---
>>>>>>     v1->v2:
>>>>>>      1. Add 'mddev->pers != NULL' check before make_request
>>>>>>      2. Delete dm-raid caller(->dtr) modifications
>>>>>>      3. Move memalloc_noio_restore after mddev_unlock_and_resume
>>>>>>     v2->v3:
>>>>>>      1. Remove modifications in array_state_store()
>>>>>>      2. update commit msg
>>>>>>     drivers/md/md.c | 15 +++++++++++++++
>>>>>>     1 file changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>>>> index 680b34a63cb3..e73338c26d53 100644
>>>>>> --- a/drivers/md/md.c
>>>>>> +++ b/drivers/md/md.c
>>>>>> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>>>>>>     		if (!percpu_ref_tryget_live(&mddev->active_io))
>>>>>>     			goto check_suspended;
>>>>>>     	}
>>>>>> +	if (!mddev->pers) {
>>>>> Isn't this case already handled by md_submit_bio(). take a look in
>>>>> md_submit_bio()
>>>> Hi,Abd-Alrhman
>>>> I understand that 'mddev == NULL || mddev->pers == NULL' in
>>>> md_submit_bio() is a qiuck check, there still exists a small race window:
>>>>         P1                     P2
>>>> md_submit_bio
>>>>     if (mddev == NULL || mddev->pers == NULL)
>>>>                            md_ioctl
>>>>                             mddev_suspend_and_lock
>>>>                             do_md_stop->__md_stop
>>>>                             // set mddev->pers/private as NULL
>>>>                             mddev_unlock_and_resume
>>>>     md_handle_request
>>>>      if (is_suspended(mddev, bio))
>>>>      percpu_ref_tryget_live(&mddev->active_io)
>>>>      mddev->pers // null-ptr-deref
>>> It makes sense. It would solve only the null pointer reference, but I
>>> see another issue. Look at the comment before calling
>>> mddev_set_closing_and_sync_blockdev() in md_ioctl(). it says "Need to
>>> flush page cache, and ensure no-one else opens and writes". The
>>> suspending happens after flushing the page cache, in this case,
>>> there might be other writes in-flight.
>> At present, the current implementation does not align with the expected
>> annotations. Yu Kai privately gave me a feasible suggestion, which is to
>> have md_ioctl operate on a character device(like /dev/mapper/control)
>> instead of an md block device. This fix patch will serve as a temporary
>> solution before the official implementation is merged. By the way, I
>> don't have much time to implment it, do you have time to deal with the
>> official solution?
> Thanks for the summary. I can take on the official implementation.
> Before I get started, could we align on the interface details and any
> other relevant considerations?

I think the following procedures(details should be considered more):

1) add a new char device in mdraid to issue ioctl to array;
2) update mdadm to use the new ioctl;
3) a kernel message to warn the old block device ioctl is deprecated, and suggest
user to upgrade mdadm;
4) Since the new ioctl will introduce UAPI change, we should wait for a long time,
perhaps more than 1 year, before the deprecated old ioctl code in kernel can be removed.

>
>>>>>> +		/*
>>>>>> +		 * The __md_stop() sets 'mddev->private' to NULL during
>>>>>> +		 * the IO submitting, check 'mddev->pers' before the IO
>>>>>> +		 * being processed by specific driver to avoid the
>>>>>> +		 * null-ptr-deref of 'mddev-><member>'. The check is
>>>>>> +		 * safe because the IO has got the 'mddev->active_io'
>>>>>> +		 * reference, and all __md_stop() callers will wait for
>>>>>> +		 * the reference to be zero.
>>>>>> +		 */
>>>>>> +		bio_io_error(bio);
>>>>>> +		percpu_ref_put(&mddev->active_io);
>>>>>> +		return true;
>>>>>> +	}
>>>>>>     	if (!mddev->pers->make_request(mddev, bio)) {
>>>>>>     		percpu_ref_put(&mddev->active_io);
>>>>>>     		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
>>>>>> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>>>>>>     	case HOT_REMOVE_DISK:
>>>>>>     	case SET_BITMAP_FILE:
>>>>>>     	case SET_ARRAY_INFO:
>>>>>> +	case STOP_ARRAY:
>>>>>>     		return true;
>>>>>>     	default:
>>>>>>     		return false;
>>>>>> -- 
>>>>>> 2.52.0
>>>>>>
-- 
Thanks,
Kuai

Re: [PATCH v3] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO

[Abd-Alrhman Masalkhi]

Hi Kuai,

On Fri, Sep 18, 2026 at 17:30 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/9/18 16:54, Abd-Alrhman Masalkhi 写道:
>> Hi Zhihao,
>>
>> On Fri, Sep 18, 2026 at 16:34 +0800, Zhihao Cheng wrote:
>>> 在 2026/9/18 16:20, Abd-Alrhman Masalkhi 写道:
>>>> On Fri, Sep 18, 2026 at 15:38 +0800, Zhihao Cheng wrote:
>>>>> 在 2026/9/18 15:29, Abd-Alrhman Masalkhi 写道:
>>>>>> Hi Zhihao,
>>>>>>
>>>>>> On Fri, Sep 18, 2026 at 15:09 +0800, Zhihao Cheng wrote:
>>>>>>> Concurrent processes md_stop and IO submitting could trigger a
>>>>>>> null-ptr-deref of 'mddev->private':
>>>>>>>
>>>>>>>     BUG: kernel NULL pointer dereference, address: 0000000000000070
>>>>>>>     RIP: 0010:_wait_barrier+0x2f/0x250
>>>>>>>     Call Trace:
>>>>>>>      raid1_make_request+0x150/0xf50
>>>>>>>      md_handle_request+0x104/0x530
>>>>>>>      md_submit_bio+0x76/0x130
>>>>>>>      submit_bio+0xdd/0x250
>>>>>>>      submit_bio_wait+0x1f/0x40
>>>>>>>      __blkdev_direct_IO_simple+0x1f6/0x370
>>>>>>>      blkdev_write_iter+0x3b2/0x520
>>>>>>>      ksys_write+0x7d/0x190
>>>>>>>
>>>>>>>                  P1
>>>>>>>      fd = open(/dev/md0, O_RDWR)
>>>>>>> 	                      P2 (forked from P1, fd' <= fd)
>>>>>>>      write(fd)
>>>>>>>       submit_bio
>>>>>>>        md_handle_request
>>>>>>>         raid1_make_request
>>>>>>>          raid1_write_request
>>>>>>>                                      ioctl(fd, STOP_ARRAY)
>>>>>>> 		                   mddev_set_closing_and_sync_blockdev
>>>>>>> 				   // check passed, mddev->openers = 1,
>>>>>>> 				   // because md_open() is only called
>>>>>>> 				   // once in P1->open
>>>>>>> 				   do_md_stop
>>>>>>> 				    __md_stop
>>>>>>> 				     mddev->private = NULL
>>>>>>>
>>>>>>>           conf = mddev->private // NULL
>>>>>>>            wait_barrier(conf, sector) // null-ptr-deref !
>>>>>>>
>>>>>>> It is a common problem for raid0/1/10/5, and __md_stop could be triggered
>>>>>>> by several paths(eg. ioctl, sysfs, ->dtr). Fix it by replacing
>>>>>>> mddev_lock() with mddev_suspend_and_lock() for all __md_stop() callers.
>>>>>>> The caller dm_table_destroy() is guaranteed being invoked with device
>>>>>>> suspended, so raid_dtr() could keep using mddev_lock_nointr().
>>>>>>> The caller array_state_store() is guaranteed by the check
>>>>>>> mddev_set_closing_and_sync_blockdev(mddev, 0). For example, someone open
>>>>>>> /dev/mdx, write something and close /dev/mdx, it won't trigger the
>>>>>>> problem, all dirty pages can be flushed before mddev->openers decrement.
>>>>>>> Besides, fail the submitting IO in md_handle_request() if the
>>>>>>> 'mddev->pers' becomes NULL.
>>>>>>>
>>>>>>> Fetch a reproducer in https://bugzilla.kernel.org/show_bug.cgi?id=222020
>>>>>>>
>>>>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>>>>> Reported-by: syzbot+3fe892ea5fc292e1353f@syzkaller.appspotmail.com
>>>>>>> Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f
>>>>>>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>>>>>>> ---
>>>>>>>     v1->v2:
>>>>>>>      1. Add 'mddev->pers != NULL' check before make_request
>>>>>>>      2. Delete dm-raid caller(->dtr) modifications
>>>>>>>      3. Move memalloc_noio_restore after mddev_unlock_and_resume
>>>>>>>     v2->v3:
>>>>>>>      1. Remove modifications in array_state_store()
>>>>>>>      2. update commit msg
>>>>>>>     drivers/md/md.c | 15 +++++++++++++++
>>>>>>>     1 file changed, 15 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>>>>> index 680b34a63cb3..e73338c26d53 100644
>>>>>>> --- a/drivers/md/md.c
>>>>>>> +++ b/drivers/md/md.c
>>>>>>> @@ -414,6 +414,20 @@ bool md_handle_request(struct mddev *mddev, struct bio *bio)
>>>>>>>     		if (!percpu_ref_tryget_live(&mddev->active_io))
>>>>>>>     			goto check_suspended;
>>>>>>>     	}
>>>>>>> +	if (!mddev->pers) {
>>>>>> Isn't this case already handled by md_submit_bio(). take a look in
>>>>>> md_submit_bio()
>>>>> Hi,Abd-Alrhman
>>>>> I understand that 'mddev == NULL || mddev->pers == NULL' in
>>>>> md_submit_bio() is a qiuck check, there still exists a small race window:
>>>>>         P1                     P2
>>>>> md_submit_bio
>>>>>     if (mddev == NULL || mddev->pers == NULL)
>>>>>                            md_ioctl
>>>>>                             mddev_suspend_and_lock
>>>>>                             do_md_stop->__md_stop
>>>>>                             // set mddev->pers/private as NULL
>>>>>                             mddev_unlock_and_resume
>>>>>     md_handle_request
>>>>>      if (is_suspended(mddev, bio))
>>>>>      percpu_ref_tryget_live(&mddev->active_io)
>>>>>      mddev->pers // null-ptr-deref
>>>> It makes sense. It would solve only the null pointer reference, but I
>>>> see another issue. Look at the comment before calling
>>>> mddev_set_closing_and_sync_blockdev() in md_ioctl(). it says "Need to
>>>> flush page cache, and ensure no-one else opens and writes". The
>>>> suspending happens after flushing the page cache, in this case,
>>>> there might be other writes in-flight.
>>> At present, the current implementation does not align with the expected
>>> annotations. Yu Kai privately gave me a feasible suggestion, which is to
>>> have md_ioctl operate on a character device(like /dev/mapper/control)
>>> instead of an md block device. This fix patch will serve as a temporary
>>> solution before the official implementation is merged. By the way, I
>>> don't have much time to implment it, do you have time to deal with the
>>> official solution?
>> Thanks for the summary. I can take on the official implementation.
>> Before I get started, could we align on the interface details and any
>> other relevant considerations?
>
> I think the following procedures(details should be considered more):
>
> 1) add a new char device in mdraid to issue ioctl to array;
> 2) update mdadm to use the new ioctl;
> 3) a kernel message to warn the old block device ioctl is deprecated, and suggest
> user to upgrade mdadm;
> 4) Since the new ioctl will introduce UAPI change, we should wait for a long time,
> perhaps more than 1 year, before the deprecated old ioctl code in kernel can be removed.
>

Thanks, I’ll start working on the implementation.

>>
>>>>>>> +		/*
>>>>>>> +		 * The __md_stop() sets 'mddev->private' to NULL during
>>>>>>> +		 * the IO submitting, check 'mddev->pers' before the IO
>>>>>>> +		 * being processed by specific driver to avoid the
>>>>>>> +		 * null-ptr-deref of 'mddev-><member>'. The check is
>>>>>>> +		 * safe because the IO has got the 'mddev->active_io'
>>>>>>> +		 * reference, and all __md_stop() callers will wait for
>>>>>>> +		 * the reference to be zero.
>>>>>>> +		 */
>>>>>>> +		bio_io_error(bio);
>>>>>>> +		percpu_ref_put(&mddev->active_io);
>>>>>>> +		return true;
>>>>>>> +	}
>>>>>>>     	if (!mddev->pers->make_request(mddev, bio)) {
>>>>>>>     		percpu_ref_put(&mddev->active_io);
>>>>>>>     		if (mddev_is_dm(mddev) && mddev->pers->prepare_suspend)
>>>>>>> @@ -8299,6 +8313,7 @@ static bool md_ioctl_need_suspend(unsigned int cmd)
>>>>>>>     	case HOT_REMOVE_DISK:
>>>>>>>     	case SET_BITMAP_FILE:
>>>>>>>     	case SET_ARRAY_INFO:
>>>>>>> +	case STOP_ARRAY:
>>>>>>>     		return true;
>>>>>>>     	default:
>>>>>>>     		return false;
>>>>>>> -- 
>>>>>>> 2.52.0
>>>>>>>
> -- 
> Thanks,
> Kuai

-- 
Best Regards,
Abd-Alrhman