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

Powered by blists - more mailing lists