<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux-stable.git/drivers/mtd, branch v3.2.99</title>
<subtitle>Linux kernel stable tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/'/>
<entry>
<title>mtd: nand: Fix writing mtdoops to nand flash.</title>
<updated>2018-02-13T18:32:12+00:00</updated>
<author>
<name>Brent Taylor</name>
<email>motobud@gmail.com</email>
</author>
<published>2017-10-31T03:32:45+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=3c467333654f621ad61701193c8b2340da52de5b'/>
<id>3c467333654f621ad61701193c8b2340da52de5b</id>
<content type='text'>
commit 30863e38ebeb500a31cecee8096fb5002677dd9b upstream.

When mtdoops calls mtd_panic_write(), it eventually calls
panic_nand_write() in nand_base.c. In order to properly wait for the
nand chip to be ready in panic_nand_wait(), the chip must first be
selected.

When using the atmel nand flash controller, a panic would occur due to
a NULL pointer exception.

Fixes: 2af7c6539931 ("mtd: Add panic_write for NAND flashes")
Signed-off-by: Brent Taylor &lt;motobud@gmail.com&gt;
Signed-off-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 30863e38ebeb500a31cecee8096fb5002677dd9b upstream.

When mtdoops calls mtd_panic_write(), it eventually calls
panic_nand_write() in nand_base.c. In order to properly wait for the
nand chip to be ready in panic_nand_wait(), the chip must first be
selected.

When using the atmel nand flash controller, a panic would occur due to
a NULL pointer exception.

Fixes: 2af7c6539931 ("mtd: Add panic_write for NAND flashes")
Signed-off-by: Brent Taylor &lt;motobud@gmail.com&gt;
Signed-off-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>mtd: sst25l: kill unused variable</title>
<updated>2017-11-11T13:34:46+00:00</updated>
<author>
<name>Artem Bityutskiy</name>
<email>artem.bityutskiy@linux.intel.com</email>
</author>
<published>2011-12-29T16:06:01+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=bb754dd4e818782b0ffa5ab775300882ff54f34f'/>
<id>bb754dd4e818782b0ffa5ab775300882ff54f34f</id>
<content type='text'>
commit d81a32f2c16a3c42cf26f2216765c520630daa4e upstream.

Fix the following gcc warning:
drivers/mtd/devices/sst25l.c: In function ‘sst25l_probe’:
drivers/mtd/devices/sst25l.c:381:11: warning: unused variable ‘i’ [-Wunused-variable]

Signed-off-by: Artem Bityutskiy &lt;artem.bityutskiy@linux.intel.com&gt;
Signed-off-by: David Woodhouse &lt;David.Woodhouse@intel.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit d81a32f2c16a3c42cf26f2216765c520630daa4e upstream.

Fix the following gcc warning:
drivers/mtd/devices/sst25l.c: In function ‘sst25l_probe’:
drivers/mtd/devices/sst25l.c:381:11: warning: unused variable ‘i’ [-Wunused-variable]

Signed-off-by: Artem Bityutskiy &lt;artem.bityutskiy@linux.intel.com&gt;
Signed-off-by: David Woodhouse &lt;David.Woodhouse@intel.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ubi/upd: Always flush after prepared for an update</title>
<updated>2017-07-18T17:38:38+00:00</updated>
<author>
<name>Sebastian Siewior</name>
<email>bigeasy@linutronix.de</email>
</author>
<published>2017-02-22T16:15:21+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=b6d248935d96475c987b32c857d3d890a97c2e74'/>
<id>b6d248935d96475c987b32c857d3d890a97c2e74</id>
<content type='text'>
commit 9cd9a21ce070be8a918ffd3381468315a7a76ba6 upstream.

In commit 6afaf8a484cb ("UBI: flush wl before clearing update marker") I
managed to trigger and fix a similar bug. Now here is another version of
which I assumed it wouldn't matter back then but it turns out UBI has a
check for it and will error out like this:

|ubi0 warning: validate_vid_hdr: inconsistent used_ebs
|ubi0 error: validate_vid_hdr: inconsistent VID header at PEB 592

