Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1363009

[PATCH 3.13.y-ckt 08/43] Revert "jffs2: Fix lock acquisition order bug in jffs2_write_begin"

From Kamal Mostafa <kamal@canonical.com>
Newsgroups linux.kernel
Subject [PATCH 3.13.y-ckt 08/43] Revert "jffs2: Fix lock acquisition order bug in jffs2_write_begin"
Date 2016-03-22 22:20 +0100
Message-ID <rfBSz-1PB-29@gated-at.bofh.it> (permalink)
References <rfBIR-1Lk-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

3.13.11-ckt37 -stable review patch.  If anyone has any objections, please let me know.

---8<------------------------------------------------------------

From: Thomas Betker <thomas.betker@rohde-schwarz.com>

commit 157078f64b8a9cd7011b6b900b2f2498df850748 upstream.

This reverts commit 5ffd3412ae55
("jffs2: Fix lock acquisition order bug in jffs2_write_begin").

The commit modified jffs2_write_begin() to remove a deadlock with
jffs2_garbage_collect_live(), but this introduced new deadlocks found
by multiple users. page_lock() actually has to be called before
mutex_lock(&c->alloc_sem) or mutex_lock(&f->sem) because
jffs2_write_end() and jffs2_readpage() are called with the page locked,
and they acquire c->alloc_sem and f->sem, resp.

In other words, the lock order in jffs2_write_begin() was correct, and
it is the jffs2_garbage_collect_live() path that has to be changed.

Revert the commit to get rid of the new deadlocks, and to clear the way
for a better fix of the original deadlock.

Reported-by: Deng Chao <deng.chao1@zte.com.cn>
Reported-by: Ming Liu <liu.ming50@gmail.com>
Reported-by: wangzaiwei <wangzaiwei@top-vision.cn>
Signed-off-by: Thomas Betker <thomas.betker@rohde-schwarz.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 fs/jffs2/file.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 1506673..60ef3fb 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -138,39 +138,33 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
 	struct page *pg;
 	struct inode *inode = mapping->host;
 	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
-	struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
-	struct jffs2_raw_inode ri;
-	uint32_t alloc_len = 0;
 	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
 	uint32_t pageofs = index << PAGE_CACHE_SHIFT;
 	int ret = 0;
 
-	jffs2_dbg(1, "%s()\n", __func__);
-
-	if (pageofs > inode->i_size) {
-		ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
-					  ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
-		if (ret)
-			return ret;
-	}
-
-	mutex_lock(&f->sem);
 	pg = grab_cache_page_write_begin(mapping, index, flags);
-	if (!pg) {
-		if (alloc_len)
-			jffs2_complete_reservation(c);
-		mutex_unlock(&f->sem);
+	if (!pg)
 		return -ENOMEM;
-	}
 	*pagep = pg;
 
-	if (alloc_len) {
+	jffs2_dbg(1, "%s()\n", __func__);
+
+	if (pageofs > inode->i_size) {
 		/* Make new hole frag from old EOF to new page */
+		struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
+		struct jffs2_raw_inode ri;
 		struct jffs2_full_dnode *fn;
+		uint32_t alloc_len;
 
 		jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new page\n",
 			  (unsigned int)inode->i_size, pageofs);
 
+		ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
+					  ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
+		if (ret)
+			goto out_page;
+
+		mutex_lock(&f->sem);
 		memset(&ri, 0, sizeof(ri));
 
 		ri.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
@@ -197,6 +191,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
 		if (IS_ERR(fn)) {
 			ret = PTR_ERR(fn);
 			jffs2_complete_reservation(c);
+			mutex_unlock(&f->sem);
 			goto out_page;
 		}
 		ret = jffs2_add_full_dnode_to_inode(c, f, fn);
@@ -211,10 +206,12 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
 			jffs2_mark_node_obsolete(c, fn->raw);
 			jffs2_free_full_dnode(fn);
 			jffs2_complete_reservation(c);
+			mutex_unlock(&f->sem);
 			goto out_page;
 		}
 		jffs2_complete_reservation(c);
 		inode->i_size = pageofs;
+		mutex_unlock(&f->sem);
 	}
 
 	/*
@@ -223,18 +220,18 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
 	 * case of a short-copy.
 	 */
 	if (!PageUptodate(pg)) {
+		mutex_lock(&f->sem);
 		ret = jffs2_do_readpage_nolock(inode, pg);
+		mutex_unlock(&f->sem);
 		if (ret)
 			goto out_page;
 	}
-	mutex_unlock(&f->sem);
 	jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags);
 	return ret;
 
 out_page:
 	unlock_page(pg);
 	page_cache_release(pg);
-	mutex_unlock(&f->sem);
 	return ret;
 }
 
-- 
2.7.0

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[3.13.y-ckt stable] Linux 3.13.11-ckt37 stable review Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 38/43] ext4: iterate over buffer heads correctly in move_extent_per_page() Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 39/43] net/mlx4_core: Allow resetting VF admin mac to zero Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 43/43] IB/core: Use GRH when the path hop-limit > 0 Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 32/43] PM / sleep / x86: Fix crash on graph trace through x86 suspend Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 25/43] ASoC: wm8994: Fix enum ctl accesses in a wrong type Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 30/43] drm/ast: Fix incorrect register check for DRAM width Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 34/43] MIPS: traps: Fix SIGFPE information leak from `do_ov' and `do_trap_or_bp' Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:10 +0100
  [PATCH 3.13.y-ckt 18/43] cifs: fix out-of-bounds access in lease parsing Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 13/43] vfio: fix ioctl error handling Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 20/43] ALSA: hdspm: Fix wrong boolean ctl value accesses Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 23/43] USB: cp210x: Add ID for Parrot NMEA GPS Flight Recorder Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 16/43] ALSA: timer: Fix broken compat timer user status ioctl Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 05/43] mac80211: fix use of uninitialised values in RX aggregation Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 22/43] ALSA: hdsp: Fix wrong boolean ctl value accesses Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 24/43] ASoC: wm8958: Fix enum ctl accesses in a wrong type Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 08/43] Revert "jffs2: Fix lock acquisition order bug in jffs2_write_begin" Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 02/43] Input: aiptek - fix crash on detecting device without endpoints Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 07/43] mac80211: minstrel_ht: set default tx aggregation timeout to 0 Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 17/43] ALSA: timer: Fix ioctls for X32 ABI Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 11/43] iommu/amd: Fix boot warning when device 00:00.0 is not iommu covered Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 01/43] [stable-only] AIO: properly check iovec sizes Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 14/43] ALSA: ctl: Fix ioctls for X32 ABI Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 04/43] cfg80211/wext: fix message ordering Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 06/43] libata: fix HDIO_GET_32BIT ioctl Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 10/43] Fix directory hardlinks from deleted directories Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 15/43] ALSA: rawmidi: Fix ioctls X32 ABI Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 27/43] USB: serial: option: add support for Telit LE922 PID 0x1045 Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 09/43] jffs2: Fix page lock / f->sem deadlock Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 19/43] CIFS: Fix SMB2+ interim response processing for read requests Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100
  [PATCH 3.13.y-ckt 26/43] ASoC: wm_adsp: Fix enum ctl accesses in a wrong type Kamal Mostafa <kamal@canonical.com> - 2016-03-22 22:20 +0100

csiph-web