Re: [PATCH] x86: Pin task-stack in __get_wchan()

[Peter Zijlstra]

On Fri, Nov 19, 2021 at 10:35:44AM -0800, Josh Poimboeuf wrote:
> On Fri, Nov 19, 2021 at 10:29:47AM +0100, Peter Zijlstra wrote:
> > On Thu, Nov 18, 2021 at 06:04:27PM -0800, Josh Poimboeuf wrote:
> > > On Thu, Nov 18, 2021 at 01:11:09PM +0100, Peter Zijlstra wrote:
> > 
> > > > I now have the below, the only thing missing is that there's a
> > > > user_mode() call on a stack based regs. Now on x86_64 we can
> > > > __get_kernel_nofault() regs->cs and call it a day, but on i386 we have
> > > > to also fetch regs->flags.
> > > > 
> > > > Is this really the way to go?
> > > 
> > > Please no.  Can we just add a check in unwind_start() to ensure the
> > > caller did try_get_task_stack()?
> > 
> > I tried; but at best it's fundamentally racy and in practise its worse
> > because init_task doesn't seem to believe in refcounts and kthreads are
> > odd for some raisin. Now those are fixable, but given the fundamental
> > races, I don't see how it's ever going to be reliable.
> 
> I'm probably out of the loop here, but I wonder what races you're
> referring to.

We can do the warn as you suggest, however, it can become 0 right after
we test and then still make the unwder explode.

That is, the test is not sufficient.

> And I assume 'stack_refcount > 0' only needs to be asserted when
> unwinding other tasks, not current.  So it shouldn't affect unwinds
> during boot, or oopses.
> 
> Yes, the unwinder has to be rock solid, but if the caller can't even
> ensure the given task's memory exists, it sounds like a bug in the
> caller that needs a warning.

Well, yes. Still it would be nice if the unwinder would not itself burn
the house, even in that case.