linux-kernel - Re: [PATCH 2/4] ARM: common/locomo: remove NO

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: <4485846.n2NSoAbukL@wuerfel>
Date:   Tue, 06 Sep 2016 16:50:02 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     linux-arm-kernel@...ts.infradead.org
Cc:     Russell King - ARM Linux <linux@...linux.org.uk>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] ARM: common/locomo: remove NO_IRQ check

On Tuesday, September 6, 2016 3:21:44 PM CEST Russell King - ARM Linux wrote:
> On Tue, Sep 06, 2016 at 03:53:28PM +0200, Arnd Bergmann wrote:
> > Since commit 489447380a29 ("[PATCH] handle errors returned by
> > platform_get_irq*()") ten years ago, the locomo driver refuses to
> > work without an interrupt line passed in its resources, so the
> > check for NO_IRQ is unnecessary.
> 
> This description is inaccurate and misleading (it looks like it was
> cut'n'pasted from patch 1.)
> 
> platform_get_irq() has nothing to do with your change, as your change
> is more about the irq_base value passed through platform data, and
> not through IRQ resources.

It was copied, but this part refers to this hunk

        irq = platform_get_irq(dev, 0);
        if (irq < 0)
                return -ENXIO;

from locomo_probe that was changed in the same patch as
the on in sa1111.c

> > We still check the irq_base argument for NO_IRQ, but as both

where the irq_base comes in.

I'll try to reword this to make it clearer.

> > @@ -387,7 +389,7 @@ __locomo_probe(struct device *me, struct resource *mem, int irq)
> >  
> >  	lchip->phys = mem->start;
> >  	lchip->irq = irq;
> > -	lchip->irq_base = (pdata) ? pdata->irq_base : NO_IRQ;
> > +	lchip->irq_base = pdata->irq_base;
> 
> This removes a NULL pointer check.  Before this change, a NULL pdata
> would be accepted and would lead to the interrupts not being setup.
> After this change, it results in a NULL pointer deference.
> 
> Thankfully, both collie and poodle supply platform data, and are the
> only providers of the locomo device.

Right, that is what I tried to say above. With the check I've added
in __locomo_probe, it would actually get the NULL pointer dereference
earlier than this line. I'll add back that check earlier in the function
and return an error in that case.

	Arnd