On Mon, 2006-07-10 at 22:04 +1000, Keith Owens wrote:
> Fernando Luis =?ISO-8859-1?Q?V=E1zquez?= Cao (on Mon, 10 Jul 2006 19:15:50 +0900) wrote:
> >Hi Keith,
> >
> >Thank you for the comments.
> >
> >On Mon, 2006-07-10 at 18:27 +1000, Keith Owens wrote:
> >> Fernando Luis Vazquez Cao (on Mon, 10 Jul 2006 16:50:52 +0900) 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.
> >> =
> >
> >> I agree with avoiding the use of thread_info when the stack might be
> >> corrupt. However your patch results in reading apic data and scanning
> >> NR_CPU sized tables for each IPI that is sent, which will slow down the
> >> sending of all IPIs, not just dump.
> >This patch only affects IPIs sent using send_IPI_allbutself which is
> >rarely called, so the impact in performance should be negligible.
>
> The main users of send_IPI_allbutself() are smp_call_function() and
> on_each_cpu(), which are used quite often. My main concern are the
> architectures that use IPI to flush TLB entries from other cpus. For
> example, i386 ioremap_page_range() -> flush_tlb_all() -> on_each_cpu().
>
> >> It would be far cheaper to define
> >> a per-cpu variable containing the logical cpu number, set that variable
> >> once as each cpu is brought up and just read it in cases where you
> >> might not trust the integrity of struct thread_info. safe_smp_processor_=
> >id()
> >> resolves to just a read of the per cpu variable.
> >But to read a per-cpu variable you need to index the corresponding array
> >with processor id of the current CPU (see code below), but that is
> >precisely what we are trying to figure out.
>
> Ouch, I am so used to ia64 where accessing the local per cpu variables
> is a direct read, with no need to use smp_processor_id().
>
> The use of smp_processor_id() in include/asm-generic/percpu.h is
> worrying, it means that any RAS code like dump or debuggers cannot
> access any per cpu variables. Corrupt the kernel stack and all per cpu
> variables become useless! That is a hidden bug, just waiting to bite
> all the RAS code.
Agreed. Some time ago I considered implementing something similar to
x86_64's PDA for i386 using the gs register. However, I discovered that
a similar approach had been discussed before and finally discarded,
because access to this segment register in i386 architectures was deemed
too slow.
Regards,
Fernando
>
> ia64, x86_64, power, s390, sparc64 do not suffer from this problem,
> they have efficient implementations of __get_cpu_var(). All other
> architectures (including i386) use the generic percpu code and per cpu
> variables will not work with corrupt kernel stacks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
