linux-kernel - Re: [PATCH v9 28/42] x86/shstk: Add user-mode shadow stack support

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]

Message-ID: <6a2309340a17070fd39a1b049ce71188bfb5eba7.camel@intel.com>
Date:   Wed, 28 Jun 2023 00:37:22 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "broonie@...nel.org" <broonie@...nel.org>,
        "Hansen, Dave" <dave.hansen@...el.com>
CC:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "Xu, Pengfei" <pengfei.xu@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "kcc@...gle.com" <kcc@...gle.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "david@...hat.com" <david@...hat.com>,
        "Schimpe, Christina" <christina.schimpe@...el.com>,
        "Torvalds, Linus" <torvalds@...ux-foundation.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "pavel@....cz" <pavel@....cz>, "bp@...en8.de" <bp@...en8.de>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "john.allen@....com" <john.allen@....com>,
        "arnd@...db.de" <arnd@...db.de>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "bsingharora@...il.com" <bsingharora@...il.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>,
        "Yu, Yu-cheng" <yu-cheng.yu@...el.com>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "szabolcs.nagy@....com" <szabolcs.nagy@....com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "Eranian, Stephane" <eranian@...gle.com>
Subject: Re: [PATCH v9 28/42] x86/shstk: Add user-mode shadow stack support

On Tue, 2023-06-27 at 16:46 -0700, Dave Hansen wrote:
> > This looks generic, not even shadow stack specific - was there any
> > discussion of making it a vm_munmap_retry() (that's not a great
> > name...)
> > or similar?  I didn't see any in old versions of the thread but I
> > might've missed something.
> 
> Yeah, that looks odd.  Also odd is that none of the other users of
> vm_munmap() bother to check the return value (except for one
> passthrough
> in the nommu code).
> 
> I don't think the EINTR happens during contention, though.  It's really
> there to be able break out in the face of SIGKILL.  I think that's why
> nobody handles it: the task is dying anyway and nobody cares.
> 
> Rick, was this hunk here for a specific reason or were you just trying
> to be diligent in handling errors?

I'm not aware of any specific required cases. I think it is just pure
dilligence, originating from this comment:
https://lore.kernel.org/lkml/9847845a-749d-47a3-2a1d-bcc7c35f1bdd@intel.com/#t

I didn't see a need to remove it. 

The SIGKILL certaintly sounds like something more true than the
comment, but I can't do much of dive until next week. I would think we
need to handle EINTR differently to not WARN at least.

Yea, some of it does seem like the kind of thing that could live
outside of x86 shstk.c. But I'm not sure about the WARN part. That
should probably live in the caller. I guess someday we might even find
some shadow stack specific logic that could be cross-arch.