Re: [PATCH] mm: kasan: Index page hierarchy as an array

From: Mike Rapoport <rppt@linux•ibm.com>
To: Linus Walleij <linus.walleij@linaro•org>
Cc: kernel test robot <lkp@intel•com>, Arnd Bergmann <arnd@arndb•de>,
	kasan-dev@googlegroups•com,
	Alexander Potapenko <glider@google•com>,
	Dmitry Vyukov <dvyukov@google•com>,
	Andrey Ryabinin <aryabinin@virtuozzo•com>,
	Ard Biesheuvel <ardb@kernel•org>,
	linux-arm-kernel@lists•infradead.org
Subject: Re: [PATCH] mm: kasan: Index page hierarchy as an array
Date: Fri, 6 Nov 2020 11:25:02 +0200	[thread overview]
Message-ID: <20201106092502.GE301789@linux.ibm.com> (raw)
In-Reply-To: <20201106085157.11211-1-linus.walleij@linaro.org>

On Fri, Nov 06, 2020 at 09:51:57AM +0100, Linus Walleij wrote:
> When freeing page directories, KASan was consistently
> indexing through the page hierarchy like this:
> 
>   static void kasan_free_pud(pud_t *pud_start, p4d_t *p4d) {
>     pud_t *pud;
>     int i;
> 
>     for (i = 0; i < PTRS_PER_PUD; i++) {
>       pud = pud_start + i;
>       if (!pud_none(*pud))
>         if (!pud_none(pud_start[i]))
>           return;
>     }
>   }
> 
> That is: implicitly add i sizeof(put_t) idices to
> the variable pud.
> 
> On ARM32 arch/arm/include/asm/pgtable-2level.h has folded
> the PMDs into the PUDs and thus has this definition of
> pud_none():
> 
>   #define pud_none(pud)           (0)
> 
> This will make the above construction emit this harmless
> build warning on ARM32:
> 
>   mm/kasan/init.c: In function 'kasan_free_pud':
>   >> mm/kasan/init.c:318:9: warning: variable 'pud' set but not used [-Wunused-but-set-variable]
>      318 |  pud_t *pud;
>          |         ^~~
> 
> Using an explicit array removes this problem and also makes
> the build warning go away. Arguably the code also gets
> easier to read.
> 
> So I fixed all the kasan_free_p??() to use explicit
> array inidices instead.
> 
> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
> Reported-by: kernel test robot <lkp@intel•com>
> Suggested-by: Ard Biesheuvel <ardb@kernel•org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro•org>

Acked-by: Mike Rapoport <rppt@linux•ibm.com>

> ---
>  mm/kasan/init.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
> index fe6be0be1f76..3c74c30996ef 100644
> --- a/mm/kasan/init.c
> +++ b/mm/kasan/init.c
> @@ -285,12 +285,10 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>  
>  static void kasan_free_pte(pte_t *pte_start, pmd_t *pmd)
>  {
> -	pte_t *pte;
>  	int i;
>  
>  	for (i = 0; i < PTRS_PER_PTE; i++) {
> -		pte = pte_start + i;
> -		if (!pte_none(*pte))
> +		if (!pte_none(pte_start[i]))
>  			return;
>  	}
>  
> @@ -300,12 +298,10 @@ static void kasan_free_pte(pte_t *pte_start, pmd_t *pmd)
>  
>  static void kasan_free_pmd(pmd_t *pmd_start, pud_t *pud)
>  {
> -	pmd_t *pmd;
>  	int i;
>  
>  	for (i = 0; i < PTRS_PER_PMD; i++) {
> -		pmd = pmd_start + i;
> -		if (!pmd_none(*pmd))
> +		if (!pmd_none(pmd_start[i]))
>  			return;
>  	}
>  
> @@ -315,12 +311,10 @@ static void kasan_free_pmd(pmd_t *pmd_start, pud_t *pud)
>  
>  static void kasan_free_pud(pud_t *pud_start, p4d_t *p4d)
>  {
> -	pud_t *pud;
>  	int i;
>  
>  	for (i = 0; i < PTRS_PER_PUD; i++) {
> -		pud = pud_start + i;
> -		if (!pud_none(*pud))
> +		if (!pud_none(pud_start[i]))
>  			return;
>  	}
>  
> @@ -330,12 +324,10 @@ static void kasan_free_pud(pud_t *pud_start, p4d_t *p4d)
>  
>  static void kasan_free_p4d(p4d_t *p4d_start, pgd_t *pgd)
>  {
> -	p4d_t *p4d;
>  	int i;
>  
>  	for (i = 0; i < PTRS_PER_P4D; i++) {
> -		p4d = p4d_start + i;
> -		if (!p4d_none(*p4d))
> +		if (!p4d_none(p4d_start[i]))
>  			return;
>  	}
>  
> -- 
> 2.26.2
> 

-- 
Sincerely yours,
Mike.

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