Date:   Fri, 19 Nov 2021 10:35:44 -0800
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Holger Hoffst??tte <holger@applied-asynchrony.com>,
        Qi Zheng <zhengqi.arch@bytedance.com>,
        Kees Cook <keescook@chromium.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Justin Forbes <jmforbes@linuxtx.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Shuah Khan <shuah@kernel.org>, patches@kernelci.org,
        lkft-triage@lists.linaro.org, Pavel Machek <pavel@denx.de>,
        Jon Hunter <jonathanh@nvidia.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        stable <stable@vger.kernel.org>
Subject: Re: [PATCH] x86: Pin task-stack in __get_wchan()
Message-ID: <20211119183544.sragh42cn2liu3pw@treble>
References: <20211117101657.463560063@linuxfoundation.org>
 <YZV02RCRVHIa144u@fedora64.linuxtx.org>
 <55c7b316-e03d-9e91-d74c-fea63c469b3b@applied-asynchrony.com>
 <CAHk-=wjHbKfck1Ws4Y0pUZ7bxdjU9eh2WK0EFsv65utfeVkT9Q@mail.gmail.com>
 <20211118080627.GH174703@worktop.programming.kicks-ass.net>
 <20211118081852.GM174730@worktop.programming.kicks-ass.net>
 <YZYfYOcqNqOyZ8Yo@hirez.programming.kicks-ass.net>
 <YZZC3Shc0XA/gHK9@hirez.programming.kicks-ass.net>
 <20211119020427.2y5esq2czquwmvwc@treble>
 <YZduix64h64cDa7R@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <YZduix64h64cDa7R@hirez.programming.kicks-ass.net>
Precedence: bulk

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.

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.

-- 
Josh