Re: [PATCH] md: allow growing logical_block_size on a running array

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] md: Unbalanced mutex unlock on mddev_lock() failure in lbs_store()
- [High] md: TOCTOU race in lbs_store() bypasses opener check and deadlocks via queue freeze
--

commit 4267b7426d911d22ecfdc4aa203d879ce4f9a759
Author: Jack Wang <jinpu.wang@cloud.ionos.com>

    md: allow growing logical_block_size on a running array
    
    Growing logical_block_size on a running array required stopping it
    first. Handle it directly in lbs_store(), similar to
    mddev_set_closing_and_sync_blockdev(): under open_mutex, refuse with
    -EBUSY if the array is closing or has openers, otherwise apply the
    bigger LBS.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..c885d405bf466 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c

[ ... ]

> @@ -6026,8 +6026,36 @@ lbs_store(struct mddev *mddev, const char *buf, size_t len)
>  	if (mddev->pers) {

[Severity: High]
Since this check of mddev->pers is performed locklessly, can a concurrent
thread starting the array race with it?

If mddev->pers is null, we fall through to the bottom path and block on
mddev_lock(mddev). Concurrently, another thread executing do_md_run() could
acquire reconfig_mutex, set mddev->pers to non-null, start the array, and
release the lock.

When lbs_store() wakes up and acquires reconfig_mutex, it does not re-verify
mddev->pers or check atomic_read(&mddev->openers) > 0 before calling
mddev_set_logical_block_size():

drivers/md/md.c:lbs_store() {
    ...
	if (mddev->pers) {
        ...
		return len;
	}

	err = mddev_lock(mddev);
	if (err)
		goto unlock;

	err = mddev_set_logical_block_size(mddev, lbs);
    ...
}

Does this blindly apply a new logical block size to a running array with
active openers? Furthermore, since mddev_set_logical_block_size() invokes
queue_limits_commit_update_frozen() which waits for all active I/O to drain
while holding reconfig_mutex, could this deadlock if active I/O completion
requires reconfig_mutex?

[Severity: High]
If mddev_lock() fails (e.g., interrupted by a signal, returning -EINTR),
does it jump to an unbalanced unlock?

drivers/md/md.c:lbs_store() {
    ...
	err = mddev_lock(mddev);
	if (err)
		goto unlock;

	err = mddev_set_logical_block_size(mddev, lbs);

unlock:
	mddev_unlock(mddev);
	return err ?: len;
}

Because mddev_lock() failed, reconfig_mutex was not acquired, but the unlock
label unconditionally calls mddev_unlock(mddev) to release it. Could this
attempt to unlock an unheld mutex and trigger a lockdep splat or kernel
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014114830.615589-1-jinpu.wang@ionos.com?part=1