linux-kernel - Re: [PATCH] x86/hyperv: Export hv_hypercall

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: <56521d85-1da5-4d25-b100-7dbe62e34d1d@linux.microsoft.com>
Date: Thu, 18 Sep 2025 11:33:18 +0530
From: Naman Jain <namjain@...ux.microsoft.com>
To: Sean Christopherson <seanjc@...gle.com>,
 Paolo Bonzini <pbonzini@...hat.com>, Peter Zijlstra <peterz@...radead.org>
Cc: Roman Kisel <romank@...ux.microsoft.com>,
 "K . Y . Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>, Thomas Gleixner <tglx@...utronix.de>,
 Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
 "H . Peter Anvin" <hpa@...or.com>, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org, mhklinux@...look.com
Subject: Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally

On 9/16/2025 8:22 PM, Sean Christopherson wrote:
> On Tue, Sep 16, 2025, Paolo Bonzini wrote:
>> On 8/27/25 01:04, Roman Kisel wrote:
>>> On 8/26/2025 5:07 AM, Peter Zijlstra wrote:
>>>> I do not know what OpenHCL is. Nor is it clear from the code what NMIs
>>>> can't happen. Anyway, same can be achieved with breakpoints / kprobes.
>>>> You can get a trap after setting CR2 and scribble it.
>>>>
>>>> You simply cannot use CR2 this way.
>>>
>>> The code in question runs with interrupts disabled, and the kernel runs
>>> without the memory swapping when using that module - the kernel is
>>> a firmware to host a vTPM for virtual machines. Somewhat similar to SMM.
>>> That should've been reflected somewhere in the comments and in Kconfig,
>>> we could do better. All in all, the page fault cannot happen in that
>>> path thus CR2 won't be trashed.
>>>
>>> Nor this kind of code can be stepped through in a self-hosted
>>> kernel debugger like kgdb. There are other examples of such code iiuc:
>>
>> As Sean mentioned, you do have to make sure that this is annotated as
>> noinstr (not instrumentable).  And also just use assembly - KVM started with
>> a similar asm block, though without the sketchy "register asm",
> 
> Ooh, yeah, don't use "register asm".  I missed that when I peeked at the code.
> Using "register asm" will most definitely cause problems, because the compiler
> doesn't track usage in C code, i.e. will happily use the GPR and clobber your
> asm value in the process.  That inevitably leads to very confusing and somewhat
> transient errors.  E.g. if someone inserts a printk for debugging, the call to
> printk can clobber the very state it's trying to print.
> 
>> and I was initially skeptical but using a dedicated .S file was absolutely
>> the right thing to do.
> 
> +1000 to putting the assembly in a .S file.  I too was a bit skeptical about
> moving the entire sequence into proper assembly; thankfully, some non-KVM folks
> talked us into it :-)

Thank you so much Sean and Paolo for your valuable inputs. I will try
out these things. Summarizing the suggestions here:
* Use noinstr (no instrumentation)
* Have separate .S file
* Don't use "register asm".
* Use static calls for solving IBT problems
* RAX:RCX is probably ok to be used, considering ABI. Whether we would 
still need to use STACK_FRAME_NON_STANDARD, I am not sure, but I will 
see based on how it goes.

I hope this addresses the concerns Peter raised. If there's anything I 
might have missed, I'm happy to make further adjustments if needed.

Regards,
Naman