Re: BUG-lockdep and freeze (was: Arrr! Linux 2.6.18)

Linus Torvalds
2006-09-30T21:56:48Z

On Sat, 30 Sep 2006, Andi Kleen wrote:
>
> Anyways, I guess we need even more validation in the fallback code,
> but just terminating the kernel thread stacks should fix that particular case.

Why not just add the simple validation?

A kernel stack is one page in size. If you move to another page, you
terminate. It's that simple.

What if the kernel stack is corrupt? Buffer overruns do that.

This patch seems to just paper over the _real_ problem, namely the fact
that the stack tracer code doesn't actually validate any of its arguments.

> When show_trace faults it is actually not the unwinder itself
> (which would be unwind()/processCFI() etc.), but fallback code
> which is actually just the old unwinder. So most of your accusations
> are hitting the wrong piece of code, Linus.

Ehh, that's not even _true_, Andi.

The old unwinder (well, at least for x86, and I assume x86-64 used that as
the beginning point) didn't have this problem at all, exactly because it
couldn't get on the wrong stack-page in the first place.

The old code literally had:

static inline int valid_stack_ptr(struct thread_info *tinfo, void *p)
{
return p > (void *)tinfo &&
p < (void *)tinfo + THREAD_SIZE - 3;
}

and would refuse to touch anything that wasn't in the stack page. It was
simple, AND WE NEVER _EVER_ HAD A BUG RELATED TO IT, AFAIK.

In contrast, the new code doesn't do any sanity checking at all, and this
is not the first or even the second time it causes problems.

So be honest here, don't try to shift the blame.

Linus