linux-kernel - Re: [PATCH] acpi: fix generic acpi_register

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:	Tue, 28 Apr 2015 18:08:44 +0100
From:	Marc Zyngier <marc.zyngier@....com>
To:	Mark Salter <msalter@...hat.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>
CC:	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"hanjun.guo@...aro.org" <hanjun.guo@...aro.org>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	Will Deacon <Will.Deacon@....com>
Subject: Re: [PATCH] acpi: fix generic acpi_register_gsi() handling of shared
 interrupts

On 28/04/15 17:51, Mark Salter wrote:
> If there are devices sharing an interrupt, acpi_register_gsi() could
> be called multiple times for the same gsi. Currently, it just maps
> the gsi without checking for a previous mapping. This patch adds a
> check for an existing mapping and uses that if found.
> 
> Cc: Hanjun Guo <hanjun.guo@...aro.org>
> Cc: Marc Zyngier <marc.zyngier@....com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> Cc: Will Deacon <will.deacon@....com>
> Signed-off-by: Mark Salter <msalter@...hat.com>
> ---
>  drivers/acpi/gsi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index 38208f2..5698c4f 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -76,6 +76,13 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>  	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
>  
>  	/*
> +	 * The gsi may be shared with other devices, so check for a
> +	 * previous mapping and use that if found.
> +	 */
> +	if (acpi_gsi_to_irq(gsi, &irq) > 0)
> +		return irq;

I'm afraid that's completely racy, and doesn't prevent much (two CPUs
can execute this at the same time and still end up executing the rest of
the code).

Furthermore, irq_create_mapping is *designed* to handle that situation,
and returns the same virtual IRQ for a given hwirq (and it has the
proper locking...).

So maybe it would be worth explaining the actual problem you're seeing,
because the code as it stands should handle the problem you seem to
describe.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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/