Hi,
在 2022/11/02 14:48, Christoph Hellwig 写道:
For gendisk that are not live or their partitions, the bd_holder_dir
pointer is not valid and the kobject might not have been allocated
yet or freed already. Check that the disk is live before creating the
linkage and error out otherwise.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
block/holder.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/block/holder.c b/block/holder.c
index a8c355b9d0806..a8806bbed2112 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -66,6 +66,11 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
return -EINVAL;
mutex_lock(&disk->open_mutex);
+ /* bd_holder_dir is only valid for live disks */
+ if (!disk_live(bdev->bd_disk)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
I think this is still not safe 🤔
How about this:
diff --git a/block/holder.c b/block/holder.c
index 5283bc804cc1..35fdfba558b8 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -85,6 +85,20 @@ int bd_link_disk_holder(struct block_device *bdev,
struct gendisk *disk)
goto out_unlock;
}
+ /*
+ * del_gendisk drops the initial reference to bd_holder_dir, so
we need
+ * to keep our own here to allow for cleanup past that point.
+ */
+ mutex_lock(&bdev->bd_disk->open_mutex);
+ if (!disk_live(bdev->bd_disk)) {
+ ret = -ENODEV;
+ mutex_unlock(&bdev->bd_disk->open_mutex);
+ goto out_unlock;
+ }
+
+ kobject_get(bdev->bd_holder_dir);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
+
holder = kzalloc(sizeof(*holder), GFP_KERNEL);
if (!holder) {
ret = -ENOMEM;
@@ -103,11 +117,6 @@ int bd_link_disk_holder(struct block_device *bdev,
struct gendisk *disk)
}
list_add(&holder->list, &disk->slave_bdevs);
- /*
- * del_gendisk drops the initial reference to bd_holder_dir, so
we need
- * to keep our own here to allow for cleanup past that point.
- */
- kobject_get(bdev->bd_holder_dir);
bdev->bd_disk->open_mutex should be hold, both dis_live() and
kobject_get() should be protected.
Thansk,
Kuai
WARN_ON_ONCE(!bdev->bd_holder);
