From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751371AbdKLO2I (ORCPT ); Sun, 12 Nov 2017 09:28:08 -0500 Received: from foss.arm.com ([217.140.101.70]:40320 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750953AbdKLO2G (ORCPT ); Sun, 12 Nov 2017 09:28:06 -0500 Date: Sun, 12 Nov 2017 14:28:00 +0000 From: Marc Zyngier To: Johan Hovold Cc: Thomas Gleixner , Jason Cooper , linux-kernel@vger.kernel.org, stable Subject: Re: [PATCH] irqchip/gic-v3: fix ppi-partitions lookup Message-ID: <20171112142800.0286fadf@why.wild-wind.fr.eu.org> In-Reply-To: <20171112134759.GP11226@localhost> References: <20171111165125.27944-1-johan@kernel.org> <20171112123208.759b9a9d@why.wild-wind.fr.eu.org> <20171112134759.GP11226@localhost> Organization: ARM Ltd X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 12 Nov 2017 14:47:59 +0100 Johan Hovold wrote: > On Sun, Nov 12, 2017 at 12:32:08PM +0000, Marc Zyngier wrote: > > On Sat, 11 Nov 2017 17:51:25 +0100 > > Johan Hovold wrote: > > > > Johan, > > > > > Fix child-node lookup during initialisation, which ended up searching > > > the whole device tree depth-first starting at the parent rather than > > > just matching on its children. > > > > > > To make things worse, the parent giq node was prematurely freed, while > > > > s/giq/gic/. > > > > Care to point out where that node would be prematurely freed? I don't > > see your patch addressing that either... > > of_find_node_by_name() is used for tree-wide searches and, as > documented, drops a reference to its first argument, which in this case > is the parent gic node. Got it. Yes, that's definitely a bad idea. > > > > the ppi-partitions node was leaked. > > > > > > Fixes: e3825ba1af3a ("irqchip/gic-v3: Add support for partitioned PPIs") > > > Cc: stable # 4.7 > > > > Do you have an example of this causing any trouble in the wild? As far > > as I remember, the whole of_node refcounting isn't really enforced, so > > while this is definitely a bug, it wouldn't cause any harm anywhere. > > Node refcounting is enabled with CONFIG_OF_DYNAMIC (e.g. when overlay > support is enabled) and getting the refcounting wrong can lead to all > sorts of issues like use-after-free and crashes. Ah, I completely forgot about this overlay madness. Fair enough, that's tricky enough to spot that it is worth plugging ASAP. I've queued this with a handful of other fixes for 4.15. Thanks, M. -- Without deviation from the norm, progress is not possible.