linux-kernel - Re: [PATCH] cacheinfo: clear cache_leaves(cpu) in free_cache

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: <20210713113315.thsvrvqvqptc7hje@bogus>
Date:   Tue, 13 Jul 2021 12:33:24 +0100
From:   Sudeep Holla <sudeep.holla@....com>
To:     Xiongfeng Wang <wangxiongfeng2@...wei.com>
Cc:     gregkh@...uxfoundation.org, rafael@...nel.org,
        Sudeep Holla <sudeep.holla@....com>,
        linux-kernel@...r.kernel.org, james.morse@....com
Subject: Re: [PATCH] cacheinfo: clear cache_leaves(cpu) in
 free_cache_attributes()

On Tue, Jul 13, 2021 at 11:47:38AM +0800, Xiongfeng Wang wrote:
> On ARM64, when PPTT(Processor Properties Topology Table) is not
> implemented in ACPI boot, we will goto 'free_ci' with the following
> print:
>   Unable to detect cache hierarchy for CPU 0
>

The change itself looks good and I am fine with that. However,...

> But some other codes may still use 'num_leaves' to iterate through the

Can you point me exactly where it is used to make sure there are no
other issues associated with that.

> 'info_list', such as get_cpu_cacheinfo_id(). If 'info_list' is NULL , it
> would crash. So clear 'num_leaves' in free_cache_attributes().
>

And can you provide the crash dump please ? If we are not hitting any
issue and you just figured this with code inspection, that is fine. It
helps to determine if this needs to be backport or just good to have
clean up.

> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@...wei.com>
> ---
>  drivers/base/cacheinfo.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> index bfc0959..dad2962 100644
> --- a/drivers/base/cacheinfo.c
> +++ b/drivers/base/cacheinfo.c
> @@ -297,6 +297,7 @@ static void free_cache_attributes(unsigned int cpu)
>
>  	kfree(per_cpu_cacheinfo(cpu));
>  	per_cpu_cacheinfo(cpu) = NULL;
> +	cache_leaves(cpu) = 0;

I initially thought it might get used and crash in cache_shared_cpu_map_remove
but you are setting it later. So where do you suspect it to be used ? Sorry
if I am missing something obvious, looking at this code after long time.

-- 
Regards,
Sudeep