[PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Fernando Luis Vazquez Cao]

On the event of a stack overflow critical data that usually resides at
the bottom of the stack is likely to be stomped and, consequently, its
use should be avoided.

In particular, in the i386 and IA64 architectures the macro
smp_processor_id ultimately makes use of the "cpu" member of struct
thread_info which resides at the bottom of the stack. x86_64, on the
other hand, is not affected by this problem because it benefits from
the use of the PDA infrastructure.

To circumvent this problem I suggest implementing
"safe_smp_processor_id()" (it already exists in x86_64) for i386 and
IA64 and use it as a replacement for smp_processor_id in the reboot path
to the dump capture kernel. This is a possible implementation for i386.

Signed-off-by: Fernando Vazquez <fernando@intellilink.co.jp>
---

diff -urNp linux-2.6.16-rc1/arch/i386/kernel/smp.c linux-2.6.16-rc1-sov/arch/i386/kernel/smp.c
--- linux-2.6.16-rc1/arch/i386/kernel/smp.c	2006-01-03 12:21:10.000000000 +0900
+++ linux-2.6.16-rc1-sov/arch/i386/kernel/smp.c	2006-01-25 14:31:35.000000000 +0900
@@ -628,3 +628,28 @@ fastcall void smp_call_function_interrup
 	}
 }
 
+static int convert_apicid_to_cpu(int apic_id)
+{
+	int i;
+
+	for (i = 0; i < NR_CPUS; i++) {
+		if (x86_cpu_to_apicid[i] == apic_id)
+		return i;
+	}
+	return -1;
+}
+
+int safe_smp_processor_id(void) {
+	int apicid, cpuid;
+
+	if (!boot_cpu_has(X86_FEATURE_APIC))
+		return 0;
+
+	apicid = hard_smp_processor_id();
+	if (apicid == BAD_APICID)
+		return 0;
+
+	cpuid = convert_apicid_to_cpu(apicid);
+
+	return cpuid >= 0 ? cpuid : 0;
+}
diff -urNp linux-2.6.16-rc1/include/asm-i386/smp.h linux-2.6.16-rc1-sov/include/asm-i386/smp.h
--- linux-2.6.16-rc1/include/asm-i386/smp.h	2006-01-03 12:21:10.000000000 +0900
+++ linux-2.6.16-rc1-sov/include/asm-i386/smp.h	2006-01-25 14:31:35.000000000 +0900
@@ -90,12 +90,14 @@ static __inline int logical_smp_processo
 
 #endif
 
+extern int safe_smp_processor_id(void);
 extern int __cpu_disable(void);
 extern void __cpu_die(unsigned int cpu);
 #endif /* !__ASSEMBLY__ */
 
 #else /* CONFIG_SMP */
 
+#define safe_smp_processor_id() 0
 #define cpu_physical_id(cpu)		boot_cpu_physical_apicid
 
 #define NO_PROC_ID		0xFF		/* No processor magic marker */

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Andrew Morton]

Fernando Luis Vazquez Cao <fernando@intellilink.co.jp> wrote:
>
> On the event of a stack overflow critical data that usually resides at
> the bottom of the stack is likely to be stomped and, consequently, its
> use should be avoided.
> 
> In particular, in the i386 and IA64 architectures the macro
> smp_processor_id ultimately makes use of the "cpu" member of struct
> thread_info which resides at the bottom of the stack. x86_64, on the
> other hand, is not affected by this problem because it benefits from
> the use of the PDA infrastructure.
> 
> To circumvent this problem I suggest implementing
> "safe_smp_processor_id()" (it already exists in x86_64) for i386 and
> IA64 and use it as a replacement for smp_processor_id in the reboot path
> to the dump capture kernel. This is a possible implementation for i386.
> 
> ...
>
> +int safe_smp_processor_id(void) {

Please fix coding style.

>  #endif
>  
> +extern int safe_smp_processor_id(void);
>  extern int __cpu_disable(void);
>  extern void __cpu_die(unsigned int cpu);
>  #endif /* !__ASSEMBLY__ */
>  
>  #else /* CONFIG_SMP */
>  
> +#define safe_smp_processor_id() 0
>  #define cpu_physical_id(cpu)		boot_cpu_physical_apicid

It assumes that all x86 SMP machines have APICs.  That's untrue of Voyager.
I think we can probably live with this assumption - others would know
better than I.

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Andi Kleen]

