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

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

Hi Kuai,

On Wed, Oct 07, 2026 at 12:09 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issu=(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 resol=e
> 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)

Sorry I do not fix it correctly. I'll fix it in the next version.

>> +		clear_bit(MD_CLOSING, &mddev->flags);
>> +
>> +	mddev_put(mddev);
>> +	return err;
>> +}
>
> [Severity: High]
> Does this unconditional clearance of MD_CLOSING destroy mutual exclusion =or
> 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 =s
> 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 protectio=s
> 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.13=84-1-abd.masalkhi@gatech.edu?part=4
>

-- 
Best Regards,
Abd-Alrhman