linux-kernel - Re: [PATCH] mm: kfence: fix PG_slab and memcg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <974ef73e-ab4f-7b24-d070-c981654e8c22@bytedance.com>
Date:   Tue, 21 Mar 2023 12:14:08 +0800
From:   Peng Zhang <zhangpeng.00@...edance.com>
To:     Muchun Song <songmuchun@...edance.com>, glider@...gle.com,
        elver@...gle.com, dvyukov@...gle.com, akpm@...ux-foundation.org,
        sjpark@...zon.de, jannh@...gle.com, muchun.song@...ux.dev,
        roman.gushchin@...ux.dev
Cc:     kasan-dev@...glegroups.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: kfence: fix PG_slab and memcg_data clearing


在 2023/3/20 11:00, Muchun Song 写道:
> It does not reset PG_slab and memcg_data when KFENCE fails to initialize
> kfence pool at runtime. It is reporting a "Bad page state" message when
> kfence pool is freed to buddy. The checking of whether it is a compound
> head page seems unnecessary sicne we already guarantee this when allocating
> kfence pool, removing the check to simplify the code.
>
> Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
> Fixes: 8f0b36497303 ("mm: kfence: fix objcgs vector allocation")
> Signed-off-by: Muchun Song <songmuchun@...edance.com>
> ---
>   mm/kfence/core.c | 30 +++++++++++++++---------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 79c94ee55f97..d66092dd187c 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -561,10 +561,6 @@ static unsigned long kfence_init_pool(void)
>   		if (!i || (i % 2))
>   			continue;
>   
> -		/* Verify we do not have a compound head page. */
> -		if (WARN_ON(compound_head(&pages[i]) != &pages[i]))
> -			return addr;
> -
>   		__folio_set_slab(slab_folio(slab));
>   #ifdef CONFIG_MEMCG
>   		slab->memcg_data = (unsigned long)&kfence_metadata[i / 2 - 1].objcg |
> @@ -597,12 +593,26 @@ static unsigned long kfence_init_pool(void)
>   
>   		/* Protect the right redzone. */
>   		if (unlikely(!kfence_protect(addr + PAGE_SIZE)))
> -			return addr;
> +			goto reset_slab;
>   
>   		addr += 2 * PAGE_SIZE;
>   	}
>   
>   	return 0;
> +
> +reset_slab:
> +	for (i = 0; i < KFENCE_POOL_SIZE / PAGE_SIZE; i++) {
> +		struct slab *slab = page_slab(&pages[i]);
> +
> +		if (!i || (i % 2))
> +			continue;
> +#ifdef CONFIG_MEMCG
> +		slab->memcg_data = 0;
> +#endif
> +		__folio_clear_slab(slab_folio(slab));
> +	}
Can this loop be simplified to this?

	for (i = 2; i < KFENCE_POOL_SIZE / PAGE_SIZE; i+=2) {
		struct slab *slab = page_slab(&pages[i]);
#ifdef CONFIG_MEMCG
		slab->memcg_data = 0;
#endif
		__folio_clear_slab(slab_folio(slab));
	}

> +
> +	return addr;
>   }
>   
>   static bool __init kfence_init_pool_early(void)
> @@ -632,16 +642,6 @@ static bool __init kfence_init_pool_early(void)
>   	 * fails for the first page, and therefore expect addr==__kfence_pool in
>   	 * most failure cases.
>   	 */
> -	for (char *p = (char *)addr; p < __kfence_pool + KFENCE_POOL_SIZE; p += PAGE_SIZE) {
> -		struct slab *slab = virt_to_slab(p);
> -
> -		if (!slab)
> -			continue;
> -#ifdef CONFIG_MEMCG
> -		slab->memcg_data = 0;
> -#endif
> -		__folio_clear_slab(slab_folio(slab));
> -	}
>   	memblock_free_late(__pa(addr), KFENCE_POOL_SIZE - (addr - (unsigned long)__kfence_pool));
>   	__kfence_pool = NULL;
>   	return false;