diff mbox series

x86/sev: Fix overflow when computing address for PVALIDATE

Message ID 20231111020019.553664-1-michael.roth@amd.com
State New
Headers
Series x86/sev: Fix overflow when computing address for PVALIDATE |

Commit Message

Michael Roth Nov. 11, 2023, 2 a.m. UTC 
  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(-)

Comments

Dave Hansen Nov. 12, 2023, 6:49 a.m. UTC | #1 
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.
diff mbox series

Patch

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;