| 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
| ||
|
Message-ID: <6a9207ac-bbd5-e2a1-1d38-9a4ffe6bbeb1@flygoat.com>
Date: Fri, 29 Oct 2021 12:42:42 +0100
From: Jiaxun Yang <jiaxun.yang@...goat.com>
To: Geert Uytterhoeven <geert+renesas@...der.be>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Markos Chandras <markos.chandras@...tec.com>,
Ralf Baechle <ralf@...ux-mips.org>
Cc: linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mips: cm: Convert to bitfield API to fix out-of-bounds
access
在 2021/10/29 10:58, Geert Uytterhoeven 写道:
> mips_cm_error_report() extracts the cause and other cause from the error
> register using shifts. This works fine for the former, as it is stored
> in the top bits, and the shift will thus remove all non-related bits.
> However, the latter is stored in the bottom bits, hence thus needs masking
> to get rid of non-related bits. Without such masking, using it as an
> index into the cm2_causes[] array will lead to an out-of-bounds access,
> probably causing a crash.
>
> Fix this by using FIELD_GET() instead. Bite the bullet and convert all
> MIPS CM handling to the bitfield API, to improve readability and safety.
>
> Fixes: 3885c2b463f6a236 ("MIPS: CM: Add support for reporting CM cache errors")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
Reviewed-by: Jiaxun Yang <jiaxun.yang@...goat.com>
> ---
> Compile-tested only, but assembler output before/after compared.
> ---
> arch/mips/include/asm/mips-cm.h | 12 ++++++------
> arch/mips/kernel/mips-cm.c | 21 ++++++++++-----------
> 2 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/arch/mips/include/asm/mips-cm.h b/arch/mips/include/asm/mips-cm.h
> index aeae2effa123d7e2..23c67c0871b17c91 100644
> --- a/arch/mips/include/asm/mips-cm.h
> +++ b/arch/mips/include/asm/mips-cm.h
> @@ -11,6 +11,7 @@
> #ifndef __MIPS_ASM_MIPS_CM_H__
> #define __MIPS_ASM_MIPS_CM_H__
>
> +#include <linux/bitfield.h>
> #include <linux/bitops.h>
> #include <linux/errno.h>
>
> @@ -153,8 +154,8 @@ GCR_ACCESSOR_RO(32, 0x030, rev)
> #define CM_GCR_REV_MINOR GENMASK(7, 0)
>
> #define CM_ENCODE_REV(major, minor) \
> - (((major) << __ffs(CM_GCR_REV_MAJOR)) | \
> - ((minor) << __ffs(CM_GCR_REV_MINOR)))
> + (FIELD_PREP(CM_GCR_REV_MAJOR, major) | \
> + FIELD_PREP(CM_GCR_REV_MINOR, minor))
>
> #define CM_REV_CM2 CM_ENCODE_REV(6, 0)
> #define CM_REV_CM2_5 CM_ENCODE_REV(7, 0)
> @@ -362,10 +363,10 @@ static inline int mips_cm_revision(void)
> static inline unsigned int mips_cm_max_vp_width(void)
> {
> extern int smp_num_siblings;
> - uint32_t cfg;
>
> if (mips_cm_revision() >= CM_REV_CM3)
> - return read_gcr_sys_config2() & CM_GCR_SYS_CONFIG2_MAXVPW;
> + return FIELD_GET(CM_GCR_SYS_CONFIG2_MAXVPW,
> + read_gcr_sys_config2());
>
> if (mips_cm_present()) {
> /*
> @@ -373,8 +374,7 @@ static inline unsigned int mips_cm_max_vp_width(void)
> * number of VP(E)s, and if that ever changes then this will
> * need revisiting.
> */
> - cfg = read_gcr_cl_config() & CM_GCR_Cx_CONFIG_PVPE;
> - return (cfg >> __ffs(CM_GCR_Cx_CONFIG_PVPE)) + 1;
> + return FIELD_GET(CM_GCR_Cx_CONFIG_PVPE, read_gcr_cl_config()) + 1;
> }
>
> if (IS_ENABLED(CONFIG_SMP))
> diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
> index 90f1c3df1f0e495e..b4f7d950c84680d3 100644
> --- a/arch/mips/kernel/mips-cm.c
> +++ b/arch/mips/kernel/mips-cm.c
> @@ -221,8 +221,7 @@ static void mips_cm_probe_l2sync(void)
> phys_addr_t addr;
>
> /* L2-only sync was introduced with CM major revision 6 */
> - major_rev = (read_gcr_rev() & CM_GCR_REV_MAJOR) >>
> - __ffs(CM_GCR_REV_MAJOR);
> + major_rev = FIELD_GET(CM_GCR_REV_MAJOR, read_gcr_rev());
> if (major_rev < 6)
> return;
>
> @@ -306,13 +305,13 @@ void mips_cm_lock_other(unsigned int cluster, unsigned int core,
> preempt_disable();
>
> if (cm_rev >= CM_REV_CM3) {
> - val = core << __ffs(CM3_GCR_Cx_OTHER_CORE);
> - val |= vp << __ffs(CM3_GCR_Cx_OTHER_VP);
> + val = FIELD_PREP(CM3_GCR_Cx_OTHER_CORE, core) |
> + FIELD_PREP(CM3_GCR_Cx_OTHER_VP, vp);
>
> if (cm_rev >= CM_REV_CM3_5) {
> val |= CM_GCR_Cx_OTHER_CLUSTER_EN;
> - val |= cluster << __ffs(CM_GCR_Cx_OTHER_CLUSTER);
> - val |= block << __ffs(CM_GCR_Cx_OTHER_BLOCK);
> + val |= FIELD_PREP(CM_GCR_Cx_OTHER_CLUSTER, cluster);
> + val |= FIELD_PREP(CM_GCR_Cx_OTHER_BLOCK, block);
> } else {
> WARN_ON(cluster != 0);
> WARN_ON(block != CM_GCR_Cx_OTHER_BLOCK_LOCAL);
> @@ -342,7 +341,7 @@ void mips_cm_lock_other(unsigned int cluster, unsigned int core,
> spin_lock_irqsave(&per_cpu(cm_core_lock, curr_core),
> per_cpu(cm_core_lock_flags, curr_core));
>
> - val = core << __ffs(CM_GCR_Cx_OTHER_CORENUM);
> + val = FIELD_PREP(CM_GCR_Cx_OTHER_CORENUM, core);
> }
>
> write_gcr_cl_other(val);
> @@ -386,8 +385,8 @@ void mips_cm_error_report(void)
> cm_other = read_gcr_error_mult();
>
> if (revision < CM_REV_CM3) { /* CM2 */
> - cause = cm_error >> __ffs(CM_GCR_ERROR_CAUSE_ERRTYPE);
> - ocause = cm_other >> __ffs(CM_GCR_ERROR_MULT_ERR2ND);
> + cause = FIELD_GET(CM_GCR_ERROR_CAUSE_ERRTYPE, cm_error);
> + ocause = FIELD_GET(CM_GCR_ERROR_MULT_ERR2ND, cm_other);
>
> if (!cause)
> return;
> @@ -445,8 +444,8 @@ void mips_cm_error_report(void)
> ulong core_id_bits, vp_id_bits, cmd_bits, cmd_group_bits;
> ulong cm3_cca_bits, mcp_bits, cm3_tr_bits, sched_bit;
>
> - cause = cm_error >> __ffs64(CM3_GCR_ERROR_CAUSE_ERRTYPE);
> - ocause = cm_other >> __ffs(CM_GCR_ERROR_MULT_ERR2ND);
> + cause = FIELD_GET(CM3_GCR_ERROR_CAUSE_ERRTYPE, cm_error);
> + ocause = FIELD_GET(CM_GCR_ERROR_MULT_ERR2ND, cm_other);
>
> if (!cause)
> return;
Powered by blists - more mailing lists