Re: [RFC PATCH v3 4/4] md: add a control device for array management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] md: MD_CLOSING mutual exclusion broken in mdctl_ioctl
--

commit fe7502eb38662870e8ecf28880f3055a33951d2a
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gatech.edu>

md: add a control device for array management

This commit adds a misc character device, /dev/md-control, to handle md
management ioctls without opening the md block device. This aims to resolve
an issue where STOP_ARRAY races with concurrent writes from tasks sharing
the same file descriptor table.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> +static long mdctl_ioctl(struct file *filp, uint cmd, ulong u)
> +{
[ ... ]
> +	if (cmd == MD_STOP_ARRAY) {
> +		/* Need to flush page cache, and ensure no openers */
> +		err = mddev_set_closing_and_sync_blockdev(mddev, 0);
> +		if (err)
> +			goto out;
> +	}
[ ... ]
> +out:
> +	if (cmd == MD_STOP_ARRAY)
> +		clear_bit(MD_CLOSING, &mddev->flags);
> +
> +	mddev_put(mddev);
> +	return err;
> +}

[Severity: High]
Does this unconditional clearance of MD_CLOSING destroy mutual exclusion for
concurrent stop attempts?

If two tasks call mdctl_ioctl() with MD_STOP_ARRAY concurrently:

Task A succeeds in mddev_set_closing_and_sync_blockdev(), which sets
MD_CLOSING.

Task B fails in mddev_set_closing_and_sync_blockdev() because MD_CLOSING is
already set, returning -EBUSY and jumping to out.

Task B then unconditionally clears MD_CLOSING at the out label.

This clears the flag while Task A is still relying on it to prevent
concurrent openers or operations, potentially circumventing the protections
this patch aims to provide. Should this bit only be cleared if the task
actually set it, or removed entirely if the old md_ioctl() didn't need to
clear it here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007115104.13884-1-abd.masalkhi@gatech.edu?part=4