linux-kernel - Re: [PATCH] ioapic: fix potential resume deadlock

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: <20110510073551.GH11595@elte.hu>
Date:	Tue, 10 May 2011 09:35:51 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Daniel J Blueman <daniel.blueman@...il.com>,
	Suresh Siddha <suresh.b.siddha@...el.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, H Peter Anvin <hpa@...or.com>,
	x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ioapic: fix potential resume deadlock


* Daniel J Blueman <daniel.blueman@...il.com> wrote:

> Fix a potential deadlock when resuming; here the calling function
> has disabled interrupts, so we cannot sleep.
> 
> Signed-off-by: Daniel J Blueman <daniel.blueman@...il.com>
> 
> ---
>  arch/x86/kernel/apic/io_apic.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 45fd33d..df63620 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -621,14 +621,14 @@ struct IO_APIC_route_entry **alloc_ioapic_entries(void)
>  	struct IO_APIC_route_entry **ioapic_entries;
>  
>  	ioapic_entries = kzalloc(sizeof(*ioapic_entries) * nr_ioapics,
> -				GFP_KERNEL);
> +				GFP_ATOMIC);
>  	if (!ioapic_entries)
>  		return 0;
>  
>  	for (apic = 0; apic < nr_ioapics; apic++) {
>  		ioapic_entries[apic] =
>  			kzalloc(sizeof(struct IO_APIC_route_entry) *
> -				nr_ioapic_registers[apic], GFP_KERNEL);
> +				nr_ioapic_registers[apic], GFP_ATOMIC);
>  		if (!ioapic_entries[apic])
>  			goto nomem;
>  	}

Hm, there must be some other bug here.

ioapic entries should be allocated on system bootup and should never really be 
deallocated.

The bug might be in the idea to call to enable_IR_x2apic() on resume - why are 
ioapic entries reallocated there? That call in default_setup_apic_routing() was 
introduced some time ago in:

  fa47f7e52874: x86, x2apic: Simplify apic init in SMP and UP builds

and that it results in reallocation in the suspend patch is distinctly wrong.

I suspect the reason why this has not triggered for others is the relative 
rarity of affected systems?

Suresh?

Thanks,

	Ingo
--
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/