linux-kernel - Re: [PATCH 06/15] irqdomain: Ensure type settings match for an existing mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56EAF4E1.5020201@nvidia.com>
Date:	Thu, 17 Mar 2016 18:18:09 +0000
From:	Jon Hunter <jonathanh@...dia.com>
To:	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	Marc Zyngier <marc.zyngier@....com>,
	Benoît Cousson <bcousson@...libre.com>,
	Tony Lindgren <tony@...mide.com>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	"Kumar Gala" <galak@...eaurora.org>,
	Stephen Warren <swarren@...dotorg.org>,
	"Thierry Reding" <thierry.reding@...il.com>
CC:	Kevin Hilman <khilman@...nel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Grygorii Strashko <grygorii.strashko@...com>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	<linux-tegra@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 06/15] irqdomain: Ensure type settings match for an
 existing mapping


On 17/03/16 14:19, Jon Hunter wrote:
> When mapping an IRQ, if a mapping already exists, then we simply return
> the virtual IRQ number. However, we do not check that the type settings
> for the existing mapping match those for the mapping that is about to be
> created. It may be unlikely that the type settings would not match, but
> check for this and don't return a valid IRQ mapping if the type settings
> do not match.
> 
> WARN if the type return by irq_domain_translate() has bits outside the
> sense mask set and then clear these bits. If these bits are not cleared
> then this will cause the comparision of the type settings for an
> existing mapping to fail with that of the new mapping even if the sense
> bit themselves match. The reason being is that the existing type
> settings are read by calling irq_get_trigger_type() which will clear
> any bits outside the sense mask. This will allow us to detect irqchips
> that are not correctly clearing these bits and fix them.
> 
> Signed-off-by: Jon Hunter <jonathanh@...dia.com>
> ---
>  kernel/irq/irqdomain.c | 59 ++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 43 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 3a519a01118b..0ea285baa619 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -549,6 +549,13 @@ static int irq_domain_translate(struct irq_domain *d,
>  				     fwspec->param, fwspec->param_count,
>  				     hwirq, type);
>  
> +	/*
> +	 * WARN if the irqchip returns a type with bits
> +	 * outside the sense mask set and clear these bits.
> +	 */
> +	if (WARN_ON(*type & ~IRQ_TYPE_SENSE_MASK))
> +		*type &= IRQ_TYPE_SENSE_MASK;
> +
>  	/* If domain has no translation, then we assume interrupt line */
>  	*hwirq = fwspec->param[0];
>  	return 0;
> @@ -570,6 +577,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>  {
>  	struct irq_domain *domain;
>  	irq_hw_number_t hwirq;
> +	unsigned int cur_type = IRQ_TYPE_NONE;
>  	unsigned int type = IRQ_TYPE_NONE;
>  	int virq;
>  
> @@ -592,23 +600,42 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>  	if (irq_domain_translate(domain, fwspec, &hwirq, &type))
>  		return 0;
>  
> -	if (irq_domain_is_hierarchy(domain)) {
> -		/*
> -		 * If we've already configured this interrupt,
> -		 * don't do it again, or hell will break loose.
> -		 */
> -		virq = irq_find_mapping(domain, hwirq);
> -		if (virq)
> -			return virq;
> -
> -		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec);
> -		if (virq <= 0)
> -			return 0;
> +	/*
> +	 * If we've already configured this interrupt,
> +	 * don't do it again, or hell will break loose.
> +	 */
> +	virq = irq_find_mapping(domain, hwirq);
> +	if (!virq) {
> +		if (irq_domain_is_hierarchy(domain)) {
> +			virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE,
> +						     fwspec);
> +			if (virq <= 0)
> +				return 0;
> +		} else {
> +			/* Create mapping */
> +			virq = irq_create_mapping(domain, hwirq);
> +			if (!virq)
> +				return virq;
> +		}
>  	} else {
> -		/* Create mapping */
> -		virq = irq_create_mapping(domain, hwirq);
> -		if (!virq)
> -			return virq;
> +		cur_type = irq_get_trigger_type(virq);
> +	}
> +
> +	/*
> +	 * If the trigger type is not specified or matches the current
> +	 * trigger type then we are done so return the interrupt number.
> +	 */
> +	if (type == IRQ_TYPE_NONE || type == cur_type)
> +		return virq;
> +
> +	/*
> +	 * If the trigger type is already set and does
> +	 * not match this interrupt, then return 0.
> +	 */
> +	if (cur_type != IRQ_TYPE_NONE) {
> +		pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n",
> +			hwirq, of_node_full_name(to_of_node(fwspec->fwnode)));
> +		return 0;

If we created a new mapping and return here, then this will leak the
mapping. I will fix this. Sorry should have caught this before!

Jon