From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966766AbbDVQrJ (ORCPT ); Wed, 22 Apr 2015 12:47:09 -0400 Received: from mail-ie0-f180.google.com ([209.85.223.180]:32774 "EHLO mail-ie0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965744AbbDVQrF (ORCPT ); Wed, 22 Apr 2015 12:47:05 -0400 Date: Wed, 22 Apr 2015 11:47:01 -0500 From: Bjorn Helgaas To: Ricardo Ribalda Delgado Cc: Greg Kroah-Hartman , Grant Likely , Rob Herring , Andrew Morton , Jakub Sitnicki , Vivek Goyal , Jiang Liu , Mike Travis , Thierry Reding , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v3 1/4] kernel/resource: Invalid memory access in __release_resource Message-ID: <20150422164701.GJ20701@google.com> References: <1429719261-18024-1-git-send-email-ricardo.ribalda@gmail.com> <1429719261-18024-2-git-send-email-ricardo.ribalda@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1429719261-18024-2-git-send-email-ricardo.ribalda@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 22, 2015 at 06:14:18PM +0200, Ricardo Ribalda Delgado wrote: > When a resource is initialized via of_platform_populate. > resource->parent is initialized to NULL via kzalloc. > (of_platform_populate->of_device_alloc->of_address_to_resource) > > If of_platform_depopulate is called later, resource->parent is > accessed (Offset 0x30 of address 0), causing a kernel error. > > This patch evaluates resouce->parent before accessing it. If it > is not initialized, -EACCESS is returned. > > Also a WARN is thrown, so the developer can have a hint about what > needs to be fixed. > > Fixes: > BUG: unable to handle kernel NULL pointer deference at 0000000000000030 > IP: release_resource+0x26/0x90 > Signed-off-by: Ricardo Ribalda Delgado > --- > kernel/resource.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/resource.c b/kernel/resource.c > index 90552aa..b7b270f 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -237,6 +237,9 @@ static int __release_resource(struct resource *old) > { > struct resource *tmp, **p; > > + if (WARN_ON(!old->parent)) > + return -EINVAL; I'm not really a fan of this. The NULL pointer oops is a very good clue all by itself, and it doesn't require any extra code here. > p = &old->parent->child; > for (;;) { > tmp = *p; > -- > 2.1.4 >