Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758398AbeAHXxV (ORCPT + 1 other); Mon, 8 Jan 2018 18:53:21 -0500 Received: from mail-oi0-f65.google.com ([209.85.218.65]:46428 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751366AbeAHXxS (ORCPT ); Mon, 8 Jan 2018 18:53:18 -0500 X-Google-Smtp-Source: ACJfBouPVJ0hMMNUifg+l60iUnQ9dImblzWtRCCpQWXgzIe3NhVtl+hqEWZJMkFH16E6HVjVAOvn9Ew0t4UCjmADEbE= MIME-Version: 1.0 In-Reply-To: References: <151520099201.32271.4677179499894422956.stgit@dwillia2-desk3.amr.corp.intel.com> <151520102670.32271.8447983009852138826.stgit@dwillia2-desk3.amr.corp.intel.com> From: Dan Williams Date: Mon, 8 Jan 2018 15:53:17 -0800 Message-ID: Subject: Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok To: Linus Torvalds Cc: 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 Mon, Jan 8, 2018 at 3:44 PM, Linus Torvalds wrote: > On Mon, Jan 8, 2018 at 1:09 PM, Dan Williams wrote: >> On Sat, Jan 6, 2018 at 5:20 PM, Linus Torvalds >> wrote: >>> On Sat, Jan 6, 2018 at 3:31 PM, Dan Williams wrote: >>>> >>>> I assume if we put this in uaccess_begin() we also need audit for >>>> paths that use access_ok but don't do on to call uaccess_begin()? A >>>> quick glance shows a few places where we are open coding the stac(). >>>> Perhaps land the lfence in stac() directly? >>> >>> Yeah, we should put it in uaccess_begin(), and in the actual user >>> accessor helpers that do stac. Some of them probably should be changed >>> to use uaccess_begin() instead while at it. >>> >>> One question for the CPU people: do we actually care and need to do >>> this for things that might *write* to something? The speculative write >>> obviously is killed, but does it perhaps bring in a cacheline even >>> when killed? >> >> As far as I understand a write could trigger a request-for-ownership >> read for the target cacheline. > > Oh, absolutely. > > I just wonder at what point that happens. > > Honestly, trying to get exclusive access to a cacheline can be _very_ > expensive (not just for the local thread), so I would actually expect > that doing so for speculative writes is actually bad for performance. > > That's doubly true because - unlike reads - there is no critical > latency issue, so trying to get the cache access started as early as > possible simply isn't all that important. > > So I suspect that a write won't actually try to allocate the cacheline > until the write has actually retired. > > End result: writes - unlike reads - *probably* will not speculatively > perturb the cache with speculative write addresses. > >> Even though writes can trigger reads, as far as I can see the write >> needs to be dependent on the first out-of-bounds read > > Yeah. A write on its own wouldn't matter, even if it were to perturb > the cache state, because the address already comes from user space, so > there's no new information in the cache perturbation for the attacker. > > But that all implies that we shouldn't need the lfence for the > "put_user()" case, only for the get_user() (where the value we read > would then perhaps be used to do another access). > > So we want to add the lfence (or "and") to get_user(), but not > necessarily put_user(). Yes, perhaps __uaccess_begin_get() and __uaccess_begin_put() to keep things separate? > Agreed? I've been thinking the "and" is only suitable for the array bounds check, for get_user() we're trying to block speculation past access_ok() at which point we can only do the lfence?