powerpc/smp: Handle missing ibm,thread-groups in parse_thread_groups()
Ritesh Harjani (IBM)
reviewerPATCH 1/1 powerpc/smp: Handle missing ibm,thread-groups in parse_thread_groups()
Power systems can have ibm,thread-groups properly absent like in one of the PowerNV Power9 boxes where there was a warning splat reported on 7.4-rc2:
WARNING: mm/slub.c:5459 at ___kmalloc_large_node+0x174/0x1a0, CPU#124: swapper/124/1 ... Call Trace: __kmalloc_noprof+0x490/0x690 _kzalloc_noprof.constprop.0+0x1c/0x7c init_thread_group_cache_map+0xd0/0x2d4 smp_prepare_cpus+0x2bc/0x484 kernel_init_freeable+0x168/0x3c4 kernel_init+0x34/0x1ac
This happens because today we don't check for whether the properly is present or not and directly pass the return value of of_property_count_u32_elems(), to the allocator.
This went unnoticed because earlier kcalloc() caught the overflow due to check_mul_overflow() check and returned NULL.
But since commit 0af298e62e5c ("treewide: refresh kmalloc_obj() conversions") switched this call to kzalloc_objs(), size_mul() saturates the size to SIZE_MAX instead and we now get a warning at boot.
Bail out early if the property is missing or empty. Follow the return value as documented in kernel doc of parse_thread_groups() function.
--- arch/powerpc/kernel/smp.c | 4 ++++ 1 file changed, 4 insertions(+)
arch/powerpc/kernel/smp.c +4 -0
@@ -824,6 +824,10 @@ static int parse_thread_groups(struct device_node *dn,int i =0 ; count =of_property_count_u32_elems (dn,"ibm,thread-groups" ); +if (count <0 ) +return count; +if (!count) +return -ENODATA ; thread_group_array =kzalloc_objs (u32, count);if (!thread_group_array)return -ENOMEM ;
--
2.39.5
Shrikanth Hegde
reviewerHi Ritesh,
+cc nathan, Kees,
Quoted 39 lines · Ritesh Harjani (IBM)
Power systems can have ibm,thread-groups properly absent like in one of the PowerNV Power9 boxes where there was a warning splat reported on 7.4-rc2:
WARNING: mm/slub.c:5459 at ___kmalloc_large_node+0x174/0x1a0, CPU#124: swapper/124/1 ... Call Trace: __kmalloc_noprof+0x490/0x690 _kzalloc_noprof.constprop.0+0x1c/0x7c init_thread_group_cache_map+0xd0/0x2d4 smp_prepare_cpus+0x2bc/0x484 kernel_init_freeable+0x168/0x3c4 kernel_init+0x34/0x1acThis happens because today we don't check for whether the properly is present or not and directly pass the return value of
of_property_count_u32_elems(), to the allocator.This went unnoticed because earlier
kcalloc()caught the overflow due tocheck_mul_overflow()check and returned NULL.
But since commit0af298e62e5c("treewide: refreshkmalloc_obj()conversions") switched this call tokzalloc_objs(),size_mul()saturates the size toSIZE_MAXinstead and we now get a warning at boot.Bail out early if the property is missing or empty. Follow the return value as documented in kernel doc of
parse_thread_groups()function.Fixes 790a1662d3a2 ("powerpc/smp: Parse ibm,thread-groups with multiple properties")Reported-by Madhavan Srinivasan <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>--- arch/powerpc/kernel/smp.c | 4 ++++ 1 file changed, 4 insertions(+)arch/powerpc/kernel/smp.c +0
-0
arch/powerpc/kernel/smp.c static int parse_thread_groups(struct device_node *dn, · line 830
Quoted 7 lines
arch/powerpc/kernel/smp.c +4
-0@@ -824,6 +824,10 @@ static int parse_thread_groups(struct device_node *dn,int i =0 ; count =of_property_count_u32_elems (dn,"ibm,thread-groups" ); +if (count <0 ) +return count; +if (!count) +return -ENODATA ;
This zero count check is not necessary.
Code doesn;t dereference thread_group_array if the count is 0.
...
while (i < count && property_idx < MAX_THREAD_GROUP_PROPERTIES) {
...
out_free:
kfree(thread_group_array);
return ret;Which should naturally pass to kfree which is expected to handle ZERO_SIZE_PTR.
No?
Quoted 6 lines
arch/powerpc/kernel/smp.c +0
-1thread_group_array =kzalloc_objs (u32, count);if (!thread_group_array)return -ENOMEM ; --2.39.5
Nathan reported the same issue on QEMU.The same symptoms.
https://lore.kernel.org/all/20261109-powerpc-smp-handle-count-err-v1-1-e777fde7f71a@kernel.org/
Ritesh Harjani (IBM)
reviewerQuoted 46 lines · Shrikanth Hegde
Hi Ritesh,
+cc nathan, Kees,
Quoted 41 lines · Ritesh Harjani (IBM)
Power systems can have ibm,thread-groups properly absent like in one of the PowerNV Power9 boxes where there was a warning splat reported on 7.4-rc2:
WARNING: mm/slub.c:5459 at ___kmalloc_large_node+0x174/0x1a0, CPU#124: swapper/124/1 ... Call Trace: __kmalloc_noprof+0x490/0x690 _kzalloc_noprof.constprop.0+0x1c/0x7c init_thread_group_cache_map+0xd0/0x2d4 smp_prepare_cpus+0x2bc/0x484 kernel_init_freeable+0x168/0x3c4 kernel_init+0x34/0x1acThis happens because today we don't check for whether the properly is present or not and directly pass the return value of
of_property_count_u32_elems(), to the allocator.This went unnoticed because earlier
kcalloc()caught the overflow due tocheck_mul_overflow()check and returned NULL.
But since commit0af298e62e5c("treewide: refreshkmalloc_obj()conversions") switched this call tokzalloc_objs(),size_mul()saturates the size toSIZE_MAXinstead and we now get a warning at boot.Bail out early if the property is missing or empty. Follow the return value as documented in kernel doc of
parse_thread_groups()function.Fixes 790a1662d3a2 ("powerpc/smp: Parse ibm,thread-groups with multiple properties")Reported-by Madhavan Srinivasan <[email protected]>Signed-off-by Ritesh Harjani (IBM) <[email protected]>--- arch/powerpc/kernel/smp.c | 4 ++++ 1 file changed, 4 insertions(+)arch/powerpc/kernel/smp.c +0
-0@@ -824,6 +824,10 @@ static int parse_thread_groups(struct device_node *dn,int i =0 ;
Quoted 8 lines
arch/powerpc/kernel/smp.c static int parse_thread_groups(struct device_node *dn, · line 830
Quoted 5 lines
arch/powerpc/kernel/smp.c +4
-0count =of_property_count_u32_elems (dn,"ibm,thread-groups" ); +if (count <0 ) +return count; +if (!count) +return -ENODATA ;This zero count check is not necessary.
yes, not strictly necessary, but good to have.
Quoted 10 lines
Quoted 6 lines
arch/powerpc/kernel/smp.c +0
-1thread_group_array =kzalloc_objs (u32, count);if (!thread_group_array)return -ENOMEM ; --2.39.5
Nathan reported the same issue on QEMU.The same symptoms.
https://lore.kernel.org/all/20261109-powerpc-smp-handle-count-err-v1-1-e777fde7f71a@kernel.org/
I didn't notice this. Thanks for pointing it out. In that case let's drop this patch.
-ritesh