From patchwork Wed Jan 23 18:59:08 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Gortmaker X-Patchwork-Id: 215022 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 35C4A2C007E for ; Thu, 24 Jan 2013 06:00:56 +1100 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Ty5Xr-0001jA-7Y; Wed, 23 Jan 2013 18:59:35 +0000 Received: from mail.windriver.com ([147.11.1.11]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Ty5Xo-0001ia-AV for linux-mtd@lists.infradead.org; Wed, 23 Jan 2013 18:59:32 +0000 Received: from yow-lpgnfs-02.corp.ad.wrs.com (yow-lpgnfs-02.wrs.com [128.224.149.8]) by mail.windriver.com (8.14.5/8.14.3) with ESMTP id r0NIxHLp013326; Wed, 23 Jan 2013 10:59:17 -0800 (PST) From: Paul Gortmaker To: linux-mtd@lists.infradead.org Subject: [PATCH] mtd: fix random pointer dereference in OF device name handling Date: Wed, 23 Jan 2013 13:59:08 -0500 Message-Id: <1358967548-25491-1-git-send-email-paul.gortmaker@windriver.com> X-Mailer: git-send-email 1.8.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130123_135932_583523_847D9A84 X-CRM114-Status: GOOD ( 13.55 ) X-Spam-Score: -4.9 (----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-4.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [147.11.1.11 listed in list.dnswl.org] -0.7 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: Paul Gortmaker , David Woodhouse , Jean-Christophe PLAGNIOL-VILLARD , linux-kernel@vger.kernel.org, Artem Bityutskiy 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: , MIME-Version: 1.0 Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org Here is the output from an mpc8548 based board. There are three instances of missing device name here: 1 |: Found 4 x16 devices at 0x0 in 32-bit bank. Manufacturer ID 0x008989 Chip ID 0x001818 2 |Intel/Sharp Extended Query Table at 0x0031 3 |Intel/Sharp Extended Query Table at 0x0031 4 |Using buffer write method 5 |cfi_cmdset_0001: Erase suspend on write enabled 6 |2 ofpart partitions found on MTD device 7 |Creating 2 MTD partitions on "": 8 |0x000000000000-0x000003f00000 : "space" 9 |0x000003f00000-0x000004000000 : "bootloader" Lines 1 (BOL), 6 (EOL) and 7 (inside quotes) have the missing device name issue. Problem introduced with commit d68cbdd4fb04d2b756ad53c22f36943167b16340 "mtd: physmap_of: allow to specify the mtd name for retro compatiblity" There are actually two bugs here. The 1st is that mtd_name is on the stack and never initialized. It uses a call to of_property_read_string() to get the pointer. However this function is explicitly documented as saying that the char "...pointer is modified only if a valid string can be decoded." Hence it isn't NULL, and we use a pointer off in the weeds as the device name, leading to undefined behaviour. The second issue is in the NULL check itself. It uses a "?" operator to choose between mtd_name and the devicetree based name. But the operator isn't given two choices. One choice (mtd_name) is missing from the RHS of the "?". With these fixed, the output appears as follows: 1 |fc000000.flash: Found 4 x16 devices at 0x0 in 32-bit bank. Manufacturer ID 0x008989 Chip ID 0x001818 2 |Intel/Sharp Extended Query Table at 0x0031 3 |Intel/Sharp Extended Query Table at 0x0031 4 |Using buffer write method 5 |cfi_cmdset_0001: Erase suspend on write enabled 6 |2 ofpart partitions found on MTD device fc000000.flash 7 |Creating 2 MTD partitions on "fc000000.flash": 8 |0x000000000000-0x000003f00000 : "space" 9 |0x000003f00000-0x000004000000 : "bootloader" All the names are now appearing where they should be. Cc: Jean-Christophe PLAGNIOL-VILLARD Cc: Artem Bityutskiy Cc: David Woodhouse Signed-off-by: Paul Gortmaker --- [Introduced into mainline at v3.8-rc1~47 ; hence no need for stable Cc] drivers/mtd/maps/physmap_of.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c index 67cc73c..a70c1c4 100644 --- a/drivers/mtd/maps/physmap_of.c +++ b/drivers/mtd/maps/physmap_of.c @@ -170,7 +170,7 @@ static int of_flash_probe(struct platform_device *dev) resource_size_t res_size; struct mtd_part_parser_data ppdata; bool map_indirect; - const char *mtd_name; + const char *mtd_name = NULL; match = of_match_device(of_flash_match, &dev->dev); if (!match) @@ -237,7 +237,8 @@ static int of_flash_probe(struct platform_device *dev) goto err_out; } - info->list[i].map.name = mtd_name ?: dev_name(&dev->dev); + info->list[i].map.name = mtd_name ? + mtd_name : dev_name(&dev->dev); info->list[i].map.phys = res.start; info->list[i].map.size = res_size; info->list[i].map.bankwidth = be32_to_cpup(width);