On Wednesday 25 January 2006 08:10, Andrew Morton wrote:

> It assumes that all x86 SMP machines have APICs.  That's untrue of Voyager.
> I think we can probably live with this assumption - others would know
> better than I.

Early x86s didn't have APICs and they are still often disabled on not so 
old mobile CPUs.  I don't think it's a good assumption to make for i386.

-Andi

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Andrew Morton]

Andi Kleen <ak@suse.de> wrote:
>
> On Wednesday 25 January 2006 08:10, Andrew Morton wrote:
> 
> > It assumes that all x86 SMP machines have APICs.  That's untrue of Voyager.
> > I think we can probably live with this assumption - others would know
> > better than I.
> 
> Early x86s didn't have APICs and they are still often disabled on not so 
> old mobile CPUs.  I don't think it's a good assumption to make for i386.
> 

But how many of those do SMP?

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Arjan van de Ven]

On Tue, 2006-01-24 at 23:59 -0800, Andrew Morton wrote:
> Andi Kleen <ak@suse.de> wrote:
> >
> > On Wednesday 25 January 2006 08:10, Andrew Morton wrote:
> > 
> > > It assumes that all x86 SMP machines have APICs.  That's untrue of Voyager.
> > > I think we can probably live with this assumption - others would know
> > > better than I.
> > 
> > Early x86s didn't have APICs and they are still often disabled on not so 
> > old mobile CPUs.  I don't think it's a good assumption to make for i386.
> > 
> 
> But how many of those do SMP?

even on SMP boxes you regularly need to (runtime) disable apics. Several
boards out there just have busted apics, or at least when used with
linux. "noapic" is one of the more frequent things distro support people
tell customers over the phone....

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Fernando Luis Vazquez Cao]

On Wed, 2006-01-25 at 09:07 +0100, Arjan van de Ven wrote:
> On Tue, 2006-01-24 at 23:59 -0800, Andrew Morton wrote:
> > Andi Kleen <ak@suse.de> wrote:
> > >
> > > On Wednesday 25 January 2006 08:10, Andrew Morton wrote:
> > > 
> > > > It assumes that all x86 SMP machines have APICs.  That's untrue of Voyager.
> > > > I think we can probably live with this assumption - others would know
> > > > better than I.
> > > 
> > > Early x86s didn't have APICs and they are still often disabled on not so 
> > > old mobile CPUs.  I don't think it's a good assumption to make for i386.
> > > 
> > 
> > But how many of those do SMP?
> 
> even on SMP boxes you regularly need to (runtime) disable apics. Several
> boards out there just have busted apics, or at least when used with
> linux. "noapic" is one of the more frequent things distro support people
> tell customers over the phone....
Checking whether ioapic_setup_disabled is set should suffice, right?
Does the patch below look good?

diff -urNp linux-2.6.16-rc1/arch/i386/kernel/smp.c linux-2.6.16-rc1-sov/arch/i386/kernel/smp.c
--- linux-2.6.16-rc1/arch/i386/kernel/smp.c	2006-01-03 12:21:10.000000000 +0900
+++ linux-2.6.16-rc1-sov/arch/i386/kernel/smp.c	2006-01-25 17:49:52.000000000 +0900
@@ -628,3 +628,32 @@ fastcall void smp_call_function_interrup
 	}
 }
 
