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: <d2c68cbe-2e92-4801-b1a3-af4645e9ba78@zytor.com>
Date: Tue, 30 Sep 2025 12:49:21 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Jürgen Groß <jgross@...e.com>,
        Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
        virtualization@...ts.linux.dev, xin@...or.com,
        Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ajay Kaher <ajay.kaher@...adcom.com>,
        Alexey Makhalov <alexey.makhalov@...adcom.com>,
        Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v2 11/12] x86/paravirt: Don't use pv_ops vector for MSR
 access functions

On 2025-09-30 03:43, Jürgen Groß wrote:
>>
>>> In Xin's series there was a patch written initially by you to solve such
>>> a problem by adding the _ASM_EXTABLE_FUNC_REWIND() exception table method.
>>> I think this is a dead end, as it will break when using a shadow stack.
>>
>> No memories, let me go search. I found this:
>>
>>    https://patchwork.ozlabs.org/project/linux-ide/
>> patch/20250331082251.3171276-12-xin@...or.com/
>>
>> That's the other Peter :-)
> 
> Oh, my bad, sorry. :-)

Yes, you would have to patch both the stack and the shadow stack.

BUT: in the end it really doesn't really buy much. The only thing that
benefits is Xen, but it is fairly easy for Xen (my original POC did this) to
filter out the quite few MSRs that they do special dispatch for (plus the
variable case), and then they can just use the native code including the
benefit of using WRMSRNS and immediates.

The other approach that I also came up with looks like this:

/* Native code (non-immediate): trap point at +7 */

   0:   48 89 c2                mov    %rax,%rdx
   3:   48 c1 ea 20             shr    $0x20,%rdx
   7:   0f 01 c6                wrmsrns
   a:

/* Native code (immediate): trap point at +0 */

   0:   36 c4 e7 7a f6 c0 xx    ds wrmsrns %rax,$XX
        xx xx xx
   a:

/* Xen code, stub sets CF = 1 on failure */

   0:   e8 xx xx xx xx          call   asm_xen_pv_wrmsr
   5:   73 03                   jnc    0xa
   7:   0f 0b                   ud2
   9:   90                      nop
   a:

The trap point even ends up in the same place! UD2 can be any 1-, 2-, or
3-byte trapping instruction.

> 
>>> Additionally I found a rather ugly hack only to avoid re-iterating most of
>>> the bare metal ALTERNATIVE() for the paravirt case. It is possible, but the
>>> bare metal case is gaining one additional ALTERNATIVE level, resulting in
>>> patching the original instruction with an identical copy first.
>>
>> OTOH the above generates atrocious crap code :/
> 
> Yes.

Please don't generate crap code -- that's exactly The Wrong Thing. I didn't
actually look at the output code; I honestly didn't think that that would even
be an issue.

If it is REALLY evil, then do something like shell script to generate the code
instead...

(One big problem here is that cpp doesn't understand colons as separators...)

	-hpa


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