[V3,1/2] mm/ioremap: Check virtual address alignment while creating huge mappings
diff mbox series

Message ID 1557377177-20695-2-git-send-email-anshuman.khandual@arm.com
State New
Headers show
Series
  • mm/ioremap: Check virtual address alignment
Related show

Commit Message

Anshuman Khandual May 9, 2019, 4:46 a.m. UTC
Virtual address alignment is essential in ensuring correct clearing for all
intermediate level pgtable entries and freeing associated pgtable pages. An
unaligned address can end up randomly freeing pgtable page that potentially
still contains valid mappings. Hence also check it's alignment along with
existing phys_addr check.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Chintan Pandya <cpandya@codeaurora.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 lib/ioremap.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Kani, Toshi May 13, 2019, 10:26 p.m. UTC | #1
On Thu, 2019-05-09 at 10:16 +0530, Anshuman Khandual wrote:
> Virtual address alignment is essential in ensuring correct clearing for all
> intermediate level pgtable entries and freeing associated pgtable pages. An
> unaligned address can end up randomly freeing pgtable page that potentially
> still contains valid mappings. Hence also check it's alignment along with
> existing phys_addr check.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Chintan Pandya <cpandya@codeaurora.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  lib/ioremap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/ioremap.c b/lib/ioremap.c
> index 063213685563..8b5c8dda857d 100644
> --- a/lib/ioremap.c
> +++ b/lib/ioremap.c
> @@ -86,6 +86,9 @@ static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
>  	if ((end - addr) != PMD_SIZE)
>  		return 0;
>  
> +	if (!IS_ALIGNED(addr, PMD_SIZE))
> +		return 0;
> +
>  	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
>  		return 0;
>  
> @@ -126,6 +129,9 @@ static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
>  	if ((end - addr) != PUD_SIZE)
>  		return 0;
>  
> +	if (!IS_ALIGNED(addr, PUD_SIZE))
> +		return 0;
> +
>  	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
>  		return 0;

Not sure if we have such case today, but I agree that it is prudent to
have such checks.  Is there any reason not to add this check to p4d for
consistency?

Thanks,
-Toshi
Anshuman Khandual May 14, 2019, 5:55 a.m. UTC | #2
On 05/14/2019 03:56 AM, Kani, Toshi wrote:
> On Thu, 2019-05-09 at 10:16 +0530, Anshuman Khandual wrote:
>> Virtual address alignment is essential in ensuring correct clearing for all
>> intermediate level pgtable entries and freeing associated pgtable pages. An
>> unaligned address can end up randomly freeing pgtable page that potentially
>> still contains valid mappings. Hence also check it's alignment along with
>> existing phys_addr check.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Toshi Kani <toshi.kani@hpe.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Chintan Pandya <cpandya@codeaurora.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> ---
>>  lib/ioremap.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/ioremap.c b/lib/ioremap.c
>> index 063213685563..8b5c8dda857d 100644
>> --- a/lib/ioremap.c
>> +++ b/lib/ioremap.c
>> @@ -86,6 +86,9 @@ static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
>>  	if ((end - addr) != PMD_SIZE)
>>  		return 0;
>>  
>> +	if (!IS_ALIGNED(addr, PMD_SIZE))
>> +		return 0;
>> +
>>  	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
>>  		return 0;
>>  
>> @@ -126,6 +129,9 @@ static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
>>  	if ((end - addr) != PUD_SIZE)
>>  		return 0;
>>  
>> +	if (!IS_ALIGNED(addr, PUD_SIZE))
>> +		return 0;
>> +
>>  	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
>>  		return 0;
> 
> Not sure if we have such case today, but I agree that it is prudent to
> have such checks.  Is there any reason not to add this check to p4d for
> consistency?

No, will add it.

Patch
diff mbox series

diff --git a/lib/ioremap.c b/lib/ioremap.c
index 063213685563..8b5c8dda857d 100644
--- a/lib/ioremap.c
+++ b/lib/ioremap.c
@@ -86,6 +86,9 @@  static int ioremap_try_huge_pmd(pmd_t *pmd, unsigned long addr,
 	if ((end - addr) != PMD_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, PMD_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, PMD_SIZE))
 		return 0;
 
@@ -126,6 +129,9 @@  static int ioremap_try_huge_pud(pud_t *pud, unsigned long addr,
 	if ((end - addr) != PUD_SIZE)
 		return 0;
 
+	if (!IS_ALIGNED(addr, PUD_SIZE))
+		return 0;
+
 	if (!IS_ALIGNED(phys_addr, PUD_SIZE))
 		return 0;