* Re: [PATCH v5] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO
[not found] <20260922030538.1634904-1-chengzhihao1@huawei.com>
@ 2026-09-22 3:25 ` Zhihao Cheng
2026-10-04 15:51 ` yu kuai
1 sibling, 0 replies; 2+ messages in thread
From: Zhihao Cheng @ 2026-09-22 3:25 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao, eadavis, abd.masalkhi
Cc: linux-kernel, yangerkun, yi.zhang, linux-raid
在 2026/9/22 11:05, Zhihao Cheng 写道:
Cc linux-raid@vger.kernel.org
> 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 array_state_store() is guaranteed by the check
> mddev_set_closing_and_sync_blockdev(mddev, 0), we just need to remove
> the check '!md_is_rdwr'. 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.
> The caller dm_table_destroy() is guaranteed being invoked with device
> suspended, so raid_dtr() could keep using mddev_lock_nointr().
> 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
> v3->v4:
> 1. Remove '!md_is_rdwr' check from array_state_store()
> 2. update commit msg
> v4->v5:
> 1. Skip checking '!md_is_rdwr' only for inactive case
> drivers/md/md.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3..02798f2dbf0e 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)
> @@ -4670,7 +4684,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
> case readonly:
> case inactive:
> case read_auto:
> - if (!mddev->pers || !md_is_rdwr(mddev))
> + if (!mddev->pers || (st != inactive && !md_is_rdwr(mddev)))
> break;
> /* write sysfs will not open mddev and opener should be 0 */
> err = mddev_set_closing_and_sync_blockdev(mddev, 0);
> @@ -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;
>
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH v5] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO
[not found] <20260922030538.1634904-1-chengzhihao1@huawei.com>
2026-09-22 3:25 ` [PATCH v5] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO Zhihao Cheng
@ 2026-10-04 15:51 ` yu kuai
1 sibling, 0 replies; 2+ messages in thread
From: yu kuai @ 2026-10-04 15:51 UTC (permalink / raw)
To: Zhihao Cheng, song, magiclinan, xiao, eadavis, abd.masalkhi,
yu kuai
Cc: linux-kernel, yangerkun, yi.zhang, linux-raid@vger.kernel.org
Hi,
在 2026/9/22 11:05, 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 array_state_store() is guaranteed by the check
> mddev_set_closing_and_sync_blockdev(mddev, 0), we just need to remove
> the check '!md_is_rdwr'. 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.
> The caller dm_table_destroy() is guaranteed being invoked with device
> suspended, so raid_dtr() could keep using mddev_lock_nointr().
> 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
> v3->v4:
> 1. Remove '!md_is_rdwr' check from array_state_store()
> 2. update commit msg
> v4->v5:
> 1. Skip checking '!md_is_rdwr' only for inactive case
> drivers/md/md.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
This patch is missed because it's not in patchwork. :(
Applied to md-7.4
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3..02798f2dbf0e 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)
> @@ -4670,7 +4684,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
> case readonly:
> case inactive:
> case read_auto:
> - if (!mddev->pers || !md_is_rdwr(mddev))
> + if (!mddev->pers || (st != inactive && !md_is_rdwr(mddev)))
> break;
> /* write sysfs will not open mddev and opener should be 0 */
> err = mddev_set_closing_and_sync_blockdev(mddev, 0);
> @@ -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;
--
Thanks,
Kuai
^ permalink raw reply [flat|nested] 2+ messages in thread