From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752593AbbBWPP2 (ORCPT ); Mon, 23 Feb 2015 10:15:28 -0500 Received: from foss-mx-na.arm.com ([217.140.108.86]:44362 "EHLO foss-mx-na.foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752239AbbBWPP1 (ORCPT ); Mon, 23 Feb 2015 10:15:27 -0500 Date: Mon, 23 Feb 2015 15:14:54 +0000 From: Mark Rutland To: Sudeep Holla Cc: "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Lorenzo Pieralisi , Greg Kroah-Hartman Subject: Re: [PATCH] drivers/base: cacheinfo: validate device node for all the caches Message-ID: <20150223151454.GL9714@leverpostej> References: <1424095816-4414-1-git-send-email-sudeep.holla@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424095816-4414-1-git-send-email-sudeep.holla@arm.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 Mon, Feb 16, 2015 at 02:10:16PM +0000, Sudeep Holla wrote: > On architectures that depend on DT for obtaining cache hierarcy, we need > to validate the device node for all the cache indices, failing to do so > might result in wrong information being exposed to the userspace. > > This is quite possible on initial/incomplete versions of the device > trees. In such cases, it's better to bail out if all the required device > nodes are not present. > > This patch adds checks for the validation of device node for all the > caches and doesn't initialise the cacheinfo if there's any error. > > Cc: Greg Kroah-Hartman > Reported-by: Mark Rutland > Signed-off-by: Sudeep Holla > --- > drivers/base/cacheinfo.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c > index 6e64563361f0..7015bf05c828 100644 > --- a/drivers/base/cacheinfo.c > +++ b/drivers/base/cacheinfo.c > @@ -62,15 +62,21 @@ static int cache_setup_of_node(unsigned int cpu) > return -ENOENT; > } > > - while (np && index < cache_leaves(cpu)) { > + while (index < cache_leaves(cpu)) { > this_leaf = this_cpu_ci->info_list + index; > if (this_leaf->level != 1) > np = of_find_next_cache_node(np); > else > np = of_node_get(np);/* cpu node itself */ > + if (!np) > + break; > this_leaf->of_node = np; > index++; > } > + > + if (index != cache_leaves(cpu)) /* not all OF nodes populated */ > + return -ENOENT; > + > return 0; > } > > @@ -189,8 +195,10 @@ static int detect_cache_attributes(unsigned int cpu) > * will be set up here only if they are not populated already > */ > ret = cache_shared_cpu_map_setup(cpu); > - if (ret) > + if (ret) { > + pr_err("failed to setup cache hierarcy from DT\n"); It would probably be better if this were something like: pr_warn("Unable to detect cache hierarcy from DT for CPU %d\n", cpu); Otherwise, this looks sane to me, and it would be nice to have this in ASAP so as to avoid exposing erroneous information to userspace. So: Acked-by: Mark Rutland Thanks, Mark. > goto free_ci; > + } > return 0; > > free_ci: > -- > 1.9.1 > >