All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Zhihao Cheng" <chengzhihao1@huawei.com>
Cc: linux-raid@vger.kernel.org, yukuai@fygo.io
Subject: Re: [PATCH] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO
Date: Wed, 16 Sep 2026 12:33:00 +0000	[thread overview]
Message-ID: <20260916123301.77B101F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260916121120.1242281-1-chengzhihao1@huawei.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] md: NULL pointer dereference in md_handle_request() on array teardown
- [High] md: memory reclaim deadlock due to premature memalloc_noio_restore
--

commit 86c9448b538e3d50a408040bfd47956ef0397e1a
Author: Zhihao Cheng <chengzhihao1@huawei.com>

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

This patch replaces mddev_lock() with mddev_suspend_and_lock() across
__md_stop() callers. It aims to prevent concurrent IO submissions from
dereferencing mddev->private while the array is being torn down.

Closes: https://syzkaller.appspot.com/bug?extid=3fe892ea5fc292e1353f

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..e292acd54bb00 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -4775,7 +4781,12 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
>  			mddev->hold_active = 0;
>  		sysfs_notify_dirent_safe(mddev->sysfs_state);
>  	}
> -	mddev_unlock(mddev);
> +	if (suspend) {
> +		memalloc_noio_restore(noio_flags);
> +		mddev_unlock_and_resume(mddev);

[Severity: Critical]
Could unconditionally resuming the array here lead to a NULL pointer
dereference for trapped IOs?

If an IO is submitted concurrently with array teardown (e.g. sysfs inactive
or STOP_ARRAY ioctl), it is trapped in md_handle_request() by wait_event()
waiting for the array to not be suspended.

The teardown thread sets mddev->pers = NULL in md_stop() and then calls
mddev_unlock_and_resume() here. The resumption resurrects active_io and wakes
up the trapped IO, which then blindly dereferences mddev->pers:

drivers/md/md.c:md_handle_request() {
        ...
        if (!percpu_ref_tryget_live(&mddev->active_io))
                goto check_suspended;
        }
        if (!mddev->pers->make_request(mddev, bio)) {
        ...
}

[Severity: High]
Is there a risk of a memory reclaim deadlock due to the NOIO restoration order
here?

By restoring NOIO flags before calling mddev_unlock_and_resume(), we run
mddev_unlock() without NOIO protection while the array is still suspended:

drivers/md/md.h:mddev_unlock_and_resume() {
        mddev_unlock(mddev);
        mddev_resume(mddev);
}

If memory reclaim is triggered by allocations inside mddev_unlock() and tries
to submit IO to this suspended array, it will block indefinitely waiting for
the array to resume, preventing this thread from ever reaching mddev_resume().

> +	} else {
> +		mddev_unlock(mddev);
> +	}
>  
>  	if (st == readonly || st == read_auto || st == inactive ||
>  	    (err && st == clear))
[ ... ]
> @@ -8299,6 +8310,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:

[Severity: Critical]
Might unconditionally suspending and then resuming the array during STOP_ARRAY
lead to a similar NULL pointer dereference for trapped IOs?

Because STOP_ARRAY now returns true here, the existing logic in md_ioctl()
will suspend the array, call do_md_stop(), and then unconditionally restore
NOIO flags and call mddev_unlock_and_resume(mddev).

This allows trapped IOs waiting in md_handle_request() to wake up after
mddev->pers has been cleared, resulting in a NULL pointer dereference when
they attempt to call mddev->pers->make_request().

[Severity: High]
Will this NOIO restoration order cause the same memory reclaim deadlock in the
ioctl path?

As in the sysfs path, md_ioctl() now suspends and later restores NOIO before
mddev_unlock_and_resume(mddev), risking a deadlock if allocations within
mddev_unlock() trigger reclaim that submits IO to the suspended array:

drivers/md/md.c:md_ioctl() {
        ...
        if (suspend) {
                memalloc_noio_restore(noio_flags);
                mddev_unlock_and_resume(mddev);
        ...
}

>  		return true;
>  	default:
>  		return false;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916121120.1242281-1-chengzhihao1@huawei.com?part=1

      reply	other threads:[~2026-09-16 12:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16 12:11 [PATCH] md: Fix the null-ptr-deref of 'mddev->private' while submitting IO Zhihao Cheng
2026-09-16 12:33 ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260916123301.77B101F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=chengzhihao1@huawei.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yukuai@fygo.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.