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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] md: MD_CLOSING flag is not cleared on error or RO transition
- [Medium] md: strict boundary check on UAPI name field rejects valid inputs
--

commit c2d6351bf75bbe25a5734b17638d14317134886c
Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>

md: add a control device for array management

This commit introduces a new misc character device, /dev/md-control, to handle
md management ioctls without requiring an open file descriptor to the md block
device itself. This prevents races during array stop and reconfiguration when
multiple tasks share the same file descriptor table. The existing block device
ioctls are preserved for compatibility but trigger a warning recommending an
upgrade to mdadm.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -8602,6 +8608,623 @@ static int md_compat_ioctl(struct block_device *bdev, blk_mode_t mode,
[ ... ]
> +static struct mddev *mdctl_get_mddev_name(const char *name)
> +{
> +	unsigned long minor;
> +	struct mddev *mddev, *ret = ERR_PTR(-EINVAL);
> +
> +	if (strncmp(name, "md", 2) || name[MD_NAME_LEN - 1])
> +		return ret;

[Severity: Medium]
Will this strict boundary check reject valid user input? If a userspace
program populates the name field with a short, properly null-terminated
string but leaves the trailing padding bytes uninitialized on the stack,
this check will fail because the absolute last byte isn't zero. Could
this be relaxed to ensure the string is null-terminated anywhere within
the buffer instead?

[ ... ]
> +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:
> +	mddev_put(mddev);
> +	return err;
> +}

[Severity: High]
Does this exit path need to clear the MD_CLOSING flag if it was set
earlier? When a privileged user calls MD_STOP_ARRAY and the array is
busy or transitions to read-only, MD_CLOSING is successfully set, but
the ioctl may later jump to out or return an error. Since the flag is
never cleared on these paths in mdctl_ioctl(), would this leave the
block device permanently locked out from future opens until reboot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001125611.943731-1-abd.masalkhi@gmail.com?part=4