Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754808AbeAIV7K (ORCPT + 1 other); Tue, 9 Jan 2018 16:59:10 -0500 Received: from mail-oi0-f67.google.com ([209.85.218.67]:35259 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754296AbeAIV7F (ORCPT ); Tue, 9 Jan 2018 16:59:05 -0500 X-Google-Smtp-Source: ACJfBouvNwWFF8nZvNZiLj8U94nsW2G2rxxZysj3Sl46dt9hdOHh1gmvv+4M/Wpd6scIAcnTyf0uXq19Y44cFlZOVOs= MIME-Version: 1.0 In-Reply-To: <20180109214902.2d4ptkld2bof3js7@treble> References: <151520099201.32271.4677179499894422956.stgit@dwillia2-desk3.amr.corp.intel.com> <151520102670.32271.8447983009852138826.stgit@dwillia2-desk3.amr.corp.intel.com> <20180109214149.yo3tnjesezgz63x4@treble> <20180109214902.2d4ptkld2bof3js7@treble> From: Dan Williams Date: Tue, 9 Jan 2018 13:59:04 -0800 Message-ID: Subject: Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok To: Josh Poimboeuf Cc: Linus Torvalds , Linux Kernel Mailing List , linux-arch@vger.kernel.org, Andi Kleen , Arnd Bergmann , Greg Kroah-Hartman , Peter Zijlstra , Network Development , "the arch/x86 maintainers" , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Alan Cox Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 9, 2018 at 1:49 PM, Josh Poimboeuf wrote: > On Tue, Jan 09, 2018 at 01:47:09PM -0800, Dan Williams wrote: >> On Tue, Jan 9, 2018 at 1:41 PM, Josh Poimboeuf wrote: >> > On Fri, Jan 05, 2018 at 06:52:07PM -0800, Linus Torvalds wrote: >> >> On Fri, Jan 5, 2018 at 5:10 PM, Dan Williams wrote: >> >> > From: Andi Kleen >> >> > >> >> > When access_ok fails we should always stop speculating. >> >> > Add the required barriers to the x86 access_ok macro. >> >> >> >> Honestly, this seems completely bogus. >> >> >> >> The description is pure garbage afaik. >> >> >> >> The fact is, we have to stop speculating when access_ok() does *not* >> >> fail - because that's when we'll actually do the access. And it's that >> >> access that needs to be non-speculative. >> >> >> >> That actually seems to be what the code does (it stops speculation >> >> when __range_not_ok() returns false, but access_ok() is >> >> !__range_not_ok()). But the explanation is crap, and dangerous. >> > >> > The description also seems to be missing the "why", as it's not >> > self-evident (to me, at least). >> > >> > Isn't this (access_ok/uaccess_begin/ASM_STAC) too early for the lfence? >> > >> > i.e., wouldn't the pattern be: >> > >> > get_user(uval, uptr); >> > if (uval < array_size) { >> > lfence(); >> > foo = a2[a1[uval] * 256]; >> > } >> > >> > Shouldn't the lfence come much later, *after* reading the variable and >> > comparing it and branching accordingly? >> >> The goal of putting the lfence in uaccess_begin() is to prevent >> speculation past access_ok(). > > Right, but what's the purpose of preventing speculation past > access_ok()? Caution. It's the same rationale for the nospec_array_ptr() patches. If we, kernel community, can identify any possible speculation past a bounds check we should inject a speculation mitigation. Unless there's a way to be 100% certain that the first unwanted speculation can be turned into a gadget later on in the instruction stream, err on the side of shutting it down early.