From: Christoph Hellwig <hch@lst.de>
To: Xiao Ni <xni@redhat.com>
Cc: linux-raid@vger.kernel.org, song@kernel.org,
yukuai1@huaweicloud.com, hch@lst.de, ming.lei@redhat.com,
ncroxon@redhat.com
Subject: Re: [RFC PATCH 1/1] md: delete gendisk in ioctl path
Date: Tue, 22 Apr 2025 07:51:08 +0200 [thread overview]
Message-ID: <20250422055108.GA29356@lst.de> (raw)
In-Reply-To: <20250422032403.63057-1-xni@redhat.com>
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;
next prev parent reply other threads:[~2025-04-22 5:51 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-22 3:24 [RFC PATCH 1/1] md: delete gendisk in ioctl path Xiao Ni
2025-04-22 5:51 ` Christoph Hellwig [this message]
2025-04-22 8:31 ` Xiao Ni
2025-04-22 8:40 ` Xiao Ni
2025-04-22 13:48 ` Christoph Hellwig
2025-04-22 9:24 ` Xiao Ni
2025-04-22 6:21 ` Yu Kuai
2025-04-22 7:31 ` Xiao Ni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250422055108.GA29356@lst.de \
--to=hch@lst.de \
--cc=linux-raid@vger.kernel.org \
--cc=ming.lei@redhat.com \
--cc=ncroxon@redhat.com \
--cc=song@kernel.org \
--cc=xni@redhat.com \
--cc=yukuai1@huaweicloud.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.