Skip to contentSkip to conversation

stack overflow safe kdump (i386) - safe_smp_processor_id

PATCH In review Fernando Luis Vazquez Cao 3 participants 4 messages Nov 28, 2005 – Nov 30, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/4 stack overflow safe kdump (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 (see code snips below). x86_64, on the other hand, is not affected by this problem because it benefits from the PDA infrastructure.

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

---

diff -urNp linux-2.6.15-rc2/arch/i386/kernel/smp.c linux-2.6.15-rc2-sov/arch/i386/kernel/smp.c
--- linux-2.6.15-rc2/arch/i386/kernel/smp.c 2005-10-28
09:02:08.000000000 +0900
+++ linux-2.6.15-rc2-sov/arch/i386/kernel/smp.c 2005-11-29
01:49:06.000000000 +0900
@@ -628,3 +628,25 @@ 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;
+
+ 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.15-rc2/include/asm-i386/smp.h linux-2.6.15-rc2-sov/include/asm-i386/smp.h
--- linux-2.6.15-rc2/include/asm-i386/smp.h 2005-11-29
01:47:17.000000000 +0900
+++ linux-2.6.15-rc2-sov/include/asm-i386/smp.h 2005-11-29
01:48:21.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 */

#1 lore raw
Quoted 5 lines · Fernando Luis Vazquez Cao

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

It's not fully safe, because a SMP kernel might run on a 32bit system without APIC. Then hard_smp_processor_id() would fault. (this cannot happen on x86-64)

You probably need to check one of the globals set by apic.c when its disabled.

-Andi

#2 lore raw
Quoted 7 lines · Andi Kleen
Quoted 5 lines · Fernando Luis Vazquez Cao

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

Quoted 6 lines

It's not fully safe, because a SMP kernel might run on a 32bit system without APIC. Then hard_smp_processor_id() would fault. (this cannot happen on x86-64)

You probably need to check one of the globals set by apic.c when its disabled.

Right. An SMP kernel on a uniprocessor, without apics.

To my knowledge all SMP systems that linux supports have apics.

Eric

#3 lore raw
Quoted 13 lines · Eric W. Biederman
Quoted 11 lines · Andi Kleen
Quoted 5 lines · Fernando Luis Vazquez Cao

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

It's not fully safe, because a SMP kernel might run on a 32bit system without APIC. Then hard_smp_processor_id() would fault. (this cannot happen on x86-64)

Quoted 7 lines
Quoted 2 lines

You probably need to check one of the globals set by apic.c when its disabled.

Right. An SMP kernel on a uniprocessor, without apics.

To my knowledge all SMP systems that linux supports have apics.

Thank you for the comments. I have modified safe_smp_processor_id so that it now checks whether APICs are enabled before using hard_smp_processor_id. Would this check suffice?

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;
}

I will be resending the stack overflow patches reflecting this change. They should apply cleanly against kernel 2.6.15-rc3. I will send the nmi handler-related patches separately once I have tested the code properly.

Regards,

Fernando