linux-kernel - Re: [PATCH v6 6/8] x86/module: prepare module loading for ROX allocations of text

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: <ZxD0EVBoO-jcxEGE@kernel.org>
Date: Thu, 17 Oct 2024 14:25:05 +0300
From: Mike Rapoport <rppt@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Andreas Larsson <andreas@...sler.com>,
	Andy Lutomirski <luto@...nel.org>, Ard Biesheuvel <ardb@...nel.org>,
	Arnd Bergmann <arnd@...db.de>, Borislav Petkov <bp@...en8.de>,
	Brian Cain <bcain@...cinc.com>,
	Catalin Marinas <catalin.marinas@....com>,
	Christoph Hellwig <hch@...radead.org>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Dinh Nguyen <dinguyen@...nel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Guo Ren <guoren@...nel.org>, Helge Deller <deller@....de>,
	Huacai Chen <chenhuacai@...nel.org>, Ingo Molnar <mingo@...hat.com>,
	Johannes Berg <johannes@...solutions.net>,
	John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
	Kent Overstreet <kent.overstreet@...ux.dev>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Mark Rutland <mark.rutland@....com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Matt Turner <mattst88@...il.com>, Max Filippov <jcmvbkbc@...il.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Michal Simek <monstr@...str.eu>, Oleg Nesterov <oleg@...hat.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Richard Weinberger <richard@....at>,
	Russell King <linux@...linux.org.uk>, Song Liu <song@...nel.org>,
	Stafford Horne <shorne@...il.com>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Uladzislau Rezki <urezki@...il.com>,
	Vineet Gupta <vgupta@...nel.org>, Will Deacon <will@...nel.org>,
	bpf@...r.kernel.org, linux-alpha@...r.kernel.org,
	linux-arch@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-csky@...r.kernel.org, linux-hexagon@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
	linux-mips@...r.kernel.org, linux-mm@...ck.org,
	linux-modules@...r.kernel.org, linux-openrisc@...r.kernel.org,
	linux-parisc@...r.kernel.org, linux-riscv@...ts.infradead.org,
	linux-sh@...r.kernel.org, linux-snps-arc@...ts.infradead.org,
	linux-trace-kernel@...r.kernel.org, linux-um@...ts.infradead.org,
	linuxppc-dev@...ts.ozlabs.org, loongarch@...ts.linux.dev,
	sparclinux@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v6 6/8] x86/module: prepare module loading for ROX
 allocations of text

On Thu, Oct 17, 2024 at 11:35:15AM +0200, Peter Zijlstra wrote:
> On Wed, Oct 16, 2024 at 05:01:28PM -0400, Steven Rostedt wrote:
> > On Wed, 16 Oct 2024 15:24:22 +0300
> > Mike Rapoport <rppt@...nel.org> wrote:
> > 
> > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > > index 8da0e66ca22d..b498897b213c 100644
> > > --- a/arch/x86/kernel/ftrace.c
> > > +++ b/arch/x86/kernel/ftrace.c
> > > @@ -118,10 +118,13 @@ ftrace_modify_code_direct(unsigned long ip, const char *old_code,
> > >  		return ret;
> > >  
> > >  	/* replace the text with the new text */
> > > -	if (ftrace_poke_late)
> > > +	if (ftrace_poke_late) {
> > >  		text_poke_queue((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL);
> > > -	else
> > > -		text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
> > > +	} else {
> > > +		mutex_lock(&text_mutex);
> > > +		text_poke((void *)ip, new_code, MCOUNT_INSN_SIZE);
> > > +		mutex_unlock(&text_mutex);
> > > +	}
> > >  	return 0;
> > >  }
> > 
> > So this slows down the boot by over 30ms. That may not sound like much, but
> > we care very much about boot times. This code is serialized with boot and
> > runs whenever ftrace is configured in the kernel. The way I measured this,
> > was that I added:
> > 
> 
> > If this is only needed for module load, can we at least still use the
> > text_poke_early() at boot up?
> 
> Right, so I don't understand why this is needed at all.
> ftrace_module_init() runs before complete_formation() which normally
> switches to ROX, as such ftrace should be able to continue to do direct
> modifications here.

With this series the module text is allocated as ROX at the first place, so
the modifications ftrace does to module text have to either use text poking
even before complete_formation() or deal with a writable copy like I did
for relocations and alternatives.

I've been carrying the ftrace changes from a very old prototype and
didn't pay enough attention to them them until Steve's complaint.

I'll look into it.
 
> Which reminds me, at some point I did patches adding a
> MODULE_STATE_UNFORMED callback in order for static_call / jump_label to
> be able to avoid the expensive patching on module load as well (arguably
> ftrace should be using that too, instead of a custom callback).
> 

-- 
Sincerely yours,
Mike.