* [PATCH v2] ubi: block: Fix cleanup handling
@ 2023-06-07 9:25 Vincent Whitchurch
2023-06-10 7:01 ` Christoph Hellwig
0 siblings, 1 reply; 2+ messages in thread
From: Vincent Whitchurch @ 2023-06-07 9:25 UTC (permalink / raw)
To: Richard Weinberger, Miquel Raynal, Vignesh Raghavendra
Cc: hch, linux-mtd, linux-kernel, kernel, Vincent Whitchurch
ubiblock's remove handling has a couple of problems:
- It uses the gendisk after put_disk(), resulting in a use-after-free.
- There is a circular locking dependency between disk->open_mutex (used
from del_gendisk() and blkdev_open()) and dev->dev_mutex (used from
ubiblock_open() and ubiblock_remove()).
Fix these by implementing ->free_disk() and moving the final cleanup
there.
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
Changes in v2:
- Combine and rework patches to implement and use ->free_disk().
- Link to v1: https://lore.kernel.org/r/20230523-ubiblock-remove-v1-0-240bed75849b@axis.com
---
drivers/mtd/ubi/block.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 3711d7f74600..570e660673ad 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -293,11 +293,23 @@ static int ubiblock_getgeo(struct block_device *bdev, struct hd_geometry *geo)
return 0;
}
+static void ubiblock_free_disk(struct gendisk *disk)
+{
+ struct ubiblock *dev = disk->private_data;
+
+ mutex_lock(&devices_mutex);
+ idr_remove(&ubiblock_minor_idr, disk->first_minor);
+ mutex_unlock(&devices_mutex);
+
+ kfree(dev);
+}
+
static const struct block_device_operations ubiblock_ops = {
.owner = THIS_MODULE,
.open = ubiblock_open,
.release = ubiblock_release,
.getgeo = ubiblock_getgeo,
+ .free_disk = ubiblock_free_disk,
};
static blk_status_t ubiblock_queue_rq(struct blk_mq_hw_ctx *hctx,
@@ -452,9 +464,8 @@ static void ubiblock_cleanup(struct ubiblock *dev)
del_gendisk(dev->gd);
/* Finally destroy the blk queue */
dev_info(disk_to_dev(dev->gd), "released");
- put_disk(dev->gd);
blk_mq_free_tag_set(&dev->tag_set);
- idr_remove(&ubiblock_minor_idr, dev->gd->first_minor);
+ put_disk(dev->gd);
}
int ubiblock_remove(struct ubi_volume_info *vi)
@@ -478,11 +489,11 @@ int ubiblock_remove(struct ubi_volume_info *vi)
/* Remove from device list */
list_del(&dev->list);
- ubiblock_cleanup(dev);
mutex_unlock(&dev->dev_mutex);
mutex_unlock(&devices_mutex);
- kfree(dev);
+ ubiblock_cleanup(dev);
+
return 0;
out_unlock_dev:
@@ -623,17 +634,19 @@ static void ubiblock_remove_all(void)
{
struct ubiblock *next;
struct ubiblock *dev;
+ LIST_HEAD(list);
mutex_lock(&devices_mutex);
- list_for_each_entry_safe(dev, next, &ubiblock_devices, list) {
+ list_splice_init(&ubiblock_devices, &list);
+ mutex_unlock(&devices_mutex);
+
+ list_for_each_entry_safe(dev, next, &list, list) {
/* The module is being forcefully removed */
WARN_ON(dev->desc);
/* Remove from device list */
list_del(&dev->list);
ubiblock_cleanup(dev);
- kfree(dev);
}
- mutex_unlock(&devices_mutex);
}
int __init ubiblock_init(void)
---
base-commit: 44c026a73be8038f03dbdeef028b642880cf1511
change-id: 20230523-ubiblock-remove-eab61cf683f0
Best regards,
--
Vincent Whitchurch <vincent.whitchurch@axis.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] ubi: block: Fix cleanup handling
2023-06-07 9:25 [PATCH v2] ubi: block: Fix cleanup handling Vincent Whitchurch
@ 2023-06-10 7:01 ` Christoph Hellwig
0 siblings, 0 replies; 2+ messages in thread
From: Christoph Hellwig @ 2023-06-10 7:01 UTC (permalink / raw)
To: Vincent Whitchurch
Cc: Richard Weinberger, Miquel Raynal, Vignesh Raghavendra, hch,
linux-mtd, linux-kernel, kernel
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-06-10 7:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-07 9:25 [PATCH v2] ubi: block: Fix cleanup handling Vincent Whitchurch
2023-06-10 7:01 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).