From patchwork Tue Oct 20 16:23:33 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hartley Sweeten X-Patchwork-Id: 36477 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id B9808B7B97 for ; Wed, 21 Oct 2009 03:25:49 +1100 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1N0HUw-00085h-CV; Tue, 20 Oct 2009 16:23:46 +0000 Received: from exprod6og101.obsmtp.com ([64.18.1.181]) by bombadil.infradead.org with smtp (Exim 4.69 #1 (Red Hat Linux)) id 1N0HUp-00084e-26 for linux-mtd@lists.infradead.org; Tue, 20 Oct 2009 16:23:43 +0000 Received: from source ([63.240.6.3]) (using TLSv1) by exprod6ob101.postini.com ([64.18.5.12]) with SMTP ID DSNKSt3kCC13N3sH3SFstrxS06mBkbcyJDJ6@postini.com; Tue, 20 Oct 2009 09:23:39 PDT Received: from D01HOST03.Mi8.com ([172.16.1.25]) by Outbound01.Mi8.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 20 Oct 2009 12:23:36 -0400 Received: from mi8nycmail19.Mi8.com ([172.16.7.219]) by D01HOST03.Mi8.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 20 Oct 2009 12:23:36 -0400 X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Subject: RE: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c Date: Tue, 20 Oct 2009 12:23:33 -0400 Message-ID: In-Reply-To: <20091021.002941.41633716.anemo@mba.ocn.ne.jp> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] mtd: Fix kernel NULL pointer dereference in physmap.c Thread-Index: AcpRmiZlw7acLrv8TVql3itUC672gwABkkcg References: <20091021.002941.41633716.anemo@mba.ocn.ne.jp> From: "H Hartley Sweeten" To: X-OriginalArrivalTime: 20 Oct 2009 16:23:36.0086 (UTC) FILETIME=[AC9A5360:01CA51A1] X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20091020_122339_291004_DEDC746D X-CRM114-Status: GOOD ( 17.67 ) X-Spam-Score: -4.0 (----) X-Spam-Report: SpamAssassin version 3.2.5 on bombadil.infradead.org summary: Content analysis details: (-4.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -4.0 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [64.18.1.181 listed in list.dnswl.org] Cc: Atsushi Nemoto , dwmw2@infradead.org X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 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 During the probe for physmap platform flash devices there are a number error exit conditions that all do a goto err_out which then calls physmap_flash_remove(). In that function one of the cleanup steps is: #ifdef CONFIG_MTD_CONCAT if (info->cmtd != info->mtd[0]) mtd_concat_destroy(info->cmtd); #endif This test will succeed since info->cmtd == NULL and info->mtd[0] is valid, which then causes a NULL pointer dereference when mtd_concat_destroy() is called. Fix this by moving the mtd_concat_destroy() step into the if (info->cmtd) condition. Also, move the kfree(info->parts) cleanup to remove an #ifdef. Signed-off-by: H Hartley Sweeten Cc: David Woodhouse Cc: Atsushi Nemoto --- V2 - As pointed out by Atsushi Nemoto, the map_destroy loop should not be skipped even when info->cmtd == NULL. diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c index 380648e..3f13a96 100644 --- a/drivers/mtd/maps/physmap.c +++ b/drivers/mtd/maps/physmap.c @@ -48,23 +48,22 @@ static int physmap_flash_remove(struct platform_device *dev) if (info->cmtd) { #ifdef CONFIG_MTD_PARTITIONS - if (info->nr_parts || physmap_data->nr_parts) + if (info->nr_parts || physmap_data->nr_parts) { del_mtd_partitions(info->cmtd); - else + + if (info->nr_parts) + kfree(info->parts); + } else { del_mtd_device(info->cmtd); + } #else del_mtd_device(info->cmtd); #endif - } -#ifdef CONFIG_MTD_PARTITIONS - if (info->nr_parts) - kfree(info->parts); -#endif - #ifdef CONFIG_MTD_CONCAT - if (info->cmtd != info->mtd[0]) - mtd_concat_destroy(info->cmtd); + if (info->cmtd != info->mtd[0]) + mtd_concat_destroy(info->cmtd); #endif + } for (i = 0; i < MAX_RESOURCES; i++) { if (info->mtd[i] != NULL)