Received-SPF: pass (google.com: domain of linux-kernel+bounces-60368-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-144-ge5821d614e-fm-20240125.002-ge5821d61
Precedence: bulk
MIME-Version: 1.0
Message-Id: <4f0eb08d-bb5d-4544-aeba-ac9d8a663005@app.fastmail.com>
In-Reply-To: <202402091606.A181673F0A@keescook>
References: <20240208-alice-mm-v2-0-d821250204a6@google.com>
 <20240208-alice-mm-v2-2-d821250204a6@google.com>
 <202402091606.A181673F0A@keescook>
Date: Sat, 10 Feb 2024 12:07:14 +0100
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Kees Cook" <keescook@chromium.org>, "Alice Ryhl" <aliceryhl@google.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>, "Alex Gaynor" <alex.gaynor@gmail.com>,
 "Wedson Almeida Filho" <wedsonaf@gmail.com>,
 "Boqun Feng" <boqun.feng@gmail.com>, "Gary Guo" <gary@garyguo.net>,
 =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
 "Benno Lossin" <benno.lossin@proton.me>,
 "Andreas Hindborg" <a.hindborg@samsung.com>,
 "Alexander Viro" <viro@zeniv.linux.org.uk>,
 "Andrew Morton" <akpm@linux-foundation.org>,
 "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
 =?UTF-8?Q?Arve_Hj=C3=B8nnev=C3=A5g?= <arve@android.com>,
 "Todd Kjos" <tkjos@android.com>, "Martijn Coenen" <maco@android.com>,
 "Joel Fernandes" <joel@joelfernandes.org>,
 "Carlos Llamas" <cmllamas@google.com>,
 "Suren Baghdasaryan" <surenb@google.com>, linux-mm@kvack.org,
 linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
 "Christian Brauner" <brauner@kernel.org>
Subject: Re: [PATCH v2 2/4] uaccess: always export _copy_[from|to]_user with
 CONFIG_RUST
Content-Type: text/plain

On Sat, Feb 10, 2024, at 01:15, Kees Cook wrote:
> On Thu, Feb 08, 2024 at 03:47:52PM +0000, Alice Ryhl wrote:
>>  	unsigned long res = n;
>>  	might_fault();
>>  	if (!should_fail_usercopy() && likely(access_ok(from, n))) {
>> +		/*
>> +		 * Ensure that bad access_ok() speculation will not
>> +		 * lead to nasty side effects *after* the copy is
>> +		 * finished:
>> +		 */
>> +		barrier_nospec();
>
> This means all callers just gained this barrier. That's a behavioral
> change -- is it intentional here? I don't see it mentioned in the commit
> log.

My bad, I probably should have explained it when I did the
patch as this is very subtle:

The barrier_nospec() definition is a nop on everything other
than x86 and powerpc, but those two were using the out-of-line
version that did in fact use it.

After this patch, the out-of-line function calls the inline
function, so it needs to be added here to keep the behavior
unchanged on the architectures that need it. For the rest,
this change has no effect.

     Arnd