Re: [PATCH] mtd_blkdevs: add mtd_table_mutex lock back to blktrans_{open, release} to avoid race condition

From: ChristophHellwig
Date: Fri Sep 09 2022 - 10:36:36 EST


Can you try this patch (it'll need to be split up into a few if it
works):

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 60b222799871e..9eda1dd98a406 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -24,24 +24,16 @@

static LIST_HEAD(blktrans_majors);

-static void blktrans_dev_release(struct kref *kref)
+static void blktrans_free_disk(struct gendisk *disk)
{
- struct mtd_blktrans_dev *dev =
- container_of(kref, struct mtd_blktrans_dev, ref);
+ struct mtd_blktrans_dev *dev = disk->private_data;

- put_disk(dev->disk);
blk_mq_free_tag_set(dev->tag_set);
kfree(dev->tag_set);
list_del(&dev->list);
kfree(dev);
}

-static void blktrans_dev_put(struct mtd_blktrans_dev *dev)
-{
- kref_put(&dev->ref, blktrans_dev_release);
-}
-
-
static blk_status_t do_blktrans_request(struct mtd_blktrans_ops *tr,
struct mtd_blktrans_dev *dev,
struct request *req)
@@ -187,63 +179,58 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
int ret = 0;

- kref_get(&dev->ref);
+ if (disk_openers(bdev->bd_disk) > 0)
+ return 0;

- mutex_lock(&dev->lock);
-
- if (dev->open)
- goto unlock;
+ mutex_lock(&mtd_table_mutex);
+ if (!dev->mtd) {
+ mutex_lock(&mtd_table_mutex);
+ return -EINVAL;
+ }
+ ret = __get_mtd_device(dev->mtd);
+ mutex_unlock(&mtd_table_mutex);
+ if (ret)
+ return ret;

+ mutex_lock(&dev->lock);
__module_get(dev->tr->owner);
-
- if (!dev->mtd)
- goto unlock;
-
if (dev->tr->open) {
ret = dev->tr->open(dev);
if (ret)
goto error_put;
}
-
- ret = __get_mtd_device(dev->mtd);
- if (ret)
- goto error_release;
dev->file_mode = mode;
-
-unlock:
dev->open++;
mutex_unlock(&dev->lock);
- return ret;

-error_release:
- if (dev->tr->release)
- dev->tr->release(dev);
+ return 0;
+
error_put:
module_put(dev->tr->owner);
mutex_unlock(&dev->lock);
- blktrans_dev_put(dev);
+
+ put_mtd_device(dev->mtd);
return ret;
}

static void blktrans_release(struct gendisk *disk, fmode_t mode)
{
struct mtd_blktrans_dev *dev = disk->private_data;
+ struct mtd_info *mtd = NULL;

- mutex_lock(&dev->lock);
-
- if (--dev->open)
- goto unlock;
+ if (disk_openers(disk) > 0)
+ return;

+ mutex_lock(&dev->lock);
+ dev->open--;
module_put(dev->tr->owner);
-
- if (dev->mtd) {
- if (dev->tr->release)
- dev->tr->release(dev);
- __put_mtd_device(dev->mtd);
- }
-unlock:
+ mtd = dev->mtd;
+ if (mtd && dev->tr->release)
+ dev->tr->release(dev);
mutex_unlock(&dev->lock);
- blktrans_dev_put(dev);
+
+ if (mtd)
+ put_mtd_device(dev->mtd);
}

static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
@@ -266,6 +253,7 @@ static const struct block_device_operations mtd_block_ops = {
.owner = THIS_MODULE,
.open = blktrans_open,
.release = blktrans_release,
+ .free_disk = blktrans_free_disk,
.getgeo = blktrans_getgeo,
};

@@ -318,7 +306,6 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
added:

mutex_init(&new->lock);
- kref_init(&new->ref);
if (!tr->writesect)
new->readonly = 1;

@@ -410,6 +397,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)

int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
{
+ struct mtd_info *old_mtd = NULL;
unsigned long flags;

lockdep_assert_held(&mtd_table_mutex);
@@ -438,13 +426,14 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
if (old->open) {
if (old->tr->release)
old->tr->release(old);
- __put_mtd_device(old->mtd);
+ old_mtd = old->mtd;
}
-
old->mtd = NULL;
-
mutex_unlock(&old->lock);
- blktrans_dev_put(old);
+ put_disk(old->disk);
+
+ if (old->mtd)
+ put_mtd_device(old_mtd);
return 0;
}

diff --git a/include/linux/mtd/blktrans.h b/include/linux/mtd/blktrans.h
index 15cc9b95e32b5..41a81fc9f0462 100644
--- a/include/linux/mtd/blktrans.h
+++ b/include/linux/mtd/blktrans.h
@@ -7,7 +7,6 @@
#define __MTD_TRANS_H__

#include <linux/mutex.h>
-#include <linux/kref.h>
#include <linux/sysfs.h>

struct hd_geometry;
@@ -26,7 +25,6 @@ struct mtd_blktrans_dev {
unsigned long size;
int readonly;
int open;
- struct kref ref;
struct gendisk *disk;
struct attribute_group *disk_attributes;
struct request_queue *rq;