On Tue, Apr 22, 2025 at 1:51 PM Christoph Hellwig <hch@xxxxxx> wrote:
>
> On Tue, Apr 22, 2025 at 11:24:03AM +0800, Xiao Ni wrote:
> > Now del_gendisk and put_disk are called asynchronously in workqueue work.
> > del_gendisk deletes device node by devtmpfs. devtmpfs tries to open this
> > array again and it flush the workqueue at the bigging of open process. So
> > a deadlock happens.
> >
> > The asynchronous way also 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 ioctl path and still leave put_disk in
> > md_kobj_release to avoid uaf.
>
> The md lifetime rules are complicated enough as-is. So while I won't
> object to this change per-see I'd rather have it reviewed by the md
> maintainers independently.
>
> In the meantime this should ensure devtmpfs doesn't call into
> blkdev_get_no_open and thus put_disk:
>
> diff --git a/block/bdev.c b/block/bdev.c
> index 6a34179192c9..97d4c0ab1670 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -1274,18 +1274,23 @@ void sync_bdevs(bool wait)
> */
> void bdev_statx(const struct path *path, struct kstat *stat, u32 request_mask)
> {
> - struct inode *backing_inode;
> struct block_device *bdev;
>
> - backing_inode = d_backing_inode(path->dentry);
> -
> /*
> - * Note that backing_inode is the inode of a block device node file,
> - * not the block device's internal inode. Therefore it is *not* valid
> - * to use I_BDEV() here; the block device has to be looked up by i_rdev
> - * instead.
> + * Note that d_backing_inode() returnsthe inode of a block device node
> + * file, not the block device's internal inode.
> + *
> + * Therefore it is *not* valid to use I_BDEV() here; the block device
> + * has to be looked up by i_rdev instead.
> + *
> + * Only do this lookup if actually needed to avoid the performance
> + * overhead of the lookup, and to avoid injecting bdev lifetime issues
> + * into devtmpfs.
> */
> - bdev = blkdev_get_no_open(backing_inode->i_rdev);
> + if (!(request_mask & (STATX_DIOALIGN | STATX_WRITE_ATOMIC)))
> + return;
> +
> + bdev = blkdev_get_no_open(d_backing_inode(path->dentry)->i_rdev);
> if (!bdev)
> return;
>
>
This patch resolves this deadlock problem.
Tested-by: Xiao Ni <xni@xxxxxxxxxx>
Regards
Xiao
