stack overflow safe kdump (2.6.18-rc1-i386) - safe_smp_processor_id

PATCH In review Fernando Luis Vázquez Cao 2 participants 3 messages Jul 11, 2006 – Jul 12, 2006

Share & tools

Fernando Luis Vázquez Cao

author

2006-07-11 06:05 UTC #0 lore raw

PATCH 1/4 stack overflow safe kdump (2.6.18-rc1-i386) - safe_smp_processor_id

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 <[email protected]>

---

arch/i386/kernel/smp.c +26 -0

diff -urNp linux-2.6.18-rc1/arch/i386/kernel/smp.c linux-2.6.18-rc1-sof/arch/i386/kernel/smp.c
@@ -634,3 +634,29 @@ 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;
+}

include/asm-i386/smp.h +2 -0

diff -urNp linux-2.6.18-rc1/include/asm-i386/smp.h linux-2.6.18-rc1-sof/include/asm-i386/smp.h
@@ -89,12 +89,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 */

Andrew Morton

2006-07-12 06:47 UTC #1 lore raw

Quoted 1 line · On Tue, 11 Jul 2006 15:05:18 +0900 Fernando Luis Vázquez Cao…

...

With CONFIG_SMP=n:

arch/i386/kernel/crash.c: In function 'crash_save_self':
arch/i386/kernel/crash.c:91: warning: implicit declaration of function 'safe_smp_processor_id'

And it fails to link.

Quoted 7 lines

include/asm-i386/smp.h +1 -0

@@ -89,12 +89,14 @@ static __inline int logical_smp_processo
 
 #endif
 
+extern int safe_smp_processor_id(void);

include/asm-i386/smp.h static __inline int logical_smp_processo · line 102

Quoted 10 lines

include/asm-i386/smp.h +1 -0

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

The reason for this is that include/linux/smp.h only includes asm/smp.h if CONFIG_SMP=y. This is not the cleverest thing we've ever done.

I fixed that in cowardly fashion:

arch/i386/kernel/crash.c renamed from arch/i386/kernel/crash.c~stack-overflow-safe-kdump-crash_use_safe_smp_processor_id-fix +1 -0

@@ -23,6 +23,7 @@
 #include <asm/hw_irq.h>
 #include <asm/apic.h>
 #include <asm/kdebug.h>
+#include <asm/smp.h>
 
 #include <mach_ipi.h>

Fernando Luis Vázquez Cao

author

2006-07-12 06:59 UTC #2 lore raw

Quoted 10 lines · Andrew Morton

Quoted 1 line · On Tue, 11 Jul 2006 15:05:18 +0900 Fernando Luis Vázquez Cao…
...
With CONFIG_SMP=n:
arch/i386/kernel/crash.c: In function 'crash_save_self':
arch/i386/kernel/crash.c:91: warning: implicit declaration of function 'safe_smp_processor_id'
And it fails to link.

Thank you for catching this! I had only tried i386-SMP and voyager-SMP configurations. I will try several UP configurations and see if it compiles properly.

Sorry for the trouble.

- Fernando

Quoted 36 lines

Quoted 7 lines
include/asm-i386/smp.h +1 -0
@@ -89,12 +89,14 @@ static __inline int logical_smp_processo
 
 #endif
 
+extern int safe_smp_processor_id(void);
include/asm-i386/smp.h static __inline int logical_smp_processo · line 102
Quoted 10 lines
include/asm-i386/smp.h +1 -0
 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 */
The reason for this is that include/linux/smp.h only includes asm/smp.h if CONFIG_SMP=y. This is not the cleverest thing we've ever done.
I fixed that in cowardly fashion:
arch/i386/kernel/crash.c renamed from arch/i386/kernel/crash.c~stack-overflow-safe-kdump-crash_use_safe_smp_processor_id-fix +1 -0
@@ -23,6 +23,7 @@
 #include <asm/hw_irq.h>
 #include <asm/apic.h>
 #include <asm/kdebug.h>
+#include <asm/smp.h>
 
 #include <mach_ipi.h>
 
_