• Alexander Potapenko's avatar
    x86/asm: Use stricter assembly constraints in bitops · 5b77e95d
    Alexander Potapenko authored
    There's a number of problems with how arch/x86/include/asm/bitops.h
    is currently using assembly constraints for the memory region
    bitops are modifying:
    
    1) Use memory clobber in bitops that touch arbitrary memory
    
    Certain bit operations that read/write bits take a base pointer and an
    arbitrarily large offset to address the bit relative to that base.
    Inline assembly constraints aren't expressive enough to tell the
    compiler that the assembly directive is going to touch a specific memory
    location of unknown size, therefore we have to use the "memory" clobber
    to indicate that the assembly is going to access memory locations other
    than those listed in the inputs/outputs.
    
    To indicate that BTR/BTS instructions don't necessarily touch the first
    sizeof(long) bytes of the argument, we also move the address to assembly
    inputs.
    
    This particular change leads to size increase of 124 kernel functions in
    a defconfig build. For some of them the diff is in NOP operations, other
    end up re-reading values from memory and may potentially slow down the
    execution. But without these clobbers the compiler is free to cache
    the contents of the bitmaps and use them as if they weren't changed by
    the inline assembly.
    
    2) Use byte-sized arguments for operations touching single bytes.
    
    Passing a long value to ANDB/ORB/XORB instructions makes the compiler
    treat sizeof(long) bytes as being clobbered, which isn't the case. This
    may theoretically lead to worse code in the case of heavy optimization.
    
    Practical impact:
    
    I've built a defconfig kernel and looked through some of the functions
    generated by GCC 7.3.0 with and without this clobber, and didn't spot
    any miscompilations.
    
    However there is a (trivial) theoretical case where this code leads to
    miscompilation:
    
      https://lkml.org/lkml/2019/3/28/393
    
    using just GCC 8.3.0 with -O2.  It isn't hard to imagine someone writes
    such a function in the kernel someday.
    
    So the primary motivation is to fix an existing misuse of the asm
    directive, which happens to work in certain configurations now, but
    isn't guaranteed to work under different circumstances.
    
    [ --mingo: Added -stable tag because defconfig only builds a fraction
      of the kernel and the trivial testcase looks normal enough to
      be used in existing or in-development code. ]
    Signed-off-by: default avatarAlexander Potapenko <glider@google.com>
    Cc: <stable@vger.kernel.org>
    Cc: Andy Lutomirski <luto@kernel.org>
    Cc: Borislav Petkov <bp@alien8.de>
    Cc: Brian Gerst <brgerst@gmail.com>
    Cc: Denys Vlasenko <dvlasenk@redhat.com>
    Cc: Dmitry Vyukov <dvyukov@google.com>
    Cc: H. Peter Anvin <hpa@zytor.com>
    Cc: James Y Knight <jyknight@google.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Paul E. McKenney <paulmck@linux.ibm.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Link: http://lkml.kernel.org/r/20190402112813.193378-1-glider@google.com
    [ Edited the changelog, tidied up one of the defines. ]
    Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
    5b77e95d
bitops.h 13.7 KB