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
| ||
|
Date: Thu, 24 Mar 2022 15:29:56 +0000 From: Mark Rutland <mark.rutland@....com> To: Borislav Petkov <bp@...en8.de> Cc: Nick Desaulniers <ndesaulniers@...gle.com>, Nathan Chancellor <nathan@...nel.org>, x86-ml <x86@...nel.org>, lkml <linux-kernel@...r.kernel.org>, Peter Zijlstra <peterz@...radead.org>, Josh Poimboeuf <jpoimboe@...hat.com> Subject: Re: clang memcpy calls On Thu, Mar 24, 2022 at 12:19:19PM +0100, Borislav Petkov wrote: > Hi folks, Hi Boris, > so I've been looking at a recent objtool noinstr warning from clang > builds: > > vmlinux.o: warning: objtool: sync_regs()+0x20: call to memcpy() leaves .noinstr.text section > > The issue is that clang generates a memcpy() call when a struct copy > happens: > > if (regs != eregs) > *regs = *eregs; > > see below for asm output. > > While gcc does simply generate an actual "rep; movsq". I think there's a more general soundness problem with noinstr here, because with the options we pass today it's entirely legitimate for the compiler to generate out-of-line calls to a number of support functions (e.g. memcpy, but also memset and others), and we either need to inhibit out-of-line calls to *any* of those, or ensure the out-of-line copies used are never instrumented. I'm not entirely sure how to prevent this on arm64 short of some whole-compilation-unit shennanigans -- we don't have short sequence like "rep movsq" that can be easily inlined, and we explicitly instrument mem*() when certain KASAN options are selected. I think we need more compiler help to make noinstr sound generally, and/or may need to rethink the way we use noinstr. Thanks, Mark. > So, how hard would it be to make clang do that too pls? > > Oh, and another thing while we're comparing asm: I'd love for clang's > -fverbose-asm to issue interleaved C source lines too, like gcc does. > > That's it - no pink pony - just "normal" wishes. :-) > > GCC: > ==== > > sync_regs: > .LASANPC4246: > # arch/x86/kernel/traps.c:770: { > movq %rdi, %rsi # tmp91, eregs > # arch/x86/kernel/traps.c:771: struct pt_regs *regs = (struct pt_regs *)this_cpu_read(cpu_current_top_of_stack) - 1; > #APP > # 771 "arch/x86/kernel/traps.c" 1 > movq %gs:cpu_current_top_of_stack(%rip), %rax # cpu_current_top_of_stack, pfo_val__ > # 0 "" 2 > # arch/x86/kernel/traps.c:771: struct pt_regs *regs = (struct pt_regs *)this_cpu_read(cpu_current_top_of_stack) - 1; > #NO_APP > subq $168, %rax #, <retval> > # arch/x86/kernel/traps.c:772: if (regs != eregs) > cmpq %rdi, %rax # eregs, <retval> > je .L387 #, > # arch/x86/kernel/traps.c:773: *regs = *eregs; > movl $21, %ecx #, tmp89 > movq %rax, %rdi # <retval>, <retval> > rep movsq > .L387: > # arch/x86/kernel/traps.c:775: } > ret > > CLANG: > ====== > > .section .noinstr.text,"ax",@progbits > .globl sync_regs # -- Begin function sync_regs > .p2align 6, 0x90 > .type sync_regs,@function > sync_regs: # @sync_regs > # %bb.0: # %entry > pushq %rbx > #APP > movq %gs:cpu_current_top_of_stack(%rip), %rbx > #NO_APP > addq $-168, %rbx > cmpq %rdi, %rbx > je .LBB19_2 > # %bb.1: # %if.then > movq %rdi, %rsi > movl $168, %edx > movq %rbx, %rdi > callq memcpy@PLT > .LBB19_2: # %if.end > movq %rbx, %rax > popq %rbx > retq > > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists