linux-kernel - Re: [GIT PULL] x86/shstk for 6.4

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: <ad5b44e17c1c17ebdc581169fec7e80f7ef2a4d4.camel@intel.com>
Date:   Sun, 7 May 2023 00:10:29 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "Torvalds, Linus" <torvalds@...ux-foundation.org>,
        "Hansen, Dave" <dave.hansen@...el.com>
CC:     "keescook@...omium.org" <keescook@...omium.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [GIT PULL] x86/shstk for 6.4

On Sat, 2023-05-06 at 12:34 -0700, Linus Torvalds wrote:
> > > > End result: all those architectures that do *not* want the vma
> > > > argument don't need to do any extra work, and they just
> > > > implement > > the
> > > > old version, and the only thing that happened was that it was >
> > > > > > renamed.
> > > > 
> > > > Because I really don't want to pull this series as-is, when I
> > > > found
> > > > what looks like a "this broke an architecture that DOES NOT
> > > > EVEN > > > CARE"
> > > > bug in the series.
> > > > 
> > > > And yes, my bad for not getting to this earlier to notice this.
> > > > 
> > > > Or alternatively - your bad for not going through this with a
> > > > fine
> > > > comb like I started doing.

Oof, yes that definitely looks like a bug. Yes, the ifdef solution
would be a less error prone way to pull off the addition of the VMA. 

I think I did try something like your suggestion during development. My
(maybe misguided) concern was that pte_mkwrite_kernel() might not make
semantic sense for all architectures since not all architectures were
using pte_mkwrite() on kernel memory. Like I know a lot of
architectures don't do read-only memory in the kernel even though they
do it in userspace.

I also think it would still be leaving things in a slightly worse place
than we started by having the actual guts of the pte_mkwrite()'s harder
to grep for.

I'm surprised this was missed in automated testing, since the
consequence of breaking these should have been pretty immediately
obvious. Between that and all the times myself and others looked at it
and still failed, maybe the less error prone solution is better.