[PATCH] arm64/mm: Fix pgtable page offset address in [pud|pmd]_free_[pmd|pte]_page

[PATCH] arm64/mm: Fix pgtable page offset address in [pud|pmd]_free_[pmd|pte]_page

[Anshuman Khandual]

The pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if
the input address is either PMD_SIZE or PTE_SIZE aligned. Though incoming
input addresses tend to be aligned to the required size (PMD_SIZE|PTE_SIZE)
which is the reason why there were no reported problems earlier. But it is
not correct. However 0UL as input address will guarantee that the fetched
pgtable page address is always correct without any possible skid. While at
this just warn once when the addresses are not aligned.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/mmu.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e97f018ff740..17af49585fb2 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1005,7 +1005,8 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
 		return 1;
 	}
 
-	table = pte_offset_kernel(pmdp, addr);
+	WARN_ON_ONCE(!IS_ALIGNED(addr, PMD_SIZE));
+	table = pte_offset_kernel(pmdp, 0UL);
 	pmd_clear(pmdp);
 	__flush_tlb_kernel_pgtable(addr);
 	pte_free_kernel(NULL, table);
@@ -1026,8 +1027,9 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 		return 1;
 	}
 
-	table = pmd_offset(pudp, addr);
-	pmdp = table;
+	WARN_ON_ONCE(!IS_ALIGNED(addr, PUD_SIZE));
+	table = pmd_offset(pudp, 0UL);
+	pmdp = pmd_offset(pudp, addr);
 	next = addr;
 	end = addr + PUD_SIZE;
 	do {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm: Fix pgtable page offset address in [pud|pmd]_free_[pmd|pte]_page

[Catalin Marinas]

On Mon, Apr 29, 2019 at 09:21:54AM +0530, Anshuman Khandual wrote:
> The pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if
> the input address is either PMD_SIZE or PTE_SIZE aligned. Though incoming
> input addresses tend to be aligned to the required size (PMD_SIZE|PTE_SIZE)
> which is the reason why there were no reported problems earlier. But it is
> not correct. However 0UL as input address will guarantee that the fetched
> pgtable page address is always correct without any possible skid. While at
> this just warn once when the addresses are not aligned.

I'm fine with using 0UL but did you actually hit any problem to be worth
adding a WARN_ON?

> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/mm/mmu.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e97f018ff740..17af49585fb2 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1005,7 +1005,8 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>  		return 1;
>  	}
>  
> -	table = pte_offset_kernel(pmdp, addr);
> +	WARN_ON_ONCE(!IS_ALIGNED(addr, PMD_SIZE));

I'd use VM_WARN_ON_ONCE() as we do, for example, in set_pte_at().

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/mm: Fix pgtable page offset address in [pud|pmd]_free_[pmd|pte]_page

[Anshuman Khandual]

On 04/29/2019 04:53 PM, Catalin Marinas wrote:
> On Mon, Apr 29, 2019 at 09:21:54AM +0530, Anshuman Khandual wrote:
>> The pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if
>> the input address is either PMD_SIZE or PTE_SIZE aligned. Though incoming
>> input addresses tend to be aligned to the required size (PMD_SIZE|PTE_SIZE)
>> which is the reason why there were no reported problems earlier. But it is
>> not correct. However 0UL as input address will guarantee that the fetched
>> pgtable page address is always correct without any possible skid. While at
>> this just warn once when the addresses are not aligned.
> 
> I'm fine with using 0UL but did you actually hit any problem to be worth
> adding a WARN_ON?

No I did not hit any problem. But the higher level function (ioremap_page_range)
from which this gets called does not check the alignment for incoming address
range. As it can be called from outside of core MM like drivers we cannot even
correct the alignment without changing the range which would risk potential
undefined behavior. Hence just warn once and then move on to do what ever has
been asked by the caller.

> 
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/mm/mmu.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index e97f018ff740..17af49585fb2 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1005,7 +1005,8 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>>  		return 1;
>>  	}
>>  
>> -	table = pte_offset_kernel(pmdp, addr);
>> +	WARN_ON_ONCE(!IS_ALIGNED(addr, PMD_SIZE));
> 
> I'd use VM_WARN_ON_ONCE() as we do, for example, in set_pte_at().

Did you mean VM_WARN_ONCE() instead which is in set_pte_at(). Sure will change it.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel