linux-hardening - Re: [PATCH v2 1/1] x86/paravirt: write paravirt ident function in assembly

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>] [day] [month] [year] [list]

Message-ID: <fa6df43b-8a1a-8ad1-0236-94d2a0b588fa@suse.com>
Date:   Thu, 15 Sep 2022 08:59:43 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Bill Wendling <morbo@...gle.com>, linux-hardening@...r.kernel.org
Cc:     Kees Cook <keescook@...omium.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        "Srivatsa S. Bhat (VMware)" <srivatsa@...il.mit.edu>,
        Alexey Makhalov <amakhalov@...are.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        VMware PV-Drivers Reviewers <pv-drivers@...are.com>,
        x86@...nel.org, virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
        Peter Zijlstra <peterz@...radead.org>,
        Nathan Chancellor <nathan@...nel.org>
Subject: Re: [PATCH v2 1/1] x86/paravirt: write paravirt ident function in
 assembly

On 14.09.22 18:21, Bill Wendling wrote:
> The ZERO_CALL_USED_REGS feature may zero out caller-saved registers
> before returning. However, alternate code may call this function without
> first saving %[re]di, because the proper clobbers aren't taken into
> account.
> 
> This shows up in spurious_kernel_fault() where the "pte_offset_kernel()"
> call results in this assembly code:
> 
> .Ltmp151:
>          #APP
>          # ALT: oldnstr
> .Ltmp152:
> .Ltmp153:
> .Ltmp154:
>          .section        .discard.retpoline_safe,"",@progbits
>          .quad   .Ltmp154
>          .text
> 
>          callq   *pv_ops+536(%rip)
> 
> .Ltmp155:
>          .section        .parainstructions,"a",@progbits
>          .p2align        3, 0x0
>          .quad   .Ltmp153
>          .byte   67
>          .byte   .Ltmp155-.Ltmp153
>          .short  1
>          .text
> .Ltmp156:
>          # ALT: padding
>          .zero   (-(((.Ltmp157-.Ltmp158)-(.Ltmp156-.Ltmp152))>0))*((.Ltmp157-.Ltmp158)-(.Ltmp156-.Ltmp152)),144
> .Ltmp159:
>          .section        .altinstructions,"a",@progbits
> .Ltmp160:
>          .long   .Ltmp152-.Ltmp160
> .Ltmp161:
>          .long   .Ltmp158-.Ltmp161
>          .short  33040
>          .byte   .Ltmp159-.Ltmp152
>          .byte   .Ltmp157-.Ltmp158
>          .text
> 
>          .section        .altinstr_replacement,"ax",@progbits
>          # ALT: replacement 1
> .Ltmp158:
>          movq    %rdi, %rax
> .Ltmp157:
>          .text
>          #NO_APP
> .Ltmp162:
>          testb   $-128, %dil
> 
> The %dil register was zeroed out by the call to "*pv_ops+536(%rip)".
> 
> In general, the _paravirt_ident_64() function appears like it shouldn't
> have any instrumentation or other modifications applied to it. Thus just
> write it in assembly to avoid having to continually modify it whenever a
> new feature comes along.
> 
> Link: https://github.com/KSPP/linux/issues/192
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Cc: Juergen Gross <jgross@...e.com>
> Cc: "Srivatsa S. Bhat (VMware)" <srivatsa@...il.mit.edu>
> Cc: Alexey Makhalov <amakhalov@...are.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: VMware PV-Drivers Reviewers <pv-drivers@...are.com>
> Cc: x86@...nel.org
> Cc: virtualization@...ts.linux-foundation.org
> Cc: linux-kernel@...r.kernel.org
> Cc: llvm@...ts.linux.dev
> Signed-off-by: Bill Wendling <morbo@...gle.com>
> Suggested-by: Peter Zijlstra <peterz@...radead.org>
> Reported-and-tested-by: Nathan Chancellor <nathan@...nel.org>

Reviewed-by: Juergen Gross <jgross@...e.com>


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)