linux-kernel - Re: [PATCH v3 5/5] hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp

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

Message-ID: <7f4b0c42-34e7-4321-858f-22e8896609af@linux.microsoft.com>
Date: Fri, 27 Dec 2024 09:32:51 -0800
From: Roman Kisel <romank@...ux.microsoft.com>
To: Easwar Hariharan <eahariha@...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, nunodasneves@...ux.microsoft.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 v3 5/5] hyperv: Do not overlap the hvcall IO areas in
 hv_vtl_apicid_to_vp_id()



On 12/26/2024 2:01 PM, Easwar Hariharan wrote:
> On 12/26/2024 1:31 PM, Roman Kisel wrote:
>> The Top-Level Functional Specification for Hyper-V, Section 3.6 [1, 2],
>> disallows overlapping of the input and output hypercall areas, and
>> hv_vtl_apicid_to_vp_id() overlaps them.
>>
>> Use the output hypercall page of the current vCPU for the hypercall.
>>
>> [1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
>> [2] https://github.com/MicrosoftDocs/Virtualization-Documentation/tree/main/tlfs
>>
>> Signed-off-by: Roman Kisel <romank@...ux.microsoft.com>
>> ---
>>   arch/x86/hyperv/hv_vtl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
>> index 04775346369c..ec5716960162 100644
>> --- a/arch/x86/hyperv/hv_vtl.c
>> +++ b/arch/x86/hyperv/hv_vtl.c
>> @@ -189,7 +189,7 @@ static int hv_vtl_apicid_to_vp_id(u32 apic_id)
>>   	input->partition_id = HV_PARTITION_ID_SELF;
>>   	input->apic_ids[0] = apic_id;
>>   
>> -	output = (u32 *)input;
>> +	output = (u32*)*this_cpu_ptr(hyperv_pcpu_output_arg);
>                       ^
> Nit: I believe the space is preferred, but I won't insist on respinning
> it for that.
I think we can drop the whole type cast thing (as the other patch does).
Type coercion will do what is needed, and the type cast just appears to
be noise I guess.

> 
> It's a good idea to give credit to Michael with a Reported-by tag, and
> maybe a Closes: tag with a link to his email.
My bad, I should have definitely done that! Will certainly do!

> 
> As with the Fixes tag for patch 2, you don't need to respin the series
> and can just reply to this thread.
"Fixes" means something when the commit is present in the Linus'es tree
as I understood from the Wei's reply 
(https://lore.kernel.org/lkml/Z2OIsFUXcjVXpqtw@liuwe-devbox-debian-v2/):

" This commit is not in the mainline kernel yet, so this tag is not
needed.

It will most likely to be wrong since I will need to rebase the
hyperv-next branch.

I can fold this patch into the original patch and leave your
Signed-off-by there."

I'll check if the commit has been pulled into the mainline tree.

> 
> Otherwise, looks good to me.
> 
> Reviewed-by: Easwar Hariharan <eahariha@...ux.microsoft.com>

-- 
Thank you,
Roman