From patchwork Thu Nov 22 15:20:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Prins Anton (ST-CO/ENG1.1)" X-Patchwork-Id: 201066 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 364012C00A6 for ; Fri, 23 Nov 2012 02:21:50 +1100 (EST) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TbYaH-00081k-5i; Thu, 22 Nov 2012 15:20:57 +0000 Received: from smtp2-v.fe.bosch.de ([139.15.237.6]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TbYaD-00081O-5y for linux-mtd@lists.infradead.org; Thu, 22 Nov 2012 15:20:55 +0000 Received: from vsmta11.fe.internet.bosch.com (unknown [10.4.98.51]) by imta24.fe.bosch.de (Postfix) with ESMTP id B78EFB000E4; Thu, 22 Nov 2012 16:20:47 +0100 (CET) Received: from localhost (vsgw3.fe.internet.bosch.com [10.4.98.16]) by vsmta11.fe.internet.bosch.com (Postfix) with SMTP id 228C243C04CA; Thu, 22 Nov 2012 16:20:21 +0100 (CET) Received: from SI-MBX14.de.bosch.com ([10.3.153.115]) by fe-hub05.de.bosch.com ([10.3.153.64]) with mapi; Thu, 22 Nov 2012 16:20:45 +0100 From: "Prins Anton (ST-CO/ENG1.1)" To: "dedekind1@gmail.com" Date: Thu, 22 Nov 2012 16:20:44 +0100 Subject: RE: Patch to solve NULL pointer dereference in physmap_of.c Thread-Topic: Patch to solve NULL pointer dereference in physmap_of.c Thread-Index: Ac3HvQyiJhnhbtSBTGK168hKxLPEwgBB8wrQ Message-ID: <85D877DD6EE67B4A9FCA9B9C3A4865670C3AF1D4E4@SI-MBX14.de.bosch.com> References: <85D877DD6EE67B4A9FCA9B9C3A4865670C3ADE0635@SI-MBX14.de.bosch.com> <1353483740.2701.1.camel@sauron.fi.intel.com> In-Reply-To: <1353483740.2701.1.camel@sauron.fi.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: x-secude-secure-mail: 5.0.12.1 acceptlanguage: en-US MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20121122_102053_694512_EAB089B8 X-CRM114-Status: GOOD ( 20.43 ) X-Spam-Score: -6.9 (------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-6.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [139.15.237.6 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: "linux-mtd@lists.infradead.org" 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 [PATCH] mtd: maps/physmap_of.c: change error checking to prevent a NULL pointer dereference if no DTS tuple is mappable This patch solves a NULL pointer dereference, this may occur if the tuple is not mappable (jumps to continue in the for-loop). Out of the loop possible results are: - info->list_size == 0 if no of the tuples is mappable - info->list_size == 1 - info->list_size > 1 If no one of the supplied tuples is mappable (info->list_size == 0) and info->cmtd will not be set. But it is used in mtd_device_parse_register, OOPS... if should generate an error in this case! [From: Anton Prins ] From: Artem Bityutskiy [mailto:dedekind1@gmail.com] Sent: woensdag 21 november 2012 8:42 To: Prins Anton (ST-CO/ENG1.1) Cc: linux-mtd@lists.infradead.org Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote: > commit 0905a6f4aec377123e94d2260f2f7a0d867e19be > Author: Anton Prins > Date: Fri Nov 9 10:12:58 2012 +0100 > > Correct error checking to prevent a NULL pointer dereference > > The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus. > In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error! Sorry, I do not really understand which problem this patch solves, could you please improve the commit message and re-send? > > diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c > index 2e6fb68..83d121e 100644 > --- a/drivers/mtd/maps/physmap_of.c > +++ b/drivers/mtd/maps/physmap_of.c > @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev) > info->list[i].mtd->dev.parent = &dev->dev; > } > It seems the error condition should be checked and acted upon here. What you looks more like making the code less readable. > - err = 0; > if (info->list_size == 1) { > + err = 0; > info->cmtd = info->list[0].mtd; > } else if (info->list_size > 1) { > /* > * We detected multiple devices. Concatenate them together. > */ > + err = 0; > info->cmtd = mtd_concat_create(mtd_list, info->list_size, > dev_name(&dev->dev)); > if (info->cmtd == NULL) diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c index 2e6fb68..f6de444 100644 --- a/drivers/mtd/maps/physmap_of.c +++ b/drivers/mtd/maps/physmap_of.c @@ -268,6 +268,7 @@ static int __devinit of_flash_probe(struct platform_device *dev) } err = 0; + info->cmtd = NULL; if (info->list_size == 1) { info->cmtd = info->list[0].mtd; } else if (info->list_size > 1) { @@ -276,9 +277,10 @@ static int __devinit of_flash_probe(struct platform_device *dev) */ info->cmtd = mtd_concat_create(mtd_list, info->list_size, dev_name(&dev->dev)); - if (info->cmtd == NULL) - err = -ENXIO; } + if (info->cmtd == NULL) + err = -ENXIO; + if (err) goto err_out; -----Original Message-----