linux-kernel - Re: [PATCH 1/5] x86/pvh: Call C code via the kernel virtual mapping

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: <06f800e1-f0b1-4040-83f8-7c026ca17268@amd.com>
Date: Thu, 26 Sep 2024 16:29:54 -0400
From: Jason Andryuk <jason.andryuk@....com>
To: Ard Biesheuvel <ardb@...nel.org>, Ard Biesheuvel <ardb+git@...gle.com>
CC: <linux-kernel@...r.kernel.org>, Juergen Gross <jgross@...e.com>, Boris
 Ostrovsky <boris.ostrovsky@...cle.com>, <x86@...nel.org>,
	<xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH 1/5] x86/pvh: Call C code via the kernel virtual mapping

On 2024-09-26 06:55, Ard Biesheuvel wrote:
> On Thu, 26 Sept 2024 at 12:41, Ard Biesheuvel <ardb+git@...gle.com> wrote:
>>
>> From: Ard Biesheuvel <ardb@...nel.org>
>>
>> Calling C code via a different mapping than it was linked at is
>> problematic, because the compiler assumes that RIP-relative and absolute
>> symbol references are interchangeable. GCC in particular may use
>> RIP-relative per-CPU variable references even when not using -fpic.
>>
>> So call xen_prepare_pvh() via its kernel virtual mapping on x86_64, so
>> that those RIP-relative references produce the correct values. This
>> matches the pre-existing behavior for i386, which also invokes
>> xen_prepare_pvh() via the kernel virtual mapping before invoking
>> startup_32 with paging disabled again.
>>
>> Fixes: 7243b93345f7 ("xen/pvh: Bootstrap PVH guest")
>> Tested-by: Jason Andryuk <jason.andryuk@....com>
>> Reviewed-by: Jason Andryuk <jason.andryuk@....com>
>> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
>> ---
>>   arch/x86/platform/pvh/head.S | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S
>> index 64fca49cd88f..98ddd552885a 100644
>> --- a/arch/x86/platform/pvh/head.S
>> +++ b/arch/x86/platform/pvh/head.S
>> @@ -172,7 +172,13 @@ SYM_CODE_START_LOCAL(pvh_start_xen)
>>          movq %rbp, %rbx
>>          subq $_pa(pvh_start_xen), %rbx
>>          movq %rbx, phys_base(%rip)
>> -       call xen_prepare_pvh
>> +
>> +       /* Call xen_prepare_pvh() via the kernel virtual mapping */
>> +       leaq xen_prepare_pvh(%rip), %rax
> 
> Just realized that we probably need
> 
> +       subq phys_base(%rip), %rax

Yes, this is necessary when phys_base is non-0.  I intended to test a 
non-0 case yesterday, but it turns out I didn't.  Re-testing, I have 
confirmed this subq is necessary.

Thanks,
Jason