From 073db4a51ee43ccb827f54a4261c0583b028d5ab Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Thu, 7 May 2015 17:55:16 -0700 Subject: mtd: fix: avoid race condition when accessing mtd->usecount On A MIPS 32-cores machine a BUG_ON was triggered because some acesses to mtd->usecount were done without taking mtd_table_mutex. kernel: Call Trace: kernel: [] __put_mtd_device+0x20/0x50 kernel: [] blktrans_release+0x8c/0xd8 kernel: [] __blkdev_put+0x1a8/0x200 kernel: [] blkdev_close+0x1c/0x30 kernel: [] __fput+0xac/0x250 kernel: [] task_work_run+0xd8/0x120 kernel: [] work_notifysig+0x10/0x18 kernel: kernel: Code: 2442ffff ac8202d8 000217fe <00020336> dc820128 10400003 00000000 0040f809 00000000 kernel: ---[ end trace 080fbb4579b47a73 ]--- Fixed by taking the mutex in blktrans_open and blktrans_release. Note that this locking is already suggested in include/linux/mtd/blktrans.h: struct mtd_blktrans_ops { ... /* Called with mtd_table_mutex held; no race with add/remove */ int (*open)(struct mtd_blktrans_dev *dev); void (*release)(struct mtd_blktrans_dev *dev); ... }; But we weren't following it. Originally reported by (and patched by) Zhang and Giuseppe, independently. Improved and rewritten. Cc: stable@vger.kernel.org Reported-by: Zhang Xingcai Reported-by: Giuseppe Cantavenera Tested-by: Giuseppe Cantavenera Acked-by: Alexander Sverdlin Signed-off-by: Brian Norris --- drivers/mtd/mtd_blkdevs.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 2b0c52870999..df7c6c70757a 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -197,6 +197,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode) return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/ mutex_lock(&dev->lock); + mutex_lock(&mtd_table_mutex); if (dev->open) goto unlock; @@ -220,6 +221,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode) unlock: dev->open++; + mutex_unlock(&mtd_table_mutex); mutex_unlock(&dev->lock); blktrans_dev_put(dev); return ret; @@ -230,6 +232,7 @@ error_release: error_put: module_put(dev->tr->owner); kref_put(&dev->ref, blktrans_dev_release); + mutex_unlock(&mtd_table_mutex); mutex_unlock(&dev->lock); blktrans_dev_put(dev); return ret; @@ -243,6 +246,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode) return; mutex_lock(&dev->lock); + mutex_lock(&mtd_table_mutex); if (--dev->open) goto unlock; @@ -256,6 +260,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode) __put_mtd_device(dev->mtd); } unlock: + mutex_unlock(&mtd_table_mutex); mutex_unlock(&dev->lock); blktrans_dev_put(dev); } -- cgit v1.2.3 From 50183936254b76997d222cd36bac997ebf8115de Mon Sep 17 00:00:00 2001 From: Wenlin Kang Date: Thu, 21 May 2015 14:49:38 +0800 Subject: mtd: blktrans: change blktrans_getgeo return value Modify function blktrans_getgeo()'s return value to -EOPNOTSUPP when dev->tr->getgeo == NULL. We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, because the function blktrans_getgeo() has an output value "hd_geometry" which is usually used by some application, if returns 0 (i.e., "success"), it will make some application get the wrong information. Signed-off-by: Wenlin Kang Signed-off-by: Brian Norris --- drivers/mtd/mtd_blkdevs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index df7c6c70757a..c545c9325acf 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -278,7 +278,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) if (!dev->mtd) goto unlock; - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -EOPNOTSUPP; unlock: mutex_unlock(&dev->lock); blktrans_dev_put(dev); -- cgit v1.2.3 From c4a3f13c2ab997617eaf7020af33b057a68285fd Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Thu, 21 May 2015 10:44:32 -0700 Subject: mtd: blktrans: use better error code for unimplemented ioctl() In commit 50183936254b ("mtd: blktrans: change blktrans_getgeo return value") we fixed the problem that ioctl(HDIO_GETGEO) might return 0 (success) for mtdblock devices which did not implement the feature and would leave a blank (zero) result. But now, let's get the error code right. Other code paths on this ioctl tend to use -ENOTTY to notify the user that the ioctl() is not supported for the device, so let's use that instead of -EOPNOTSUPP. Suggested-by: Richard Weinberger Signed-off-by: Brian Norris --- drivers/mtd/mtd_blkdevs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/mtd/mtd_blkdevs.c') diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index c545c9325acf..41acc507b22e 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -278,7 +278,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) if (!dev->mtd) goto unlock; - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -EOPNOTSUPP; + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENOTTY; unlock: mutex_unlock(&dev->lock); blktrans_dev_put(dev); -- cgit v1.2.3