[PATCH v5 1/6] x86/sev: Fix calculation of end address based on number of pages

Tom Lendacky posted 6 patches 2 years, 1 month ago
There is a newer version of this series
[PATCH v5 1/6] x86/sev: Fix calculation of end address based on number of pages
Posted by Tom Lendacky 2 years, 1 month ago
When calculating an end address based on an unsigned int number of pages,
the number of pages must be cast to an unsigned long so that any value
greater than or equal to 0x100000 does not result in zero after the shift.

Fixes: 5e5ccff60a29 ("x86/sev: Add helper for validating pages in early enc attribute changes")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/sev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index c05f0124c410..cac56540929d 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -649,7 +649,7 @@ static void pvalidate_pages(unsigned long vaddr, unsigned int npages, bool valid
 	int rc;
 
 	vaddr = vaddr & PAGE_MASK;
-	vaddr_end = vaddr + (npages << PAGE_SHIFT);
+	vaddr_end = vaddr + ((unsigned long)npages << PAGE_SHIFT);
 
 	while (vaddr < vaddr_end) {
 		rc = pvalidate(vaddr, RMP_PG_SIZE_4K, validate);
@@ -666,7 +666,7 @@ static void __init early_set_pages_state(unsigned long paddr, unsigned int npage
 	u64 val;
 
 	paddr = paddr & PAGE_MASK;
-	paddr_end = paddr + (npages << PAGE_SHIFT);
+	paddr_end = paddr + ((unsigned long)npages << PAGE_SHIFT);
 
 	while (paddr < paddr_end) {
 		/*
-- 
2.37.3
Re: [PATCH v5 1/6] x86/sev: Fix calculation of end address based on number of pages
Posted by Dionna Amalie Glaze 2 years, 1 month ago
>
> When calculating an end address based on an unsigned int number of pages,
> the number of pages must be cast to an unsigned long so that any value
> greater than or equal to 0x100000 does not result in zero after the shift.
>
> Fixes: 5e5ccff60a29 ("x86/sev: Add helper for validating pages in early enc attribute changes")

Tested-by: Dionna Glaze <dionnaglaze@google.com>

-- 
-Dionna Glaze, PhD (she/her)
Re: [PATCH v5 1/6] x86/sev: Fix calculation of end address based on number of pages
Posted by Dave Hansen 2 years, 1 month ago
On 9/27/22 10:04, Tom Lendacky wrote:
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -649,7 +649,7 @@ static void pvalidate_pages(unsigned long vaddr, unsigned int npages, bool valid
>  	int rc;
>  
>  	vaddr = vaddr & PAGE_MASK;
> -	vaddr_end = vaddr + (npages << PAGE_SHIFT);
> +	vaddr_end = vaddr + ((unsigned long)npages << PAGE_SHIFT);

Could we please just fix the fragile typing that cascaded down to this
point?

Shouldn't 'npages' in this interface be a long?

> struct x86_guest {
>         void (*enc_status_change_prepare)(unsigned long vaddr, int npages, bool enc);
>         bool (*enc_status_change_finish)(unsigned long vaddr, int npages, bool enc);
>         bool (*enc_tlb_flush_required)(bool enc);
>         bool (*enc_cache_flush_required)(void);
> };
Re: [PATCH v5 1/6] x86/sev: Fix calculation of end address based on number of pages
Posted by Tom Lendacky 2 years, 1 month ago
On 9/27/22 12:10, Dave Hansen wrote:
> On 9/27/22 10:04, Tom Lendacky wrote:
>> --- a/arch/x86/kernel/sev.c
>> +++ b/arch/x86/kernel/sev.c
>> @@ -649,7 +649,7 @@ static void pvalidate_pages(unsigned long vaddr, unsigned int npages, bool valid
>>   	int rc;
>>   
>>   	vaddr = vaddr & PAGE_MASK;
>> -	vaddr_end = vaddr + (npages << PAGE_SHIFT);
>> +	vaddr_end = vaddr + ((unsigned long)npages << PAGE_SHIFT);
> 
> Could we please just fix the fragile typing that cascaded down to this
> point?
> 
> Shouldn't 'npages' in this interface be a long?

I'll take a look at that.

Thanks,
Tom

> 
>> struct x86_guest {
>>          void (*enc_status_change_prepare)(unsigned long vaddr, int npages, bool enc);
>>          bool (*enc_status_change_finish)(unsigned long vaddr, int npages, bool enc);
>>          bool (*enc_tlb_flush_required)(bool enc);
>>          bool (*enc_cache_flush_required)(void);
>> };