linux-kernel - Re: [PATCH] gpio/msm-v1: re-read IRQ flags on each iteration

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: <20120516010919.GA1295@codeaurora.org>
Date:	Tue, 15 May 2012 18:09:19 -0700
From:	David Brown <davidb@...eaurora.org>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Linus Walleij <linus.walleij@...ricsson.com>,
	linux-kernel@...r.kernel.org, Julia Lawall <julia.lawall@...6.fr>,
	Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH] gpio/msm-v1: re-read IRQ flags on each iteration

On Fri, May 11, 2012 at 11:41:12AM -0600, Grant Likely wrote:
> On Fri, 11 May 2012 14:07:12 +0200, Linus Walleij <linus.walleij@...ricsson.com> wrote:
> > From: Linus Walleij <linus.walleij@...aro.org>
> > 
> > We have recently found a number or erroneous IRQ handlers in
> > the kernel where the flag iterator loop miss IRQs that get
> > raised when the loop is executing. This was spotted in the
> > MSM v1 GPIO driver by Julia Lawall using this cocinelle
> > snippet:
> > 
> > @@
> > expression pending,gedr,e1;
> > statement S;
> > @@
> > 
> > *pending = readl(gedr);
> > ... when != pending = e1
> > while (pending) S
> > 
> > Reported-by: Julia Lawall <julia.lawall@...6.fr>
> > Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> 
> Thanks Linus,
> 
> I'll need an ack from an msm developer before applying this.

I though I saw Jeff Ohlstein reply, but I'm not finding that message.
I believe this patch is actually incorrect.  The register needs to
only be read once, and the bits walked through.  If another interrupt
comes in during the loop, the interrupt will remain asserted, and we
will re-enter the irq handler.  I suppose it could be wrapped in yet
another loop, but I'm not sure it is worth it.

I'm pretty sure the code works as it is, since it is the same as the
code in the Android kernel.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
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/