linux-kernel - Re: [PATCH 1/4] mfd: arizona: Add additional dummy IRQ callbacks

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] [day] [month] [year] [list]

Message-ID: <20140902142652.GE870@lee--X1>
Date:	Tue, 2 Sep 2014 15:26:52 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
Cc:	sameo@...ux.intel.com, patches@...nsource.wolfsonmicro.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] mfd: arizona: Add additional dummy IRQ callbacks

On Tue, 02 Sep 2014, Charles Keepax wrote:

> On Thu, Aug 21, 2014 at 12:56:31PM +0100, Lee Jones wrote:
> > On Tue, 12 Aug 2014, Charles Keepax wrote:
> > 
> > > We use a dummy IRQ chip to dispatch interrupts to the two seperate IRQ
> > > domains on the Arizona devices. Currently only the enable and disable
> > > callbacks are defined however, there are some situations where additional
> > > callbacks will be used from the IRQ core, which currently results in an
> > > NULL pointer deference. Add handlers for more of the IRQ callbacks and
> > > combine these into a single function since they are all identical.
> > > 
> > > Signed-off-by: Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
> > > ---
> > 
> > If you provide .irq_enable(), then .irq_unmask becomes redundant
> > and/or is checked for before invoking.  There is a chance of
> > .irq_mask() being called, but if this is a problem, it should be fixed
> > in the IRQ Chip code.  There is also one unprotected invocation of
> > .irq_ack(), but I think this should be fixed rather than forcing each
> > user of IRQ Chip to provide all of these call-backs.
> 
> So I have been looking at this further and going back over things
> from when the issue was discovered and it looks like it was
> probably the unprotected irq_ack call (in handle_edge_irq) that
> was the problem. But thinking about this more I am fairly
> convinced that the call is unprotected because it is expected that
> an edge IRQ will always have an ack, as it doesn't really make
> sense for an edge IRQ to not have an ack.
> 
> The IRQ chip here is just a software device to distribute the IRQ
> to the two sub-domains. As such I think the problem lies here:
> 
> 	irq_set_chip_and_handler(virq, &arizona_irq_chip, handle_edge_irq);
> 
> I am pretty sure the correct fix is to change this to a
> handle_simple_irq as it is just a software dummy and there is
> nothing really edge triggered about it. Do you see any problems
> with that as a solution?

I don't pretend to be an expert on the IRQ framework, but this
certainly looks a lot less hacky than your previous submission.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/