stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Deng Chao <deng.chao1@zte.com.cn>,
	Ming Liu <liu.ming50@gmail.com>,
	wangzaiwei <wangzaiwei@top-vision.cn>,
	Thomas Betker <thomas.betker@rohde-schwarz.com>,
	David Woodhouse <David.Woodhouse@intel.com>
Subject: [PATCH 3.14 12/36] Revert "jffs2: Fix lock acquisition order bug in jffs2_write_begin"
Date: Mon,  7 Mar 2016 15:45:55 -0800	[thread overview]
Message-ID: <20160307234602.143925787@linuxfoundation.org> (raw)
In-Reply-To: <20160307234600.344036091@linuxfoundation.org>

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

------------------

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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/jffs2/file.c |   39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -139,39 +139,33 @@ static int jffs2_write_begin(struct file
 	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);
@@ -198,6 +192,7 @@ static int jffs2_write_begin(struct file
 		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);
@@ -212,10 +207,12 @@ static int jffs2_write_begin(struct file
 			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);
 	}
 
 	/*
@@ -224,18 +221,18 @@ static int jffs2_write_begin(struct file
 	 * 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;
 }
 

  parent reply	other threads:[~2016-03-07 23:45 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-07 23:45 [PATCH 3.14 00/36] 3.14.64-stable review Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 01/36] bio: return EINTR if copying to user space got interrupted Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 02/36] locks: fix unlock when fcntl_setlk races with a close Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 03/36] EDAC, mc_sysfs: Fix freeing bus name Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 04/36] cifs: fix out-of-bounds access in lease parsing Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 05/36] CIFS: Fix SMB2+ interim response processing for read requests Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 06/36] iommu/amd: Fix boot warning when device 00:00.0 is not iommu covered Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 07/36] x86/entry/compat: Add missing CLAC to entry_INT80_32 Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 08/36] drm/ast: Fix incorrect register check for DRAM width Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 09/36] libata: fix HDIO_GET_32BIT ioctl Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 10/36] libata: Align ata_devices id on a cacheline Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 11/36] PM / sleep / x86: Fix crash on graph trace through x86 suspend Greg Kroah-Hartman
2016-03-07 23:45 ` Greg Kroah-Hartman [this message]
2016-03-07 23:45 ` [PATCH 3.14 15/36] ALSA: ctl: Fix ioctls for X32 ABI Greg Kroah-Hartman
2016-03-07 23:45 ` [PATCH 3.14 16/36] ALSA: rawmidi: Fix ioctls " Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 17/36] ALSA: timer: Fix ioctls for " Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 18/36] ALSA: seq: oss: Dont drain at closing a client Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 19/36] ALSA: hdspm: Fix wrong boolean ctl value accesses Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 20/36] ALSA: hdsp: " Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 21/36] ALSA: hdspm: Fix zero-division Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 22/36] ALSA: timer: Fix broken compat timer user status ioctl Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 23/36] usb: chipidea: otg: change workqueue ci_otg as freezable Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 24/36] USB: cp210x: Add ID for Parrot NMEA GPS Flight Recorder Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 25/36] USB: serial: option: add support for Telit LE922 PID 0x1045 Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 26/36] USB: serial: option: add support for Quectel UC20 Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 27/36] MIPS: traps: Fix SIGFPE information leak from `do_ov and `do_trap_or_bp Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 28/36] ubi: Fix out of bounds write in volume update code Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 29/36] target: Fix Task Aborted Status (TAS) handling Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 30/36] target: Add TFO->abort_task for aborted task resources release Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 31/36] target: Fix LUN_RESET active TMR descriptor handling Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 32/36] target: Fix LUN_RESET active I/O handling for ACK_KREF Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 34/36] target: Fix remote-port TMR ABORT + se_cmd fabric stop Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 35/36] target: Fix race with SCF_SEND_DELAYED_TAS handling Greg Kroah-Hartman
2016-03-07 23:46 ` [PATCH 3.14 36/36] target: Fix WRITE_SAME/DISCARD conversion to linux 512b sectors Greg Kroah-Hartman
2016-03-08 11:44 ` [PATCH 3.14 00/36] 3.14.64-stable review Guenter Roeck
2016-03-08 16:20 ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160307234602.143925787@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=David.Woodhouse@intel.com \
    --cc=deng.chao1@zte.com.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liu.ming50@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=thomas.betker@rohde-schwarz.com \
    --cc=wangzaiwei@top-vision.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).