From patchwork Tue Oct 9 19:11:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Betker X-Patchwork-Id: 190415 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from merlin.infradead.org (merlin.infradead.org [IPv6:2001:4978:20e::2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 2217B2C009A for ; Wed, 10 Oct 2012 06:15:22 +1100 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TLfFp-0005sN-9X; Tue, 09 Oct 2012 19:14:09 +0000 Received: from mout2.freenet.de ([2001:748:100:40::2:4]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TLfFl-0005s7-6e for linux-mtd@lists.infradead.org; Tue, 09 Oct 2012 19:14:06 +0000 Received: from [195.4.92.142] (helo=mjail2.freenet.de) by mout2.freenet.de with esmtpa (ID thomas.betker@freenet.de) (port 25) (Exim 4.76 #1) id 1TLfFY-00047c-KA; Tue, 09 Oct 2012 21:13:52 +0200 Received: from localhost ([::1]:53273 helo=mjail2.freenet.de) by mjail2.freenet.de with esmtpa (ID thomas.betker@freenet.de) (Exim 4.76 #1) id 1TLfFY-000704-E3; Tue, 09 Oct 2012 21:13:52 +0200 Received: from [195.4.92.19] (port=41983 helo=9.mx.freenet.de) by mjail2.freenet.de with esmtpa (ID thomas.betker@freenet.de) (Exim 4.76 #1) id 1TLfDM-0005eZ-3T; Tue, 09 Oct 2012 21:11:36 +0200 Received: from dslb-084-057-028-182.pools.arcor-ip.net ([84.57.28.182]:1858 helo=[192.168.2.101]) by 9.mx.freenet.de with esmtpa (ID thomas.betker@freenet.de) (port 587) (Exim 4.76 #1) id 1TLfDL-0003uL-SK; Tue, 09 Oct 2012 21:11:35 +0200 Message-ID: <507476E7.8030705@freenet.de> Date: Tue, 09 Oct 2012 21:11:35 +0200 From: Thomas Betker User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20120909 Firefox/15.0.1 SeaMonkey/2.12.1 MIME-Version: 1.0 To: linux-mtd@lists.infradead.org, thomas.betker@rohde-schwarz.com Subject: [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin References: <507321E6.1050609@freenet.de> In-Reply-To: <507321E6.1050609@freenet.de> X-Forwarded-Message-Id: <507321E6.1050609@freenet.de> X-Spam-Note: CRM114 invocation failed X-Spam-Score: -4.0 (----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-4.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (thomas.betker[at]freenet.de) -2.1 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org jffs2_write_begin() first acquires the page lock, then f->sem. This causes an AB-BA deadlock with jffs2_garbage_collect_live(), which first acquires f->sem, then the page lock: jffs2_garbage_collect_live mutex_lock(&f->sem) (A) jffs2_garbage_collect_dnode jffs2_gc_fetch_page read_cache_page_async do_read_cache_page lock_page(page) (B) jffs2_write_begin grab_cache_page_write_begin find_lock_page lock_page(page) (B) mutex_lock(&f->sem) (A) We fix this by restructuring jffs2_write_begin() to take f->sem before the page lock. However, we make sure that f->sem is not held when calling jffs2_reserve_space(), as this is not permitted by the locking rules. The deadlock above was observed multiple times on an SoC with a dual ARMv7 (Cortex-A9), running the long-term 3.4.11 kernel; it occurred when using scp to copy files from a host system to the ARM target system. The fix was heavily tested on the same target system. If the patch is accepted, please get it also pushed to 3.4; it applies cleanly both to linux-mtd.git and the current linux-3.4 tree. Cc: Joakim Tjernlund Signed-off-by: Thomas Betker Acked-by: Joakim Tjernlund diff -Naur linux-3.4.11-orig/fs/jffs2/file.c linux-3.4.11/fs/jffs2/file.c --- linux-3.4.11-orig/fs/jffs2/file.c 2012-09-14 22:18:55.000000000 +0000 +++ linux-3.4.11/fs/jffs2/file.c 2012-10-05 09:01:10.000000000 +0000 @@ -138,33 +138,39 @@ 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 (!pg) { + if (alloc_len) + jffs2_complete_reservation(c); + mutex_unlock(&f->sem); return -ENOMEM; + } *pagep = pg; - jffs2_dbg(1, "%s()\n", __func__); - - if (pageofs > inode->i_size) { + if (alloc_len) { /* 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); @@ -191,7 +197,6 @@ 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); @@ -206,12 +211,10 @@ 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); } /* @@ -220,18 +223,18 @@ * 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; }