linux-kernel - Re: [RFC PATCH 1/4] genirq: Provide basic NMI management for interrupt lines

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: <alpine.DEB.2.21.1807171114280.3238@nanos.tec.linutronix.de>
Date:   Tue, 17 Jul 2018 11:22:25 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Julien Thierry <julien.thierry@....com>
cc:     linux-kernel@...r.kernel.org,
        ricardo.neri-calderon@...ux.intel.com, marc.zyngier@....com,
        peterz@...radead.org, mingo@...nel.org
Subject: Re: [RFC PATCH 1/4] genirq: Provide basic NMI management for interrupt
 lines

On Mon, 9 Jul 2018, Julien Thierry wrote:
> +/**
> + *	disable_nmi_nosync - disable an nmi without waiting
> + *	@irq: Interrupt to disable
> + *
> + *	Disable the selected interrupt line. Disables and enables are
> + *	nested.
> + *	The interrupt to disable must have been requested through request_nmi.
> + *	Unlike disable_nmi(), this function does not ensure existing
> + *	instances of the IRQ handler have completed before returning.
> + */
> +void disable_nmi_nosync(unsigned int irq)
> +{
> +	disable_irq_nosync(irq);
> +}
> +
> +/**
> + *	disable_nmi - disable an nmi and wait for completion
> + *	@irq: Interrupt to disable
> + *
> + *	Disable the selected interrupt line. Enables and disables are
> + *	nested.
> + *	The interrupt to disable must have been requested through request_nmi.
> + *	This function waits for any pending IRQ handlers for this interrupt
> + *	to complete before returning. If you use this function while
> + *	holding a resource the IRQ handler may need you will deadlock.

I don't see how that wait would work with NMIs.

> +/* This function must be called with desc->lock held */
> +static const void *__cleanup_nmi(unsigned int irq, struct irq_desc *desc)
> +{
> +	const char *devname = NULL;
> +
> +	desc->istate &= ~IRQS_NMI;
> +
> +#ifdef CONFIG_SMP
> +	if (WARN_ON(desc->affinity_notify))
> +		desc->affinity_notify = NULL;
> +#endif

We should not support that affinity notify horror for NMIs at all.

> +	if (!WARN_ON(desc->action == NULL)) {
> +		irq_pm_remove_action(desc, desc->action);
> +		devname = desc->action->name;
> +		unregister_handler_proc(irq, desc->action);
> +
> +		kfree(desc->action);
> +		desc->action = NULL;
> +	}
> +
> +	irq_settings_clr_disable_unlazy(desc);
> +	irq_shutdown(desc);
> +
> +#ifdef CONFIG_SMP
> +	/* make sure affinity_hint is cleaned up */
> +	if (WARN_ON_ONCE(desc->affinity_hint))
> +		desc->affinity_hint = NULL;
> +#endif

Especially not twice :)

Thanks,

	tglx