linux-kernel - Re: [PATCH v5 03/16] x86/alternatives: Disable LASS when patching kernel alternatives

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: <20241029113611.GS14555@noisy.programming.kicks-ass.net>
Date: Tue, 29 Oct 2024 12:36:11 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Andy Lutomirski <luto@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Xiongwei Song <xiongwei.song@...driver.com>,
	Xin Li <xin3.li@...el.com>, "Mike Rapoport (IBM)" <rppt@...nel.org>,
	Brijesh Singh <brijesh.singh@....com>,
	Michael Roth <michael.roth@....com>,
	Tony Luck <tony.luck@...el.com>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Alexey Kardashevskiy <aik@....com>,
	Jonathan Corbet <corbet@....net>,
	Sohil Mehta <sohil.mehta@...el.com>, Ingo Molnar <mingo@...nel.org>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
	Kai Huang <kai.huang@...el.com>,
	Sandipan Das <sandipan.das@....com>,
	Breno Leitao <leitao@...ian.org>,
	Rick Edgecombe <rick.p.edgecombe@...el.com>,
	Alexei Starovoitov <ast@...nel.org>, Hou Tao <houtao1@...wei.com>,
	Juergen Gross <jgross@...e.com>,
	Vegard Nossum <vegard.nossum@...cle.com>,
	Kees Cook <kees@...nel.org>, Eric Biggers <ebiggers@...gle.com>,
	Jason Gunthorpe <jgg@...pe.ca>,
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Yuntao Wang <ytcoode@...il.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Tejun Heo <tj@...nel.org>, Changbin Du <changbin.du@...wei.com>,
	Huang Shijie <shijie@...amperecomputing.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Namhyung Kim <namhyung@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-efi@...r.kernel.org
Subject: Re: [PATCH v5 03/16] x86/alternatives: Disable LASS when patching
 kernel alternatives

On Mon, Oct 28, 2024 at 10:49:07AM -0700, Dave Hansen wrote:
> On 10/28/24 09:07, Alexander Shishkin wrote:
> >  static void text_poke_memcpy(void *dst, const void *src, size_t len)
> >  {
> > -	memcpy(dst, src, len);
> > +	lass_stac();
> > +	__inline_memcpy(dst, src, len);
> > +	lass_clac();
> >  }
> >  
> >  static void text_poke_memset(void *dst, const void *src, size_t len)
> >  {
> >  	int c = *(const int *)src;
> >  
> > -	memset(dst, c, len);
> > +	lass_stac();
> > +	__inline_memset(dst, c, len);
> > +	lass_clac();
> >  }
> 
> These are the _only_ users of lass_stac/clac() or the new inlines.

For now; I have vague memories of running into trouble with compilers
doing random things with memcpy before, and having these inline versions
gives us more control.

One of the cases I remember running into was KASAN, where a compiler is
SUPPOSED to issue __asan_memcpy calls instead of the regular memcpy
calls, except they weren't all doing that, with the end result that our
regular memcpy implementation grew instrumentation to deal with that.

That got sorted -- by deprecating / breaking all those non-conformant
compilers. But still, I think it would be good to have the option to
force a simple inline memcpy when needed.

> First of all, I totally agree that the _existing_ strict objtool
> behavior around STAC/CLAC is a good idea.
> 
> But text poking really is special and the context is highly unlikely to
> result in bugs or exploits.  My first instinct here would have been to
> tell objtool that the text poking code is OK and to relax objtool's
> STAC/CLAC paranoia here.
> 
> Looking at objtool, I can see how important it is to keep the STAC/CLAC
> code as dirt simple and foolproof as possible.  I don't see an obvious
> way to except the text poking code without adding at least some complexity.
> 
> Basically what I'm asking for is if the goal is to keep objtool simple,
> please *SAY* that.  Because on the surface this doesn't look like a good
> idea.

There is, you can add it to uaccess_safe_builtin[], but I'm not sure we
want to blanked accept memcpy() -- or perhaps that is what you're
saying.

Anyway, looking at this, I see we grew rep_{movs,stos}_alternative, as
used in copy_user_generic() and __clear_user(). Which are all somewhat
similar.