linux-kernel - Re: [PATCH] csky: syscache: Fixup duplicate cache flush

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] [day] [month] [year] [list]

Date:   Tue, 4 May 2021 10:21:12 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     guoren@...nel.org, arnd@...db.de
Cc:     linux-kernel@...r.kernel.org, linux-csky@...r.kernel.org,
        linux-arch@...r.kernel.org, Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH] csky: syscache: Fixup duplicate cache flush

On 5/4/21 12:14 AM, guoren@...nel.org wrote:
> From: Guo Ren <guoren@...ux.alibaba.com>
> 
> The current csky logic of sys_cacheflush is wrong, it'll cause
> icache flush call dcache flush again. Now fixup it with a
> conditional "break & fallthrough".
> 
> Fixes: 997153b9a75c ("csky: Add flush_icache_mm to defer flush icache all")
> Fixes: 0679d29d3e23 ("csky: fix syscache.c fallthrough warning")
> Signed-off-by: Guo Ren <guoren@...ux.alibaba.com>
> Cc: Randy Dunlap <rdunlap@...radead.org>
> Cc: Arnd Bergmann <arnd@...db.de>

Acked-by: Randy Dunlap <rdunlap@...radead.org>

more below:

> ---
>  arch/csky/mm/syscache.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/csky/mm/syscache.c b/arch/csky/mm/syscache.c
> index 4e51d63..a7b53b0 100644
> --- a/arch/csky/mm/syscache.c
> +++ b/arch/csky/mm/syscache.c
> @@ -12,15 +12,18 @@ SYSCALL_DEFINE3(cacheflush,
>  		int, cache)
>  {
>  	switch (cache) {
> -	case ICACHE:
>  	case BCACHE:
> -		flush_icache_mm_range(current->mm,
> -				(unsigned long)addr,
> -				(unsigned long)addr + bytes);
> -		fallthrough;
>  	case DCACHE:
>  		dcache_wb_range((unsigned long)addr,
>  				(unsigned long)addr + bytes);
> +		if (cache == BCACHE)
> +			fallthrough;
> +		else
> +			break;

I think the above would be more readable as:

		if (cache != BCACHE)
			break;
		fallthrough;

fwiw.

> +	case ICACHE:
> +		flush_icache_mm_range(current->mm,
> +				(unsigned long)addr,
> +				(unsigned long)addr + bytes);
>  		break;
>  	default:
>  		return -EINVAL;
> 

thanks.
-- 
~Randy