Patch "mtd: blkdevs: fix potential deadlock + lockdep warnings" has been added to the 4.3-stable tree
Subsystems (2)
This is a note to let you know that I've just added the patch titled
mtd: blkdevs: fix potential deadlock + lockdep warnings
to the 4.3-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mtd-blkdevs-fix-potential-deadlock-lockdep-warnings.patch
and it can be found in the queue-4.3 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From
f3c63795e90f0c6238306883b6c72f14d5355721Mon Sep 17 00:00:00 2001
From: Brian Norris <[email protected]>
Date: Mon, 26 Oct 2015 10:20:23 -0700
Subject: mtd: blkdevs: fix potential deadlock + lockdep warnings
From: Brian Norris <[email protected]>
commit f3c63795e90f0c6238306883b6c72f14d5355721 upstream.
Commit 073db4a51ee4 ("mtd: fix: avoid race condition when accessing mtd->usecount") fixed a race condition but due to poor ordering of the mutex acquisition, introduced a potential deadlock.
The deadlock can occur, for example, when rmmod'ing the m25p80 module, which will delete one or more MTDs, along with any corresponding mtdblock devices. This could potentially race with an acquisition of the block device as follows.
-> blktrans_open()
-> mutex_lock(&dev->lock);
-> mutex_lock(&mtd_table_mutex); -> del_mtd_device()
-> mutex_lock(&mtd_table_mutex);
-> blktrans_notify_remove() -> del_mtd_blktrans_dev()
-> mutex_lock(&dev->lock);This is a classic (potential) ABBA deadlock, which can be fixed by making the A->B ordering consistent everywhere. There was no real purpose to the ordering in the original patch, AFAIR, so this shouldn't be a problem. This ordering was actually already present in del_mtd_blktrans_dev(), for one, where the function tried to ensure that its caller already held mtd_table_mutex before it acquired &dev->lock:
if (mutex_trylock(&mtd_table_mutex)) {
mutex_unlock(&mtd_table_mutex);
BUG();
}So, reverse the ordering of acquisition of &dev->lock and &mtd_table_mutex so we always acquire mtd_table_mutex first.
Snippets of the lockdep output follow:
# modprobe -r m25p80
[ 53.419251]
[ 53.420838] ======================================================
[ 53.427300] [ INFO: possible circular locking dependency detected ]
[ 53.433865] 4.3.0-rc6 #96 Not tainted
[ 53.437686] -------------------------------------------------------
[ 53.444220] modprobe/372 is trying to acquire lock:
[ 53.449320] (&new->lock){+.+...}, at: [<c043fe4c>] del_mtd_blktrans_dev+0x80/0xdc
[ 53.457271]
[ 53.457271] but task is already holding lock:
[ 53.463372] (mtd_table_mutex){+.+.+.}, at: [<c0439994>] del_mtd_device+0x18/0x100
[ 53.471321]
[ 53.471321] which lock already depends on the new lock.
[ 53.471321]
[ 53.479856]
[ 53.479856] the existing dependency chain (in reverse order) is:
[ 53.487660]
-> #1 (mtd_table_mutex){+.+.+.}:
[ 53.492331] [<c043fc5c>] blktrans_open+0x34/0x1a4
[ 53.497879] [<c01afce0>] __blkdev_get+0xc4/0x3b0
[ 53.503364] [<c01b0bb8>] blkdev_get+0x108/0x320
[ 53.508743] [<c01713c0>] do_dentry_open+0x218/0x314
[ 53.514496] [<c0180454>] path_openat+0x4c0/0xf9c
[ 53.519959] [<c0182044>] do_filp_open+0x5c/0xc0
[ 53.525336] [<c0172758>] do_sys_open+0xfc/0x1cc
[ 53.530716] [<c000f740>] ret_fast_syscall+0x0/0x1c
[ 53.536375]
-> #0 (&new->lock){+.+...}:
[ 53.540587] [<c063f124>] mutex_lock_nested+0x38/0x3cc
[ 53.546504] [<c043fe4c>] del_mtd_blktrans_dev+0x80/0xdc
[ 53.552606] [<c043f164>] blktrans_notify_remove+0x7c/0x84
[ 53.558891] [<c04399f0>] del_mtd_device+0x74/0x100
[ 53.564544] [<c043c670>] del_mtd_partitions+0x80/0xc8
[ 53.570451] [<c0439aa0>] mtd_device_unregister+0x24/0x48
[ 53.576637] [<c046ce6c>] spi_drv_remove+0x1c/0x34
[ 53.582207] [<c03de0f0>] __device_release_driver+0x88/0x114
[ 53.588663] [<c03de19c>] device_release_driver+0x20/0x2c
[ 53.594843] [<c03dd9e8>] bus_remove_device+0xd8/0x108
[ 53.600748] [<c03dacc0>] device_del+0x10c/0x210
[ 53.606127] [<c03dadd0>] device_unregister+0xc/0x20
[ 53.611849] [<c046d878>] __unregister+0x10/0x20
[ 53.617211] [<c03da868>] device_for_each_child+0x50/0x7c
[ 53.623387] [<c046eae8>] spi_unregister_master+0x58/0x8c
[ 53.629578] [<c03e12f0>] release_nodes+0x15c/0x1c8
[ 53.635223] [<c03de0f8>] __device_release_driver+0x90/0x114
[ 53.641689] [<c03de900>] driver_detach+0xb4/0xb8
[ 53.647147] [<c03ddc78>] bus_remove_driver+0x4c/0xa0
[ 53.652970] [<c00cab50>] SyS_delete_module+0x11c/0x1e4
[ 53.658976] [<c000f740>] ret_fast_syscall+0x0/0x1c
[ 53.664621]
[ 53.664621] other info that might help us debug this:
[ 53.664621]
[ 53.672979] Possible unsafe locking scenario:
[ 53.672979]
[ 53.679169] CPU0 CPU1
[ 53.683900] ---- ----
[ 53.688633] lock(mtd_table_mutex);
[ 53.692383] lock(&new->lock);
[ 53.698306] lock(mtd_table_mutex);
[ 53.704658] lock(&new->lock);
[ 53.707946]
[ 53.707946] *** DEADLOCK ***--- drivers/mtd/mtd_blkdevs.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mtd/mtd_blkdevs.c +5 -5
@@ -192,8 +192,8 @@ static int blktrans_open(struct block_deif (!dev)return -ERESTARTSYS ;/* FIXME: busy loop! -arnd*/ -mutex_lock (&dev->lock);mutex_lock (&mtd_table_mutex); +mutex_lock (&dev->lock);if (dev->open) goto unlock;
@@ -217,8 +217,8 @@ static int blktrans_open(struct block_de unlock: dev->open++; -mutex_unlock (&mtd_table_mutex);mutex_unlock (&dev->lock); +mutex_unlock (&mtd_table_mutex);blktrans_dev_put (dev);return ret;
@@ -228,8 +228,8 @@ 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); +mutex_unlock (&mtd_table_mutex);blktrans_dev_put (dev);return ret; }
@@ -241,8 +241,8 @@ static void blktrans_release(struct gendif (!dev)return ; -mutex_lock (&dev->lock);mutex_lock (&mtd_table_mutex); +mutex_lock (&dev->lock);if (--dev->open) goto unlock;
@@ -256,8 +256,8 @@ static void blktrans_release(struct gend__put_mtd_device (dev->mtd); } unlock: -mutex_unlock (&mtd_table_mutex);mutex_unlock (&dev->lock); +mutex_unlock (&mtd_table_mutex);blktrans_dev_put (dev); }
Patches currently in stable-queue which might be from [email protected] are
queue-4.3/mtd-mtdpart-fix-add_mtd_partitions-error-path.patch queue-4.3/mtd-blkdevs-fix-potential-deadlock-lockdep-warnings.patch queue-4.3/mtd-nand-fix-shutdown-reboot-for-multi-chip-systems.patch queue-4.3/mtd-jz4740_nand-fix-build-on-jz4740-after-removing-gpio.h.patch