All you need to trigger this is? "ubiupdatevol /dev/ubi0_0 file" + a
powercut in the middle of the operation.
ubi_start_update() sets the update-marker and puts all EBs on the erase
list. After that userland can proceed to write new data while the old EB
aren't erased completely. A powercut at this point is usually not that
much of a tragedy. UBI won't give read access to the static volume
because it has the update marker. It will most likely set the corrupted
flag because it misses some EBs.
So we are all good. Unless the size of the image that has been written
differs from the old image in the magnitude of at least one EB. In that
case UBI will find two different values for `used_ebs' and refuse to
attach the image with the error message mentioned above.

So in order not to get in the situation, the patch will ensure that we
wait until everything is removed before it tries to write any data.
The alternative would be to detect such a case and remove all EBs at the
attached time after we processed the volume-table and see the
update-marker set. The patch looks bigger and I doubt it is worth it
since usually the write() will wait from time to time for a new EB since
usually there not that many spare EB that can be used.

Signed-off-by: Sebastian Andrzej Siewior &lt;bigeasy@linutronix.de&gt;
Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
[bwh: Backported to 3.2: ubi_wl_flush() only takes 1 parameter]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 9cd9a21ce070be8a918ffd3381468315a7a76ba6 upstream.

In commit 6afaf8a484cb ("UBI: flush wl before clearing update marker") I
managed to trigger and fix a similar bug. Now here is another version of
which I assumed it wouldn't matter back then but it turns out UBI has a
check for it and will error out like this:

|ubi0 warning: validate_vid_hdr: inconsistent used_ebs
|ubi0 error: validate_vid_hdr: inconsistent VID header at PEB 592

All you need to trigger this is? "ubiupdatevol /dev/ubi0_0 file" + a
powercut in the middle of the operation.
ubi_start_update() sets the update-marker and puts all EBs on the erase
list. After that userland can proceed to write new data while the old EB
aren't erased completely. A powercut at this point is usually not that
much of a tragedy. UBI won't give read access to the static volume
because it has the update marker. It will most likely set the corrupted
flag because it misses some EBs.
So we are all good. Unless the size of the image that has been written
differs from the old image in the magnitude of at least one EB. In that
case UBI will find two different values for `used_ebs' and refuse to
attach the image with the error message mentioned above.

So in order not to get in the situation, the patch will ensure that we
wait until everything is removed before it tries to write any data.
The alternative would be to detect such a case and remove all EBs at the
attached time after we processed the volume-table and see the
update-marker set. The patch looks bigger and I doubt it is worth it
since usually the write() will wait from time to time for a new EB since
usually there not that many spare EB that can be used.

Signed-off-by: Sebastian Andrzej Siewior &lt;bigeasy@linutronix.de&gt;
Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
[bwh: Backported to 3.2: ubi_wl_flush() only takes 1 parameter]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>mtd: nand: davinci: Reinitialize the HW ECC engine in 4bit hwctl</title>
<updated>2016-11-20T01:01:37+00:00</updated>
<author>
<name>Karl Beldan</name>
<email>kbeldan@baylibre.com</email>
</author>
<published>2016-08-29T07:45:49+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=acf46003d6c037bdfff3025c38c412652801e708'/>
<id>acf46003d6c037bdfff3025c38c412652801e708</id>
<content type='text'>
commit f6d7c1b5598b6407c3f1da795dd54acf99c1990c upstream.

This fixes subpage writes when using 4-bit HW ECC.

There has been numerous reports about ECC errors with devices using this
driver for a while.  Also the 4-bit ECC has been reported as broken with
subpages in [1] and with 16 bits NANDs in the driver and in mach* board
files both in mainline and in the vendor BSPs.

What I saw with 4-bit ECC on a 16bits NAND (on an LCDK) which got me to
try reinitializing the ECC engine:
- R/W on whole pages properly generates/checks RS code
- try writing the 1st subpage only of a blank page, the subpage is well
  written and the RS code properly generated, re-reading the same page
  the HW detects some ECC error, reading the same page again no ECC
  error is detected

Note that the ECC engine is already reinitialized in the 1-bit case.

Tested on my LCDK with UBI+UBIFS using subpages.
This could potentially get rid of the issue workarounded in [1].

[1] 28c015a9daab ("mtd: davinci-nand: disable subpage write for keystone-nand")

Fixes: 6a4123e581b3 ("mtd: nand: davinci_nand, 4-bit ECC for smallpage")
Signed-off-by: Karl Beldan &lt;kbeldan@baylibre.com&gt;
Acked-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit f6d7c1b5598b6407c3f1da795dd54acf99c1990c upstream.

This fixes subpage writes when using 4-bit HW ECC.

