Re: [PATCH 3/3] reliable stack trace support (i386)

[Ingo Molnar <mingo@elte.hu>]

* Jan Beulich <jbeulich@novell.com> wrote:

> These are the i386-specific pieces to enable reliable stack traces. This is
> going to be even more useful once CFI annotations get added to he assembly
> code, namely to entry.S (a patch for that had been submitted several times).

> +#define UNW_PC(frame) (frame)->regs.eip
> +#define UNW_SP(frame) (frame)->regs.esp
> +#ifdef CONFIG_FRAME_POINTER
> +#define UNW_FP(frame) (frame)->regs.ebp
> +#define FRAME_RETADDR_OFFSET 4
> +#define FRAME_LINK_OFFSET 0
> +#define STACK_BOTTOM(tsk) (((tsk)->thread.esp0 - 1) & ~(THREAD_SIZE - 1))
> +#define STACK_TOP(tsk) ((tsk)->thread.esp0)
> +#endif

style: i'd suggest to improve readability by aligning these.

> +static inline void
> +arch_unw_init_blocked(struct unwind_frame_info *info)
> +{
> +	memset(&info->regs, 0, sizeof(info->regs));
> +	info->regs.eip = info->task->thread.eip;
> +	info->regs.xcs = __KERNEL_CS;
> +	__get_user(info->regs.ebp, (long *)info->task->thread.esp);
> +	info->regs.esp = info->task->thread.esp;
> +	info->regs.xss = __KERNEL_DS;
> +	info->regs.xds = __USER_DS;
> +	info->regs.xes = __USER_DS;

hm, arent you using this from within show_trace()? In that case we 
shouldnt do a __get_user() i think, we might be in an arbitrary context 
...

> +static inline int
> +arch_unw_user_mode(const struct unwind_frame_info *info)
> +{
> +#if 0 /* This can only work when selector register and EFLAGS saves/restores
> +         are properly annotated (and tracked in UNW_REGISTER_INFO). */
> +	return user_mode_vm(&info->regs);
> +#else
> +	return info->regs.eip < PAGE_OFFSET;
> +#endif

same here as for x86_64: is this condition safe? Userspace can provoke 
an EIP of >= PAGE_OFFSET by for example jumping to the vsyscall page.

	Ingo