linux-kernel - Re: [PATCH v2] gpio: interrupt consistency check for OF GPIO IRQs

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]

Date:	Thu, 22 Aug 2013 15:08:12 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Lars Poeschel <poeschel@...onage.de>
CC:	Tomasz Figa <tomasz.figa@...il.com>,
	Lars Poeschel <larsi@....tu-dresden.de>,
	grant.likely@...aro.org, linus.walleij@...aro.org,
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, mark.rutland@....com,
	ian.campbell@...rix.com, galak@...eaurora.org, pawel.moll@....com,
	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	Enric Balletbo i Serra <eballetbo@...il.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Kevin Hilman <khilman@...aro.org>,
	Balaji T K <balajitk@...com>,
	Tony Lindgren <tony@...mide.com>,
	Jon Hunter <jgchunter@...il.com>
Subject: Re: [PATCH v2] gpio: interrupt consistency check for OF GPIO IRQs

On 08/22/2013 03:01 AM, Lars Poeschel wrote:
> On Thursday 22 August 2013 at 01:10:27, Stephen Warren wrote:
>> On 08/21/2013 03:49 PM, Tomasz Figa wrote:
>>>> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c

>>>> +			 */
>>>> +			if (irq_domain && irq_domain->ops->xlate)
>>>> +				irq_domain->ops->xlate(irq_domain, gcn,
>>>> +						       intspec + i, intsize,
>>>> +						       &hwirq, &type);
>>>> +			else
>>>> +				hwirq = intspec[0];
>>>
>>> Is it a correct fallback when irq_domain is NULL?
>>
>> Indeed this fallback is dangerous. The /only/ way to parse an IRQ
>> specifier is with binding-specific knowledge, which is obtained by
>> calling irq_domain->ops->xlate(). If the IRQ domain can't be found, this
>> operation simply has to be deferred; we can't just guess and hope.
> 
> At least the of irq mapping code make this assumption also: 
> kernel/irq/irqdomain.c:483
> It should be valid for us here too.
> The additional assumption that I made is that if irq_domain == NULL (not 
> only xlate), that we can use intspec[0] either.

OK, I guess it's likely this won't cause any additional issue then. I
suspect most IRQ domains use within the context of device tree already
provide an explicit xlate op anyway; for example irq_domain_simple_ops
points at the default irq_domain_xlate_onetwocell.

>>>> +
>>>> +			hwirq = be32_to_cpu(hwirq);
>>>
>>> Is this conversion correct? I don't think hwirq could be big endian
>>> here (unless running on a big endian CPU).
>>
>> I think that should be inside the else branch above.
> 
> No it has to be in both branches as it is. Device tree data is big endian. 
> The conversion is converting big endian data (from device tree in both 
> cases) to cpu endianess and not coverting TO big endian.
> My test machine is a arm in little endian mode and it provided wrong values 
> if I did not do the conversion.
> What I am a bit unsure about is if the xlate function is expecting the 
> intspec pointer to point to big endian device tree data or data already 
> converted to cpu endianess. For the standard xlate functions 
> irq_domain_xlate_[one|two|onetwo]cell it does not matter.

The xlate function assumes that data is already converted to CPU-endian.
See:

irq_of_parse_and_map() ->
    of_irq_map_one() ->
        of_irq_map_raw() ->
            out_irq->specifier[i] = of_read_number(intspec +i, 1);
    irq_create_of_mapping()

(of_read_number does the be32_to_cpu() internally)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/