linux-kernel - Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg

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: <0726a5b9-fff6-a15c-e705-db7abd4b1abd@arm.com>
Date:   Tue, 26 May 2020 15:04:14 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     anshuman.khandual@....com, linux-arm-kernel@...ts.infradead.org
Cc:     mark.rutland@....com, catalin.marinas@....com, will@...nel.org,
        broonie@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()

On 05/26/2020 02:39 PM, Anshuman Khandual wrote:
> There is no way to proceed when requested register could not be searched in
> arm64_ftr_reg[]. Requesting for a non present register would be an error as
> well. Hence lets just WARN_ON() when search fails in get_arm64_ftr_reg()
> rather than checking for return value and doing a BUG_ON() instead in some
> individual callers. But there are also caller instances that dont error out
> when register search fails. Add a new helper get_arm64_ftr_reg_nowarn() for
> such cases.
> 
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@....com>
> Cc: Mark Brown <broonie@...nel.org>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
> ---
> Changes in V2:
> 
> - Added get_arm64_ftr_reg_nowarn() per Will
> - read_sanitised_ftr_reg() returns 0 when register search fails per Catalin
> 
> Changes in V1: (https://patchwork.kernel.org/patch/11559083/)
> 
>   arch/arm64/kernel/cpufeature.c | 42 +++++++++++++++++++++++-----------
>   1 file changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index bc5048f152c1..f4555b9d145c 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -548,16 +548,16 @@ static int search_cmp_ftr_reg(const void *id, const void *regp)
>   }
>   

...

>   static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
>   			       s64 ftr_val)
>   {
> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
>   	const struct arm64_ftr_bits *ftrp;
>   	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>   
> -	BUG_ON(!reg);
> -
>   	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
>   		u64 ftr_mask = arm64_ftr_mask(ftrp);
>   		s64 ftr_new = arm64_ftr_value(ftrp, new);
> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
>   {
>   	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>   
> -	BUG_ON(!regp);
>   	update_cpu_ftr_reg(regp, val);
>   	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
>   		return 0;
> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
>   	const struct arm64_ftr_bits *ftrp;
>   	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>   
> -	if (WARN_ON(!regp))
> -		return;
> -

You need to return here, on !regp. Rest looks fine to me.

Suzuki