linux-kernel - Re: [PATCH v3] x86/uaccess: Use pointer masking to limit uaccess speculation

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: <3619a590-6f1c-d6b6-f4e1-69239489c165@citrix.com>
Date:   Mon, 14 Sep 2020 20:34:25 +0100
From:   Andrew Cooper <andrew.cooper3@...rix.com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Borislav Petkov <bp@...en8.de>
CC:     Dan Williams <dan.j.williams@...el.com>, X86 ML <x86@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Will Deacon <will@...nel.org>,
        "Andrea Arcangeli" <aarcange@...hat.com>,
        Waiman Long <longman@...hat.com>,
        "Peter Zijlstra" <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Andy Lutomirski" <luto@...nel.org>,
        Christoph Hellwig <hch@....de>,
        David Laight <David.Laight@...lab.com>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v3] x86/uaccess: Use pointer masking to limit uaccess
 speculation

On 14/09/2020 20:27, Josh Poimboeuf wrote:
> On Mon, Sep 14, 2020 at 09:21:56PM +0200, Borislav Petkov wrote:
>> On Mon, Sep 14, 2020 at 11:48:55AM -0700, Dan Williams wrote:
>>>> Err, stupid question: can this macro then be folded into access_ok() so
>>>> that you don't have to touch so many places and the check can happen
>>>> automatically?
>>> I think that ends up with more changes because it changes the flow of
>>> access_ok() from returning a boolean to returning a modified user
>>> address that can be used in the speculative path.
>> I mean something like the totally untested, only to show intent hunk
>> below? (It is late here so I could very well be missing an aspect):
>>
>> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
>> index 2bffba2a1b23..c94e1589682c 100644
>> --- a/arch/x86/include/asm/uaccess.h
>> +++ b/arch/x86/include/asm/uaccess.h
>> @@ -7,6 +7,7 @@
>>  #include <linux/compiler.h>
>>  #include <linux/kasan-checks.h>
>>  #include <linux/string.h>
>> +#include <linux/nospec.h>
>>  #include <asm/asm.h>
>>  #include <asm/page.h>
>>  #include <asm/smap.h>
>> @@ -92,8 +93,15 @@ static inline bool pagefault_disabled(void);
>>   */
>>  #define access_ok(addr, size)					\
>>  ({									\
>> +	bool range;							\
>> +	typeof(addr) a = addr, b;					\
>> +									\
>>  	WARN_ON_IN_IRQ();						\
>> -	likely(!__range_not_ok(addr, size, user_addr_max()));		\
>> +									\
>> +	range = __range_not_ok(addr, size, user_addr_max());		\
>> +	b = (typeof(addr)) array_index_nospec((__force unsigned long)addr, TASK_SIZE_MAX); \
>> +									\
>> +	likely(!range && a == b);					\
> That's not going to work because 'a == b' can (and will) be
> misspeculated.

Correct.

array_index_nospec() only works (safety wise) when there are no
conditional jumps between it and the memory access it is protecting.

Otherwise, an attacker can just take control of that later jump and skip
the check that way.

~Andrew