From patchwork Fri Nov 29 21:13:49 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Khoroshilov X-Patchwork-Id: 295527 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from casper.infradead.org (unknown [IPv6:2001:770:15f::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EAE862C00C7 for ; Sat, 30 Nov 2013 08:15:01 +1100 (EST) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VmVOf-0007ji-0g; Fri, 29 Nov 2013 21:14:45 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VmVOd-0002WP-B1; Fri, 29 Nov 2013 21:14:43 +0000 Received: from mail.ispras.ru ([83.149.199.45]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VmVOa-0002VY-Mx for linux-mtd@lists.infradead.org; Fri, 29 Nov 2013 21:14:42 +0000 Received: from localhost.localdomain (ppp85-141-248-161.pppoe.mtu-net.ru [85.141.248.161]) by mail.ispras.ru (Postfix) with ESMTPSA id 392D9540159; Sat, 30 Nov 2013 01:14:14 +0400 (MSK) From: Alexey Khoroshilov To: David Woodhouse Subject: [PATCH] jffs2: make jffs2_do_read_inode_internal() consistent regarding f->sem handling Date: Sat, 30 Nov 2013 01:13:49 +0400 Message-Id: <1385759629-31176-1-git-send-email-khoroshilov@ispras.ru> X-Mailer: git-send-email 1.8.3.2 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131129_161441_050814_614B14C1 X-CRM114-Status: GOOD ( 13.09 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 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] Cc: ldv-project@linuxtesting.org, Artem Bityutskiy , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Alexey Khoroshilov X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org jffs2_do_read_inode_internal() does unlock of f->sem mutex on several failure paths, while its users expect f->sem is still held. As a result double unlock of the mutex can happen. The patch makes jffs2_do_read_inode_internal() consistent regarding f->sem handling. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov --- fs/jffs2/readinode.c | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c index ae81b01e6fd7..a1c9ed65b502 100644 --- a/fs/jffs2/readinode.c +++ b/fs/jffs2/readinode.c @@ -1223,18 +1223,17 @@ static int jffs2_do_read_inode_internal(struct jffs2_sb_info *c, JFFS2_ERROR("failed to read from flash: error %d, %zd of %zd bytes read\n", ret, retlen, sizeof(*latest_node)); /* FIXME: If this fails, there seems to be a memory leak. Find it. */ - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return ret?ret:-EIO; + if (!ret) + ret = -EIO; + goto clear_inode; } crc = crc32(0, latest_node, sizeof(*latest_node)-8); if (crc != je32_to_cpu(latest_node->node_crc)) { JFFS2_ERROR("CRC failed for read_inode of inode %u at physical location 0x%x\n", f->inocache->ino, ref_offset(rii.latest_ref)); - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return -EIO; + ret = -EIO; + goto clear_inode; } switch(jemode_to_cpu(latest_node->mode) & S_IFMT) { @@ -1271,16 +1270,14 @@ static int jffs2_do_read_inode_internal(struct jffs2_sb_info *c, * operation. */ uint32_t csize = je32_to_cpu(latest_node->csize); if (csize > JFFS2_MAX_NAME_LEN) { - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return -ENAMETOOLONG; + ret = -ENAMETOOLONG; + goto clear_inode; } f->target = kmalloc(csize + 1, GFP_KERNEL); if (!f->target) { JFFS2_ERROR("can't allocate %u bytes of memory for the symlink target path cache\n", csize); - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return -ENOMEM; + ret = -ENOMEM; + goto clear_inode; } ret = jffs2_flash_read(c, ref_offset(rii.latest_ref) + sizeof(*latest_node), @@ -1291,9 +1288,7 @@ static int jffs2_do_read_inode_internal(struct jffs2_sb_info *c, ret = -EIO; kfree(f->target); f->target = NULL; - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return ret; + goto clear_inode; } f->target[csize] = '\0'; @@ -1309,25 +1304,22 @@ static int jffs2_do_read_inode_internal(struct jffs2_sb_info *c, if (f->metadata) { JFFS2_ERROR("Argh. Special inode #%u with mode 0%o had metadata node\n", f->inocache->ino, jemode_to_cpu(latest_node->mode)); - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return -EIO; + ret = -EIO; + goto clear_inode; } if (!frag_first(&f->fragtree)) { JFFS2_ERROR("Argh. Special inode #%u with mode 0%o has no fragments\n", f->inocache->ino, jemode_to_cpu(latest_node->mode)); - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return -EIO; + ret = -EIO; + goto clear_inode; } /* ASSERT: f->fraglist != NULL */ if (frag_next(frag_first(&f->fragtree))) { JFFS2_ERROR("Argh. Special inode #%u with mode 0x%x had more than one node\n", f->inocache->ino, jemode_to_cpu(latest_node->mode)); /* FIXME: Deal with it - check crc32, check for duplicate node, check times and discard the older one */ - mutex_unlock(&f->sem); - jffs2_do_clear_inode(c, f); - return -EIO; + ret = -EIO; + goto clear_inode; } /* OK. We're happy */ f->metadata = frag_first(&f->fragtree)->node; @@ -1339,6 +1331,12 @@ static int jffs2_do_read_inode_internal(struct jffs2_sb_info *c, jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT); return 0; + +clear_inode: + mutex_unlock(&f->sem); + jffs2_do_clear_inode(c, f); + mutex_lock(&f->sem); + return ret; } /* Scan the list of all nodes present for this ino, build map of versions, etc. */