+static int convert_apicid_to_cpu(int apic_id)
+{
+	int i;
+
+	for (i = 0; i < NR_CPUS; i++) {
+		if (x86_cpu_to_apicid[i] == apic_id)
+		return i;
+	}
+	return -1;
+}
+
+int safe_smp_processor_id(void)
+{
+	int apicid, cpuid;
+
+	if (ioapic_setup_disabled())
+		return smp_processor_id();
+
+	if (!boot_cpu_has(X86_FEATURE_APIC))
+		return 0;
+
+	apicid = hard_smp_processor_id();
+	if (apicid == BAD_APICID)
+		return 0;
+
+	cpuid = convert_apicid_to_cpu(apicid);
+
+	return cpuid >= 0 ? cpuid : 0;
+}
diff -urNp linux-2.6.16-rc1/include/asm-i386/smp.h linux-2.6.16-rc1-sov/include/asm-i386/smp.h
--- linux-2.6.16-rc1/include/asm-i386/smp.h	2006-01-03 12:21:10.000000000 +0900
+++ linux-2.6.16-rc1-sov/include/asm-i386/smp.h	2006-01-25 18:00:04.000000000 +0900
@@ -90,12 +90,14 @@ static __inline int logical_smp_processo
 
 #endif
 
+extern int safe_smp_processor_id(void);
 extern int __cpu_disable(void);
 extern void __cpu_die(unsigned int cpu);
 #endif /* !__ASSEMBLY__ */
 
 #else /* CONFIG_SMP */
 
+#define safe_smp_processor_id() 0
 #define cpu_physical_id(cpu)		boot_cpu_physical_apicid
 
 #define NO_PROC_ID		0xFF		/* No processor magic marker */

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Andi Kleen]

On Wednesday 25 January 2006 08:59, Andrew Morton wrote:
> Andi Kleen <ak@suse.de> wrote:
> > On Wednesday 25 January 2006 08:10, Andrew Morton wrote:
> > > It assumes that all x86 SMP machines have APICs.  That's untrue of
> > > Voyager. I think we can probably live with this assumption - others
> > > would know better than I.
> >
> > Early x86s didn't have APICs and they are still often disabled on not so
> > old mobile CPUs.  I don't think it's a good assumption to make for i386.
>
> But how many of those do SMP?

The SMP kernel should still run on those. Even x86-64 had special code
for this.

-Andi

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> It assumes that all x86 SMP machines have APICs.  That's untrue of Voyager.
> I think we can probably live with this assumption - others would know
> better than I.

So looking at the code hard_smp_processor_id is fine.  Voyager
also implements that.

If we are running UP with an SMP kernel we are fine.

But I think x86_cpu_to_apicid will get us into trouble
on Voyager, because I don't think we should compile smpboot.c
as it has conflicting simples with voyager_smp.c

Although reading the makefile I don't see how we can avoid
compiling them both in SMP mode.

Everything else has a local apic when running SMP so we should
be good there.

Eric

Re: [PATCH 1/5] stack overflow safe kdump (2.6.16-rc1-i386) - safe_smp_processor_id

[Eric W. Biederman]

Fernando Luis Vazquez Cao <fernando@intellilink.co.jp> writes:

> On Wed, 2006-01-25 at 09:07 +0100, Arjan van de Ven wrote:
>> On Tue, 2006-01-24 at 23:59 -0800, Andrew Morton wrote:
>> > Andi Kleen <ak@suse.de> wrote:
>> > >
>> > > On Wednesday 25 January 2006 08:10, Andrew Morton wrote:
>> > > 
>> > > > It assumes that all x86 SMP machines have APICs.  That's untrue of
> Voyager.
>> > > > I think we can probably live with this assumption - others would know
>> > > > better than I.
>> > > 
>> > > Early x86s didn't have APICs and they are still often disabled on not so 
>> > > old mobile CPUs.  I don't think it's a good assumption to make for i386.
>> > > 
>> > 
>> > But how many of those do SMP?
>> 
>> even on SMP boxes you regularly need to (runtime) disable apics. Several
>> boards out there just have busted apics, or at least when used with
>> linux. "noapic" is one of the more frequent things distro support people
>> tell customers over the phone....
> Checking whether ioapic_setup_disabled is set should suffice, right?
> Does the patch below look good?

Actually hard_smp_processor_id should only care about local apics.
Disabling the io_apics should have no affect, on this code path.

So I believe testing for io_apics being enabled is actively wrong.

If the local apics are disabled the feature flag should be cleared,
and we are uniprocessor anyway.

Eric