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

["Masalkhi, Abd Alrhman" <abd.masalkhi@gatech.edu>]

Hi Kuai,

On Thu, Oct 01, 2026 at 13:19 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issu=(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
I will fix those in v3.

> --
>
> 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 h=ndle
> md management ioctls without requiring an open file descriptor to the md =lock
> device itself. This prevents races during array stop and reconfiguration =hen
> multiple tasks share the same file descriptor table. The existing block d=vice
> ioctls are preserved for compatibility but trigger a warning recommending=n
> 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 *=dev, 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.94=731-1-abd.masalkhi@gmail.com?part=4

-- 
Best Regards,
Abd-Alrhman