From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S933710AbcIEObA (ORCPT <rfc822;w@1wt.eu>);
        Mon, 5 Sep 2016 10:31:00 -0400
Received: from foss.arm.com ([217.140.101.70]:58020 "EHLO foss.arm.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S933222AbcIEOa6 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 5 Sep 2016 10:30:58 -0400
Subject: Re: [PATCH 2/2] genirq: Use firmware identifier while adding domain
To: Punit Agrawal <punit.agrawal@arm.com>, tglx@linutronix.de,
        jiang.liu@linux.intel.com
References: <1464699409-23113-1-git-send-email-punit.agrawal@arm.com>
 <1464699409-23113-2-git-send-email-punit.agrawal@arm.com>
Cc: linux-kernel@vger.kernel.org
From: Marc Zyngier <marc.zyngier@arm.com>
X-Enigmail-Draft-Status: N1110
Organization: ARM Ltd
Message-ID: <57CD8194.2090306@arm.com>
Date: Mon, 5 Sep 2016 15:30:44 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Icedove/38.7.0
MIME-Version: 1.0
In-Reply-To: <1464699409-23113-2-git-send-email-punit.agrawal@arm.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 31/05/16 13:56, Punit Agrawal wrote:
> Use the firmware provided identifier for the domain name.
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  kernel/irq/irqdomain.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 1fe2fea..3af09e1 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -114,6 +114,10 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
>  	domain->hwirq_max = hwirq_max;
>  	domain->revmap_size = size;
>  	domain->revmap_direct_max_irq = direct_max;
> +	if (is_fwnode_irqchip(fwnode))
> +		domain->name = container_of(fwnode, struct irqchip_fwid, fwnode)->name;

So this thing worries me to no end. Look at the way irqchip_fwid is
constructed:

	name = kasprintf(GFP_KERNEL, "irqchip@%p", data);

You'd end-up disclosing a kernel address, which is not a very good idea.

> +	else
> +		domain->name = of_node_full_name(of_node);

And what if the node gets pruned (as it can happen on OpenFirmware
implementations)?

>  	irq_domain_check_hierarchy(domain);
>  
>  	mutex_lock(&irq_domain_mutex);
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...