| Message ID | 1556509914-21138-1-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/mm: Fix pgtable page offset address in [pud|pmd]_free_[pmd|pte]_page | expand |
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().
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.
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 {
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(-)