linux-kernel - Re: [PATCH] x86: Pin task-stack in __get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 22 Nov 2021 10:32:32 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Holger Hoffst??tte <holger@...lied-asynchrony.com>,
        Qi Zheng <zhengqi.arch@...edance.com>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Justin Forbes <jmforbes@...uxtx.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Shuah Khan <shuah@...nel.org>, patches@...nelci.org,
        lkft-triage@...ts.linaro.org, Pavel Machek <pavel@...x.de>,
        Jon Hunter <jonathanh@...dia.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH] x86: Pin task-stack in __get_wchan()

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.