linux-kernel - Re: [PATCH 1/4] regmap: irq: Fix lost interrupts by introducing handle

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:   Mon, 27 Mar 2017 18:49:52 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Tony Lindgren <tony@...mide.com>
Cc:     linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
        Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>,
        Lee Jones <lee.jones@...aro.org>,
        Marcel Partap <mpartap@....net>,
        Michael Scott <michael.scott@...aro.org>
Subject: Re: [PATCH 1/4] regmap: irq: Fix lost interrupts by introducing
 handle_reread

On Wed, Mar 22, 2017 at 10:10:49AM -0700, Tony Lindgren wrote:
> At least Motorola CPCAP PMIC needs it's device interrupts re-read
> until there are no more interrupts. Otherwise the PMIC interrupt to
> the SoC will eventually stop toggling. This seems to be a bug in the
> CPCAP PMIC where it can stop driving the GPIO interrupt to the SoC
> with pending CPCAP interrupts.

> Let's allow handling issues like this by introducing a flag for
> handle_reread and by splitting regmap_irq_thread() into two separate
> functions for regmap_read_irq_status() and regmap_irq_handle_pending().

So, I see your use case but the fact is that as Charles observed this is
exactly the code used for emulating level triggered IRQs with edge
triggered interrupt controllers.  This means someone is doubtless going
to end up using it for precisely that.  This makes me uncomfortable, we
do have this open coded into various drivers already but this is more of
a core thing and it feels like this should be in genirq rather than
here...  that said, looking at the code:

> +	do {
> +		ret = regmap_read_irq_status(data);
> +		if (ret)
> +			goto out_runtime_put;
> +
> +		ret = regmap_irq_handle_pending(data);
> +		if (ret < 0)
> +			goto out_runtime_put;
> +
> +		if (!ret)
> +			break;
> +
> +		handled += ret;
> +	} while (chip->handle_reread);

There's no protection against screaming interrupts here, I'd really like
to see that.  Also some tracing of the number of times we spin.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)