linux-kernel - Re: [PATCH 1/2] hyperv: Fix pointer type for the output of the hypercall in get

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: <07820167-b415-40b0-9858-d25325c348ba@linux.microsoft.com>
Date: Thu, 19 Dec 2024 12:00:56 -0800
From: Roman Kisel <romank@...ux.microsoft.com>
To: Easwar Hariharan <eahariha@...ux.microsoft.com>,
 Nuno Das Neves <nunodasneves@...ux.microsoft.com>
Cc: hpa@...or.com, kys@...rosoft.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, decui@...rosoft.com, haiyangz@...rosoft.com,
 mingo@...hat.com, mhklinux@...look.com, tglx@...utronix.de,
 tiala@...rosoft.com, wei.liu@...nel.org, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org, x86@...nel.org, apais@...rosoft.com,
 benhill@...rosoft.com, ssengar@...rosoft.com, sunilmut@...rosoft.com,
 vdso@...bites.dev
Subject: Re: [PATCH 1/2] hyperv: Fix pointer type for the output of the
 hypercall in get_vtl(void)



On 12/19/2024 11:13 AM, Easwar Hariharan wrote:
> On 12/19/2024 10:40 AM, Nuno Das Neves wrote:
>> On 12/18/2024 12:54 PM, Roman Kisel wrote:
>>> Commit bc905fa8b633 ("hyperv: Switch from hyperv-tlfs.h to hyperv/hvhdk.h")
>>> changed the type of the output pointer to `struct hv_register_assoc` from
>>> `struct hv_get_vp_registers_output`. That leads to an incorrect computation,
>>> and leaves the system broken.
>>>
>> My bad! But, lets not use `struct hv_get_registers_output`. Instead, use
>> `struct hv_register_value`, since that is the more complete definition of a
>> register value. The output of the get_vp_registers hypercall is just an array
>> of these values.
>>
>> Ideally we remove `struct hv_get_vp_registers_output` at some point, since
>> it serves the same role as `struct hv_register_value` but in a more limited
>> capacity.
>>
>> Thanks
>> Nuno
>>
> 
> I had much the same conversation with Roman off-list yesterday.
Appreciate your help, Easwar!

> 
> The choice is between using hv_get_registers_output which is clearly the
> output of the GetVpRegisters hypercall by name, albeit limited as you
> said, and hv_register_value which is the more complete definition and
> what the hypervisor actually returns, but does not currently include the
> arm64 definitions in our copy of hvgdk_mini.h. hv_get_registers_output
> and hv_register_value overlap in layout for Roman's purposes.
> 
> FWIW, I'm in favor of adding the arm64 definitions to hv_register_value
> and using it for this get_vtl() patch.
I could do that, will wait to see if no objections.

> 
> This could be accompanied with migration of hv_get_vpreg128 in arm64/
> and removal of struct hv_get_registers_output, or that could be deferred
> to a later patch.
Having an equivalent of `hv_get_vpreg128` would be awesome on x64!
I'd bet something like that should exist in dom0/mshv already if you'd
like to have a dedicated function. So perhaps we could let mshv take
the lead with that?

If the desire is also to use the fast hypercall technique as
`hv_get_vpreg128` does, then we'd need fast hypercalls on x64 that
can use XMM registers (aka the fast extended hypercalls) that aren't
implemented in the hyperv drivers from my read of the code (although
it seems KVM knows how to do that when it emulates Hyper-V,
arch/x86/kvm/hyperv.c, struct kvm_hv_hcall *hc).

These considerations make me lean towards deferring hv_get_vpreg128-like
implementation to a later time.

> 
> - Easwar
> 

-- 
Thank you,
Roman