There has been numerous reports about ECC errors with devices using this
driver for a while.  Also the 4-bit ECC has been reported as broken with
subpages in [1] and with 16 bits NANDs in the driver and in mach* board
files both in mainline and in the vendor BSPs.

What I saw with 4-bit ECC on a 16bits NAND (on an LCDK) which got me to
try reinitializing the ECC engine:
- R/W on whole pages properly generates/checks RS code
- try writing the 1st subpage only of a blank page, the subpage is well
  written and the RS code properly generated, re-reading the same page
  the HW detects some ECC error, reading the same page again no ECC
  error is detected

Note that the ECC engine is already reinitialized in the 1-bit case.

Tested on my LCDK with UBI+UBIFS using subpages.
This could potentially get rid of the issue workarounded in [1].

[1] 28c015a9daab ("mtd: davinci-nand: disable subpage write for keystone-nand")

Fixes: 6a4123e581b3 ("mtd: nand: davinci_nand, 4-bit ECC for smallpage")
Signed-off-by: Karl Beldan &lt;kbeldan@baylibre.com&gt;
Acked-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ubi: Fix race condition between ubi device creation and udev</title>
<updated>2016-11-20T01:01:29+00:00</updated>
<author>
<name>Iosif Harutyunov</name>
<email>iharutyunov@SonicWALL.com</email>
</author>
<published>2016-07-22T23:22:42+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=016820bde3f0895d09fcad370415085ba0d1bd4a'/>
<id>016820bde3f0895d09fcad370415085ba0d1bd4a</id>
<content type='text'>
commit 714fb87e8bc05ff78255afc0dca981e8c5242785 upstream.

Install the UBI device object before we arm sysfs.
Otherwise udev tries to read sysfs attributes before UBI is ready and
udev rules will not match.

Signed-off-by: Iosif Harutyunov &lt;iharutyunov@sonicwall.com&gt;
[rw: massaged commit message]
Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 714fb87e8bc05ff78255afc0dca981e8c5242785 upstream.

Install the UBI device object before we arm sysfs.
Otherwise udev tries to read sysfs attributes before UBI is ready and
udev rules will not match.

Signed-off-by: Iosif Harutyunov &lt;iharutyunov@sonicwall.com&gt;
[rw: massaged commit message]
Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>mtd: nand: fix bug writing 1 byte less than page size</title>
<updated>2016-11-20T01:01:27+00:00</updated>
<author>
<name>Hector Palacios</name>
<email>hector.palacios@digi.com</email>
</author>
<published>2016-07-18T08:39:18+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=e96c4b7702a0f501ba7529bddfc1ef4429369fa9'/>
<id>e96c4b7702a0f501ba7529bddfc1ef4429369fa9</id>
<content type='text'>
commit 144f4c98399e2c0ca60eb414c15a2c68125c18b8 upstream.

nand_do_write_ops() determines if it is writing a partial page with the
formula:
	part_pagewr = (column || writelen &lt; (mtd-&gt;writesize - 1))

When 'writelen' is exactly 1 byte less than the NAND page size the formula
equates to zero, so the code doesn't process it as a partial write,
although it should.
As a consequence the function remains in the while(1) loop with 'writelen'
becoming 0xffffffff and iterating endlessly.

The bug may not be easy to reproduce in Linux since user space tools
usually force the padding or round-up the write size to a page-size
multiple.
This was discovered in U-Boot where the issue can be reproduced by
writing any size that is 1 byte less than a page-size multiple.
For example, on a NAND with 2K page (0x800):
	=&gt; nand erase.part &lt;partition&gt;
	=&gt; nand write $loadaddr &lt;partition&gt; 7ff

