[PATCH] x86/sev: Fix overflow when computing address for PVALIDATE

[PATCH] x86/sev: Fix overflow when computing address for PVALIDATE

[Michael Roth]

The GFN in the struct used for page-state change requests is a 40-bit
bit-field, so attempts to shift it by PAGE_SHIFT to get an address will
result in bits getting lost if they cross the 1TB boundary. This ends up
causing crashes when booting 1TB SNP guests with kernels that support
lazy-acceptance (without kernel-side lazy-acceptance support, OVMF
handles the PVALIDATE for GFNs in the upper address ranges, so this
overflow condition is never encountered by the kernel).

Fix this by casting the 40-bit GFN field to a 64-bit type before
converting it to an address.

Fixes: 6c3211796326 ("x86/sev: Add SNP-specific unaccepted memory support")
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 arch/x86/kernel/sev-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index ccb0915e84e1..d92e048f4235 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -1083,7 +1083,7 @@ static void pvalidate_pages(struct snp_psc_desc *desc)
 	for (i = 0; i <= desc->hdr.end_entry; i++) {
 		e = &desc->entries[i];
 
-		vaddr = (unsigned long)pfn_to_kaddr(e->gfn);
+		vaddr = (unsigned long)pfn_to_kaddr((unsigned long)e->gfn);
 		size = e->pagesize ? RMP_PG_SIZE_2M : RMP_PG_SIZE_4K;
 		validate = e->operation == SNP_PAGE_STATE_PRIVATE;
 
-- 
2.25.1

Re: [PATCH] x86/sev: Fix overflow when computing address for PVALIDATE

[Dave Hansen]

On 11/10/23 18:00, Michael Roth wrote:
> -		vaddr = (unsigned long)pfn_to_kaddr(e->gfn);
> +		vaddr = (unsigned long)pfn_to_kaddr((unsigned long)e->gfn);
>  		size = e->pagesize ? RMP_PG_SIZE_2M : RMP_PG_SIZE_4K;
>  		validate = e->operation == SNP_PAGE_STATE_PRIVATE;
>  

... and people wonder why some of us avoid bitfields.

Shouldn't we just fix this permanently in pfn_to_kaddr()?  Surely more
of these are lurking around.  Anything doing a large shift up on a
little type is asking for trouble.