[PATCH] ARM: sunxi: fix return code check of of_property_match_string

[PATCH] ARM: sunxi: fix return code check of of_property_match_string

[ndesaulniers]

of_property_match_string returns an int; either an index from 0 or
greater if successful or negative on failure.

Fixes the following splat observed with UBSAN:
[    0.166489][    T1] UBSAN: array-index-out-of-bounds in arch/arm/mach-sunxi/mc_smp.c:810:29
[    0.166934][    T1] index 2 is out of range for type 'sunxi_mc_smp_data [2]'
[    0.167206][    T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.4.0-rc2 #1
[    0.167515][    T1] Hardware name: Generic DT based system
[    0.167727][    T1]  unwind_backtrace from show_stack+0x18/0x1c
[    0.167979][    T1]  show_stack from dump_stack_lvl+0x68/0x90
[    0.168226][    T1]  dump_stack_lvl from ubsan_epilogue+0x8/0x34
[    0.168474][    T1]  ubsan_epilogue from __ubsan_handle_out_of_bounds+0x78/0x80
[    0.168760][    T1]  __ubsan_handle_out_of_bounds from sunxi_mc_smp_init+0xe8/0x574
[    0.169100][    T1]  sunxi_mc_smp_init from do_one_initcall+0x178/0x9c8
[    0.169364][    T1]  do_one_initcall from kernel_init_freeable+0x1dc/0x28c
[    0.169661][    T1]  kernel_init_freeable from kernel_init+0x20/0x164
[    0.169912][    T1]  kernel_init from ret_from_fork+0x14/0x2c

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm/mach-sunxi/mc_smp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index cb63921232a6..c7635e9e36ef 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -782,7 +782,7 @@ static int __init sunxi_mc_smp_init(void)
 	struct device_node *node;
 	struct resource res;
 	void __iomem *addr;
-	int i, ret;
+	int i, ret = -1;
 
 	/*
 	 * Don't bother checking the "cpus" node, as an enable-method
@@ -803,10 +803,13 @@ static int __init sunxi_mc_smp_init(void)
 	for (i = 0; i < ARRAY_SIZE(sunxi_mc_smp_data); i++) {
 		ret = of_property_match_string(node, "enable-method",
 					       sunxi_mc_smp_data[i].enable_method);
-		if (!ret)
+		if (ret >= 0)
 			break;
 	}
 
+	if (ret < 0)
+		return -ENODEV;
+
 	is_a83t = sunxi_mc_smp_data[i].is_a83t;
 
 	of_node_put(node);

---
base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
change-id: 20230516-sunxi-bc9dda2d228e

Best regards,
-- 
Nick Desaulniers <ndesaulniers@google.com>

Re: [PATCH] ARM: sunxi: fix return code check of of_property_match_string

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

+William,

On 5/16/23 09:35, ndesaulniers@google.com wrote:
> of_property_match_string returns an int; either an index from 0 or
> greater if successful or negative on failure.
> 
> Fixes the following splat observed with UBSAN:
> [    0.166489][    T1] UBSAN: array-index-out-of-bounds in arch/arm/mach-sunxi/mc_smp.c:810:29
> [    0.166934][    T1] index 2 is out of range for type 'sunxi_mc_smp_data [2]'
> [    0.167206][    T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.4.0-rc2 #1
> [    0.167515][    T1] Hardware name: Generic DT based system
> [    0.167727][    T1]  unwind_backtrace from show_stack+0x18/0x1c
> [    0.167979][    T1]  show_stack from dump_stack_lvl+0x68/0x90
> [    0.168226][    T1]  dump_stack_lvl from ubsan_epilogue+0x8/0x34
> [    0.168474][    T1]  ubsan_epilogue from __ubsan_handle_out_of_bounds+0x78/0x80
> [    0.168760][    T1]  __ubsan_handle_out_of_bounds from sunxi_mc_smp_init+0xe8/0x574
> [    0.169100][    T1]  sunxi_mc_smp_init from do_one_initcall+0x178/0x9c8
> [    0.169364][    T1]  do_one_initcall from kernel_init_freeable+0x1dc/0x28c
> [    0.169661][    T1]  kernel_init_freeable from kernel_init+0x20/0x164
> [    0.169912][    T1]  kernel_init from ret_from_fork+0x14/0x2c
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

William had a similar fix submitted back in September of last year, but 
I do not believe it got applied either:

https://lore.kernel.org/r/20220929012944.454613-1-william.zhang@broadcom.com

lore was not able to find it, but above is the message ID, and attached 
is his original patch.
-- 
Florian

[-- Attachment #2: [PATCH] ARM: sun9i: smp: fix out-of-bounds access error - William Zhang <william.zhang@broadcom.com> - 2022-09-28 1829.eml --]
[-- Type: application/x-extension-eml, Size: 13548 bytes --]

Re: [PATCH] ARM: sunxi: fix return code check of of_property_match_string

[Nick Desaulniers]

On Tue, May 16, 2023 at 11:34 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> +William,
>
> On 5/16/23 09:35, ndesaulniers@google.com wrote:
> > of_property_match_string returns an int; either an index from 0 or
> > greater if successful or negative on failure.
> >
> > Fixes the following splat observed with UBSAN:
> > [    0.166489][    T1] UBSAN: array-index-out-of-bounds in arch/arm/mach-sunxi/mc_smp.c:810:29
> > [    0.166934][    T1] index 2 is out of range for type 'sunxi_mc_smp_data [2]'
> > [    0.167206][    T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.4.0-rc2 #1
> > [    0.167515][    T1] Hardware name: Generic DT based system
> > [    0.167727][    T1]  unwind_backtrace from show_stack+0x18/0x1c
> > [    0.167979][    T1]  show_stack from dump_stack_lvl+0x68/0x90
> > [    0.168226][    T1]  dump_stack_lvl from ubsan_epilogue+0x8/0x34
> > [    0.168474][    T1]  ubsan_epilogue from __ubsan_handle_out_of_bounds+0x78/0x80
> > [    0.168760][    T1]  __ubsan_handle_out_of_bounds from sunxi_mc_smp_init+0xe8/0x574
> > [    0.169100][    T1]  sunxi_mc_smp_init from do_one_initcall+0x178/0x9c8
> > [    0.169364][    T1]  do_one_initcall from kernel_init_freeable+0x1dc/0x28c
> > [    0.169661][    T1]  kernel_init_freeable from kernel_init+0x20/0x164
> > [    0.169912][    T1]  kernel_init from ret_from_fork+0x14/0x2c
> >
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> William had a similar fix submitted back in September of last year, but
> I do not believe it got applied either:
>
> https://lore.kernel.org/r/20220929012944.454613-1-william.zhang@broadcom.com
>
> lore was not able to find it, but above is the message ID, and attached
> is his original patch.

Hi William,
How would you like to proceed here? I don't want to step on any toes
if there's an existing patch.

> --
> Florian



-- 
Thanks,
~Nick Desaulniers