linux-kernel - Re: [PATCH v3 2/6] x86/uaccess: Avoid barrier_nospec() in 64-bit __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]

Message-ID: <20241121214011.iiup2fdwsys7hhts@jpoimboe>
Date: Thu, 21 Nov 2024 13:40:11 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: David Laight <David.Laight@...lab.com>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	Waiman Long <longman@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Ingo Molnar <mingo@...hat.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	Mark Rutland <mark.rutland@....com>,
	"Kirill A . Shutemov" <kirill@...temov.name>
Subject: Re: [PATCH v3 2/6] x86/uaccess: Avoid barrier_nospec() in 64-bit
 __get_user()

> On Fri, 15 Nov 2024 at 15:06, Josh Poimboeuf <jpoimboe@...nel.org> wrote:
> So I think the thing to do is
> 
>  (a) find out which __get_user() it is that matters so much for that load
> 
> Do you have a profile somewhere?
> 
>  (b) convert them to use "unsafe_get_user()", with that whole
> 
>                 if (can_do_masked_user_access())
>                         from = masked_user_access_begin(from);
>                 else if (!user_read_access_begin(from, sizeof(*from)))
>                         return -EFAULT;
> 
>      sequence before it.
> 
> And if it's just a single __get_user() (rather than a sequence of
> them), just convert it to get_user().
> 
> Hmm?

The profile is showing futex_get_value_locked():

int futex_get_value_locked(u32 *dest, u32 __user *from)
{
	int ret;

	pagefault_disable();
	ret = __get_user(*dest, from);
	pagefault_enable();

	return ret ? -EFAULT : 0;
}

That has several callers, so we can probably just use get_user() there?

Also, is there any harm in speeding up __get_user()?  It still has ~80
callers and it's likely to be slowing down things we don't know about.

It's usually only the regressions which get noticed, and that LFENCE
went in almost 7 years ago, when there was much less automated
performance regression testing.

As a bonus, that patch will root out any "bad" users, which will
eventually allow us to simplify things and just make __get_user() an
alias of get_user().

In fact, if we aliased it for all arches, that could help in getting rid
of __get_user() altogether as there would no longer be any (real or
advertised) benefit to using it.

-- 
Josh