[Editor's note: the bug was added in commit 29072b96078f, but moved
around in commit 66507c7bc8895 ("mtd: nand: Add support to use nand_base
poi databuf as bounce buffer")]

Fixes: 29072b96078f ("[MTD] NAND: add subpage write support")
Signed-off-by: Hector Palacios &lt;hector.palacios@digi.com&gt;
Acked-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
[bwh: Backported to 3.2: adjusted context as noted above]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 144f4c98399e2c0ca60eb414c15a2c68125c18b8 upstream.

nand_do_write_ops() determines if it is writing a partial page with the
formula:
	part_pagewr = (column || writelen &lt; (mtd-&gt;writesize - 1))

When 'writelen' is exactly 1 byte less than the NAND page size the formula
equates to zero, so the code doesn't process it as a partial write,
although it should.
As a consequence the function remains in the while(1) loop with 'writelen'
becoming 0xffffffff and iterating endlessly.

The bug may not be easy to reproduce in Linux since user space tools
usually force the padding or round-up the write size to a page-size
multiple.
This was discovered in U-Boot where the issue can be reproduced by
writing any size that is 1 byte less than a page-size multiple.
For example, on a NAND with 2K page (0x800):
	=&gt; nand erase.part &lt;partition&gt;
	=&gt; nand write $loadaddr &lt;partition&gt; 7ff

[Editor's note: the bug was added in commit 29072b96078f, but moved
around in commit 66507c7bc8895 ("mtd: nand: Add support to use nand_base
poi databuf as bounce buffer")]

Fixes: 29072b96078f ("[MTD] NAND: add subpage write support")
Signed-off-by: Hector Palacios &lt;hector.palacios@digi.com&gt;
Acked-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
[bwh: Backported to 3.2: adjusted context as noted above]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>mtd: pmcmsp-flash: Allocating too much in init_msp_flash()</title>
<updated>2016-11-20T01:01:26+00:00</updated>
<author>
<name>Dan Carpenter</name>
<email>dan.carpenter@oracle.com</email>
</author>
<published>2016-07-14T10:44:56+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=d92e64dae0457f6a406fbabc200734a11ae06e66'/>
<id>d92e64dae0457f6a406fbabc200734a11ae06e66</id>
<content type='text'>
commit 79ad07d45743721010e766e65dc004ad249bd429 upstream.

There is a cut and paste issue here.  The bug is that we are allocating
more memory than necessary for msp_maps.  We should be allocating enough
space for a map_info struct (144 bytes) but we instead allocate enough
for an mtd_info struct (1840 bytes).  It's a small waste.

The other part of this is not harmful but when we allocated msp_flash
then we allocated enough space fro a map_info pointer instead of an
mtd_info pointer.  But since pointers are the same size it works out
fine.

Anyway, I decided to clean up all three allocations a bit to make them
a bit more consistent and clear.

Fixes: 68aa0fa87f6d ('[MTD] PMC MSP71xx flash/rootfs mappings')
Signed-off-by: Dan Carpenter &lt;dan.carpenter@oracle.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 79ad07d45743721010e766e65dc004ad249bd429 upstream.

There is a cut and paste issue here.  The bug is that we are allocating
more memory than necessary for msp_maps.  We should be allocating enough
space for a map_info struct (144 bytes) but we instead allocate enough
for an mtd_info struct (1840 bytes).  It's a small waste.

The other part of this is not harmful but when we allocated msp_flash
then we allocated enough space fro a map_info pointer instead of an
mtd_info pointer.  But since pointers are the same size it works out
fine.

Anyway, I decided to clean up all three allocations a bit to make them
a bit more consistent and clear.

Fixes: 68aa0fa87f6d ('[MTD] PMC MSP71xx flash/rootfs mappings')
Signed-off-by: Dan Carpenter &lt;dan.carpenter@oracle.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ubi: Make recover_peb power cut aware</title>
<updated>2016-08-22T21:37:15+00:00</updated>
<author>
<name>Richard Weinberger</name>
<email>richard@nod.at</email>
</author>
<published>2016-06-20T22:31:50+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=bedbc047e74d0962e194eb0018e9e1dbd7b9402f'/>
<id>bedbc047e74d0962e194eb0018e9e1dbd7b9402f</id>
<content type='text'>
commit 972228d87445dc46c0a01f5f3de673ac017626f7 upstream.

recover_peb() was never power cut aware,
if a power cut happened right after writing the VID header
upon next attach UBI would blindly use the new partial written
PEB and all data from the old PEB is lost.

In order to make recover_peb() power cut aware, write the new
VID with a proper crc and copy_flag set such that the UBI attach
process will detect whether the new PEB is completely written
or not.
We cannot directly use ubi_eba_atomic_leb_change() since we'd
have to unlock the LEB which is facing a write error.

Reported-by: Jörg Pfähler &lt;pfaehler@isse.de&gt;
Reviewed-by: Jörg Pfähler &lt;pfaehler@isse.de&gt;
Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
[bwh: Backported to 3.2:
 - Adjust context
 - Use next_sqnum() instead of ubi_next_sqnum()
 - Use ubi_device::peb_buf1 instead of ubi_device::peb_buf
 - No need to unlock ubi-&gt;fm_eba_sem on error]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit 972228d87445dc46c0a01f5f3de673ac017626f7 upstream.

recover_peb() was never power cut aware,
if a power cut happened right after writing the VID header
upon next attach UBI would blindly use the new partial written
PEB and all data from the old PEB is lost.

In order to make recover_peb() power cut aware, write the new
VID with a proper crc and copy_flag set such that the UBI attach
process will detect whether the new PEB is completely written
or not.
We cannot directly use ubi_eba_atomic_leb_change() since we'd
have to unlock the LEB which is facing a write error.

Reported-by: Jörg Pfähler &lt;pfaehler@isse.de&gt;
Reviewed-by: Jörg Pfähler &lt;pfaehler@isse.de&gt;
Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
[bwh: Backported to 3.2:
 - Adjust context
 - Use next_sqnum() instead of ubi_next_sqnum()
 - Use ubi_device::peb_buf1 instead of ubi_device::peb_buf
 - No need to unlock ubi-&gt;fm_eba_sem on error]
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ubi: Fix out of bounds write in volume update code</title>
<updated>2016-04-01T00:54:37+00:00</updated>
<author>
<name>Richard Weinberger</name>
<email>richard@nod.at</email>
</author>
<published>2016-02-21T09:53:03+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=7aae79649d6a88e7104d4d22aa4ba21f4e699e40'/>
<id>7aae79649d6a88e7104d4d22aa4ba21f4e699e40</id>
<content type='text'>
commit e4f6daac20332448529b11f09388f1d55ef2084c upstream.

ubi_start_leb_change() allocates too few bytes.
ubi_more_leb_change_data() will write up to req-&gt;upd_bytes +
ubi-&gt;min_io_size bytes.

Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
Reviewed-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit e4f6daac20332448529b11f09388f1d55ef2084c upstream.

ubi_start_leb_change() allocates too few bytes.
ubi_more_leb_change_data() will write up to req-&gt;upd_bytes +
ubi-&gt;min_io_size bytes.

Signed-off-by: Richard Weinberger &lt;richard@nod.at&gt;
Reviewed-by: Boris Brezillon &lt;boris.brezillon@free-electrons.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>mtd: blkdevs: fix potential deadlock + lockdep warnings</title>
<updated>2015-11-27T12:48:22+00:00</updated>
<author>
<name>Brian Norris</name>
<email>computersforpeace@gmail.com</email>
</author>
<published>2015-10-26T17:20:23+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux-stable.git/commit/?id=419608b9adde80d8638daa01c8614e1b92416fc0'/>
<id>419608b9adde80d8638daa01c8614e1b92416fc0</id>
<content type='text'>
commit f3c63795e90f0c6238306883b6c72f14d5355721 upstream.

Commit 073db4a51ee4 ("mtd: fix: avoid race condition when accessing
mtd-&gt;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.

 -&gt; blktrans_open()
    -&gt;  mutex_lock(&amp;dev-&gt;lock);
    -&gt;  mutex_lock(&amp;mtd_table_mutex);

 -&gt; del_mtd_device()
    -&gt;  mutex_lock(&amp;mtd_table_mutex);
    -&gt;  blktrans_notify_remove() -&gt; del_mtd_blktrans_dev()
       -&gt;  mutex_lock(&amp;dev-&gt;lock);

This is a classic (potential) ABBA deadlock, which can be fixed by
making the A-&gt;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 &amp;dev-&gt;lock:

        if (mutex_trylock(&amp;mtd_table_mutex)) {
                mutex_unlock(&amp;mtd_table_mutex);
                BUG();
        }

So, reverse the ordering of acquisition of &amp;dev-&gt;lock and &amp;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]  (&amp;new-&gt;lock){+.+...}, at: [&lt;c043fe4c&gt;] del_mtd_blktrans_dev+0x80/0xdc
  [   53.457271]
  [   53.457271] but task is already holding lock:
  [   53.463372]  (mtd_table_mutex){+.+.+.}, at: [&lt;c0439994&gt;] 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]
  -&gt; #1 (mtd_table_mutex){+.+.+.}:
  [   53.492331]        [&lt;c043fc5c&gt;] blktrans_open+0x34/0x1a4
  [   53.497879]        [&lt;c01afce0&gt;] __blkdev_get+0xc4/0x3b0
  [   53.503364]        [&lt;c01b0bb8&gt;] blkdev_get+0x108/0x320
  [   53.508743]        [&lt;c01713c0&gt;] do_dentry_open+0x218/0x314
  [   53.514496]        [&lt;c0180454&gt;] path_openat+0x4c0/0xf9c
  [   53.519959]        [&lt;c0182044&gt;] do_filp_open+0x5c/0xc0
  [   53.525336]        [&lt;c0172758&gt;] do_sys_open+0xfc/0x1cc
  [   53.530716]        [&lt;c000f740&gt;] ret_fast_syscall+0x0/0x1c
  [   53.536375]
  -&gt; #0 (&amp;new-&gt;lock){+.+...}:
  [   53.540587]        [&lt;c063f124&gt;] mutex_lock_nested+0x38/0x3cc
  [   53.546504]        [&lt;c043fe4c&gt;] del_mtd_blktrans_dev+0x80/0xdc
  [   53.552606]        [&lt;c043f164&gt;] blktrans_notify_remove+0x7c/0x84
  [   53.558891]        [&lt;c04399f0&gt;] del_mtd_device+0x74/0x100
  [   53.564544]        [&lt;c043c670&gt;] del_mtd_partitions+0x80/0xc8
  [   53.570451]        [&lt;c0439aa0&gt;] mtd_device_unregister+0x24/0x48
  [   53.576637]        [&lt;c046ce6c&gt;] spi_drv_remove+0x1c/0x34
  [   53.582207]        [&lt;c03de0f0&gt;] __device_release_driver+0x88/0x114
  [   53.588663]        [&lt;c03de19c&gt;] device_release_driver+0x20/0x2c
  [   53.594843]        [&lt;c03dd9e8&gt;] bus_remove_device+0xd8/0x108
  [   53.600748]        [&lt;c03dacc0&gt;] device_del+0x10c/0x210
  [   53.606127]        [&lt;c03dadd0&gt;] device_unregister+0xc/0x20
  [   53.611849]        [&lt;c046d878&gt;] __unregister+0x10/0x20
  [   53.617211]        [&lt;c03da868&gt;] device_for_each_child+0x50/0x7c
  [   53.623387]        [&lt;c046eae8&gt;] spi_unregister_master+0x58/0x8c
  [   53.629578]        [&lt;c03e12f0&gt;] release_nodes+0x15c/0x1c8
  [   53.635223]        [&lt;c03de0f8&gt;] __device_release_driver+0x90/0x114
  [   53.641689]        [&lt;c03de900&gt;] driver_detach+0xb4/0xb8
  [   53.647147]        [&lt;c03ddc78&gt;] bus_remove_driver+0x4c/0xa0
  [   53.652970]        [&lt;c00cab50&gt;] SyS_delete_module+0x11c/0x1e4
  [   53.658976]        [&lt;c000f740&gt;] 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(&amp;new-&gt;lock);
  [   53.698306]                                lock(mtd_table_mutex);
  [   53.704658]   lock(&amp;new-&gt;lock);
  [   53.707946]
  [   53.707946]  *** DEADLOCK ***

Fixes: 073db4a51ee4 ("mtd: fix: avoid race condition when accessing mtd-&gt;usecount")
Reported-by: Felipe Balbi &lt;balbi@ti.com&gt;
Tested-by: Felipe Balbi &lt;balbi@ti.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
commit f3c63795e90f0c6238306883b6c72f14d5355721 upstream.

Commit 073db4a51ee4 ("mtd: fix: avoid race condition when accessing
mtd-&gt;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.

 -&gt; blktrans_open()
    -&gt;  mutex_lock(&amp;dev-&gt;lock);
    -&gt;  mutex_lock(&amp;mtd_table_mutex);

 -&gt; del_mtd_device()
    -&gt;  mutex_lock(&amp;mtd_table_mutex);
    -&gt;  blktrans_notify_remove() -&gt; del_mtd_blktrans_dev()
       -&gt;  mutex_lock(&amp;dev-&gt;lock);

This is a classic (potential) ABBA deadlock, which can be fixed by
making the A-&gt;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 &amp;dev-&gt;lock:

        if (mutex_trylock(&amp;mtd_table_mutex)) {
                mutex_unlock(&amp;mtd_table_mutex);
                BUG();
        }

So, reverse the ordering of acquisition of &amp;dev-&gt;lock and &amp;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]  (&amp;new-&gt;lock){+.+...}, at: [&lt;c043fe4c&gt;] del_mtd_blktrans_dev+0x80/0xdc
  [   53.457271]
  [   53.457271] but task is already holding lock:
  [   53.463372]  (mtd_table_mutex){+.+.+.}, at: [&lt;c0439994&gt;] 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]
  -&gt; #1 (mtd_table_mutex){+.+.+.}:
  [   53.492331]        [&lt;c043fc5c&gt;] blktrans_open+0x34/0x1a4
  [   53.497879]        [&lt;c01afce0&gt;] __blkdev_get+0xc4/0x3b0
  [   53.503364]        [&lt;c01b0bb8&gt;] blkdev_get+0x108/0x320
  [   53.508743]        [&lt;c01713c0&gt;] do_dentry_open+0x218/0x314
  [   53.514496]        [&lt;c0180454&gt;] path_openat+0x4c0/0xf9c
  [   53.519959]        [&lt;c0182044&gt;] do_filp_open+0x5c/0xc0
  [   53.525336]        [&lt;c0172758&gt;] do_sys_open+0xfc/0x1cc
  [   53.530716]        [&lt;c000f740&gt;] ret_fast_syscall+0x0/0x1c
  [   53.536375]
  -&gt; #0 (&amp;new-&gt;lock){+.+...}:
  [   53.540587]        [&lt;c063f124&gt;] mutex_lock_nested+0x38/0x3cc
  [   53.546504]        [&lt;c043fe4c&gt;] del_mtd_blktrans_dev+0x80/0xdc
  [   53.552606]        [&lt;c043f164&gt;] blktrans_notify_remove+0x7c/0x84
  [   53.558891]        [&lt;c04399f0&gt;] del_mtd_device+0x74/0x100
  [   53.564544]        [&lt;c043c670&gt;] del_mtd_partitions+0x80/0xc8
  [   53.570451]        [&lt;c0439aa0&gt;] mtd_device_unregister+0x24/0x48
  [   53.576637]        [&lt;c046ce6c&gt;] spi_drv_remove+0x1c/0x34
  [   53.582207]        [&lt;c03de0f0&gt;] __device_release_driver+0x88/0x114
  [   53.588663]        [&lt;c03de19c&gt;] device_release_driver+0x20/0x2c
  [   53.594843]        [&lt;c03dd9e8&gt;] bus_remove_device+0xd8/0x108
  [   53.600748]        [&lt;c03dacc0&gt;] device_del+0x10c/0x210
  [   53.606127]        [&lt;c03dadd0&gt;] device_unregister+0xc/0x20
  [   53.611849]        [&lt;c046d878&gt;] __unregister+0x10/0x20
  [   53.617211]        [&lt;c03da868&gt;] device_for_each_child+0x50/0x7c
  [   53.623387]        [&lt;c046eae8&gt;] spi_unregister_master+0x58/0x8c
  [   53.629578]        [&lt;c03e12f0&gt;] release_nodes+0x15c/0x1c8
  [   53.635223]        [&lt;c03de0f8&gt;] __device_release_driver+0x90/0x114
  [   53.641689]        [&lt;c03de900&gt;] driver_detach+0xb4/0xb8
  [   53.647147]        [&lt;c03ddc78&gt;] bus_remove_driver+0x4c/0xa0
  [   53.652970]        [&lt;c00cab50&gt;] SyS_delete_module+0x11c/0x1e4
  [   53.658976]        [&lt;c000f740&gt;] 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(&amp;new-&gt;lock);
  [   53.698306]                                lock(mtd_table_mutex);
  [   53.704658]   lock(&amp;new-&gt;lock);
  [   53.707946]
  [   53.707946]  *** DEADLOCK ***

Fixes: 073db4a51ee4 ("mtd: fix: avoid race condition when accessing mtd-&gt;usecount")
Reported-by: Felipe Balbi &lt;balbi@ti.com&gt;
Tested-by: Felipe Balbi &lt;balbi@ti.com&gt;
Signed-off-by: Brian Norris &lt;computersforpeace@gmail.com&gt;
Signed-off-by: Ben Hutchings &lt;ben@decadent.org.uk&gt;
</pre>
</div>
</content>
</entry>
</feed>
