mirror of
https://github.com/torvalds/linux.git
synced 2026-04-18 06:44:00 -04:00
md: call del_gendisk in control path
Now del_gendisk and put_disk are called asynchronously in workqueue work. The asynchronous way has a problem that the device node can still exist after mdadm --stop command returns in a short window. So udev rule can open this device node and create the struct mddev in kernel again. So put del_gendisk in control path and still leave put_disk in md_kobj_release to avoid uaf of gendisk. Function del_gendisk can't be called with reconfig_mutex. If it's called with reconfig mutex, a deadlock can happen. del_gendisk waits all sysfs files access to finish and sysfs file access waits reconfig mutex. So put del_gendisk after releasing reconfig mutex. But there is still a window that sysfs can be accessed between mddev_unlock and del_gendisk. So some actions (add disk, change level, .e.g) can happen which lead unexpected results. MD_DELETED is used to resolve this problem. MD_DELETED is set before releasing reconfig mutex and it should be checked for these sysfs access which need reconfig mutex. For sysfs access which don't need reconfig mutex, del_gendisk will wait them to finish. But it doesn't need to do this in function mddev_lock_nointr. There are ten places that call it. * Five of them are in dm raid which we don't need to care. MD_DELETED is only used for md raid. * stop_sync_thread, md_do_sync and md_start_sync are related sync request, and it needs to wait sync thread to finish before stopping an array. * md_ioctl: md_open is called before md_ioctl, so ->openers is added. It will fail to stop the array. So it doesn't need to check MD_DELETED here * md_set_readonly: It needs to call mddev_set_closing_and_sync_blockdev when setting readonly or read_auto. So it will fail to stop the array too because MD_CLOSING is already set. Reviewed-by: Yu Kuai <yukuai3@huawei.com> Signed-off-by: Xiao Ni <xni@redhat.com> Link: https://lore.kernel.org/linux-raid/20250611073108.25463-2-xni@redhat.com Signed-off-by: Yu Kuai <yukuai3@huawei.com>
This commit is contained in:
@@ -700,11 +700,26 @@ static inline bool reshape_interrupted(struct mddev *mddev)
|
||||
|
||||
static inline int __must_check mddev_lock(struct mddev *mddev)
|
||||
{
|
||||
return mutex_lock_interruptible(&mddev->reconfig_mutex);
|
||||
int ret;
|
||||
|
||||
ret = mutex_lock_interruptible(&mddev->reconfig_mutex);
|
||||
|
||||
/* MD_DELETED is set in do_md_stop with reconfig_mutex.
|
||||
* So check it here.
|
||||
*/
|
||||
if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
|
||||
ret = -ENODEV;
|
||||
mutex_unlock(&mddev->reconfig_mutex);
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* Sometimes we need to take the lock in a situation where
|
||||
* failure due to interrupts is not acceptable.
|
||||
* It doesn't need to check MD_DELETED here, the owner which
|
||||
* holds the lock here can't be stopped. And all paths can't
|
||||
* call this function after do_md_stop.
|
||||
*/
|
||||
static inline void mddev_lock_nointr(struct mddev *mddev)
|
||||
{
|
||||
@@ -713,7 +728,14 @@ static inline void mddev_lock_nointr(struct mddev *mddev)
|
||||
|
||||
static inline int mddev_trylock(struct mddev *mddev)
|
||||
{
|
||||
return mutex_trylock(&mddev->reconfig_mutex);
|
||||
int ret;
|
||||
|
||||
ret = mutex_trylock(&mddev->reconfig_mutex);
|
||||
if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
|
||||
ret = -ENODEV;
|
||||
mutex_unlock(&mddev->reconfig_mutex);
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
extern void mddev_unlock(struct mddev *mddev);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user