linux-kernel - Re: [PATCH v2 3/9] x86: refcount: prevent gcc distortions

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]

Date:   Mon, 4 Jun 2018 22:20:50 +0000
From:   Nadav Amit <namit@...are.com>
To:     Kees Cook <keescook@...omium.org>
CC:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Jan Beulich <JBeulich@...e.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH v2 3/9] x86: refcount: prevent gcc distortions

Kees Cook <keescook@...omium.org> wrote:

> On Mon, Jun 4, 2018 at 4:21 AM, Nadav Amit <namit@...are.com> wrote:
>> GCC considers the number of statements in inlined assembly blocks,
>> according to new-lines and semicolons, as an indication to the cost of
>> the block in time and space. This data is distorted by the kernel code,
>> which puts information in alternative sections. As a result, the
>> compiler may perform incorrect inlining and branch optimizations.
>> 
>> The solution is to set an assembly macro and call it from the inlined
>> assembly block. As a result GCC considers the inline assembly block as
>> a single instruction.
>> 
>> This patch allows to inline functions such as __get_seccomp_filter().
>> Interestingly, this allows more aggressive inlining while reducing the
>> kernel size.
>> 
>>   text    data     bss     dec     hex filename
>> 18140970 10225412 2957312 31323694 1ddf62e ./vmlinux before
>> 18140140 10225284 2957312 31322736 1ddf270 ./vmlinux after (-958)
>> 
>> Static text symbols:
>> Before: 40302
>> After:  40286   (-16)
>> 
>> Functions such as kref_get(), free_user(), fuse_file_get() now get
>> inlined.
>> 
>> Cc: Thomas Gleixner <tglx@...utronix.de>
>> Cc: Ingo Molnar <mingo@...hat.com>
>> Cc: "H. Peter Anvin" <hpa@...or.com>
>> Cc: x86@...nel.org
>> Cc: Kees Cook <keescook@...omium.org>
>> Cc: Jan Beulich <JBeulich@...e.com>
>> Cc: Josh Poimboeuf <jpoimboe@...hat.com>
>> 
>> Signed-off-by: Nadav Amit <namit@...are.com>
>> ---
>> arch/x86/include/asm/refcount.h | 73 ++++++++++++++++++++-------------
>> arch/x86/kernel/macros.S        |  1 +
>> 2 files changed, 45 insertions(+), 29 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/refcount.h b/arch/x86/include/asm/refcount.h
>> index 4cf11d88d3b3..53462f32b58e 100644
>> --- a/arch/x86/include/asm/refcount.h
>> +++ b/arch/x86/include/asm/refcount.h
>> @@ -4,6 +4,9 @@
>>  * x86-specific implementation of refcount_t. Based on PAX_REFCOUNT from
>>  * PaX/grsecurity.
>>  */
>> +
>> +#ifndef __ASSEMBLY__
> 
> Can you swap the order here, so that the asm macros are visible first
> in the file?
> 
> #ifdef __ASSEMBLY__
> ...macros
> #else
> ....C
> #endif

Done. I also noticed that I forgot in one instance (REFCOUNT_CHECK_LT_ZERO)
to explicitly mark the parameter name (“counter=\counter”), so I’ll fix it
as well in the next version.