From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965301AbeALUBI (ORCPT + 1 other); Fri, 12 Jan 2018 15:01:08 -0500 Received: from mail-it0-f65.google.com ([209.85.214.65]:41416 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965081AbeALUBG (ORCPT ); Fri, 12 Jan 2018 15:01:06 -0500 X-Google-Smtp-Source: ACJfBosJ8cSDXHLi2XbDOwJWtvTHXdwcngKGpvq7m3AZf785VdOUT/SN0OHfqvPH9CuLKRPcCmBt3FgpLTteVHXJcSM= MIME-Version: 1.0 In-Reply-To: References: <151571798296.27429.7166552848688034184.stgit@dwillia2-desk3.amr.corp.intel.com> <151571802258.27429.932636277047687877.stgit@dwillia2-desk3.amr.corp.intel.com> <20180112175109.yoz4jaaipztdj34k@treble> <20180112185815.meiwnnb5vmnrbsdt@treble> From: Linus Torvalds Date: Fri, 12 Jan 2018 12:01:04 -0800 X-Google-Sender-Auth: OwVSTmCq1LD4--bWX8E0C3q9_NY Message-ID: Subject: Re: [PATCH v2 07/19] x86: introduce __uaccess_begin_nospec and ASM_IFENCE To: Dan Williams Cc: Josh Poimboeuf , Linux Kernel Mailing List , linux-arch@vger.kernel.org, Andi Kleen , Kees Cook , kernel-hardening@lists.openwall.com, X86 ML , Ingo Molnar , Al Viro , "H. Peter Anvin" , Thomas Gleixner , Andrew Morton , 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 Fri, Jan 12, 2018 at 11:26 AM, Dan Williams wrote: > > By the time we get to de-reference uptr we know it is not pointing at > kernel memory, because access_ok would have failed and the cpu would > have waited for that failure result before doing anything else. I'm not actually convinced that's right in the original patches, exactly because of the issue that Josh pointed out: even if there is a comparison inside access_ok() that will be properly serialized, then that comparison can (and sometimes does) just cause a truth value to be generated, and then there might be *another* comparison of that return value after the lfence. And while the return value is table, the conditional branch on that comparison isn't. The new model of just doing it together with the STAC should be fine, though. I do think that it would be a good idea to very expressly document the fact that it's not that the user access itself is unsafe. I do agree that things like "get_user()" want to be protected, but not because of any direct bugs or problems with get_user() and friends, but simply because get_user() is an excellent source of a pointer that is obviously controlled from a potentially attacking user space. So it's a prime candidate for then finding _subsequent_ accesses that can then be used to perturb the cache. Linus From mboxrd@z Thu Jan 1 00:00:00 1970 MIME-Version: 1.0 Sender: linus971@gmail.com In-Reply-To: References: <151571798296.27429.7166552848688034184.stgit@dwillia2-desk3.amr.corp.intel.com> <151571802258.27429.932636277047687877.stgit@dwillia2-desk3.amr.corp.intel.com> <20180112175109.yoz4jaaipztdj34k@treble> <20180112185815.meiwnnb5vmnrbsdt@treble> From: Linus Torvalds Date: Fri, 12 Jan 2018 12:01:04 -0800 Message-ID: Content-Type: text/plain; charset="UTF-8" Subject: [kernel-hardening] Re: [PATCH v2 07/19] x86: introduce __uaccess_begin_nospec and ASM_IFENCE To: Dan Williams Cc: Josh Poimboeuf , Linux Kernel Mailing List , linux-arch@vger.kernel.org, Andi Kleen , Kees Cook , kernel-hardening@lists.openwall.com, X86 ML , Ingo Molnar , Al Viro , "H. Peter Anvin" , Thomas Gleixner , Andrew Morton , Alan Cox List-ID: On Fri, Jan 12, 2018 at 11:26 AM, Dan Williams wrote: > > By the time we get to de-reference uptr we know it is not pointing at > kernel memory, because access_ok would have failed and the cpu would > have waited for that failure result before doing anything else. I'm not actually convinced that's right in the original patches, exactly because of the issue that Josh pointed out: even if there is a comparison inside access_ok() that will be properly serialized, then that comparison can (and sometimes does) just cause a truth value to be generated, and then there might be *another* comparison of that return value after the lfence. And while the return value is table, the conditional branch on that comparison isn't. The new model of just doing it together with the STAC should be fine, though. I do think that it would be a good idea to very expressly document the fact that it's not that the user access itself is unsafe. I do agree that things like "get_user()" want to be protected, but not because of any direct bugs or problems with get_user() and friends, but simply because get_user() is an excellent source of a pointer that is obviously controlled from a potentially attacking user space. So it's a prime candidate for then finding _subsequent_ accesses that can then be used to perturb the cache. Linus