[PATCH] mm/sparse: Check the return value of first_present_section_nr()

Liam Ni posted 1 patch 2 years, 7 months ago
mm/sparse.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] mm/sparse: Check the return value of first_present_section_nr()
Posted by Liam Ni 2 years, 7 months ago
first_present_section_nr() may return -1,
which means there is no present section in system,or other errors,
so we cause panic here.

Signed-off-by: Liam Ni <zhiguangni01@gmail.com>
---
 mm/sparse.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/sparse.c b/mm/sparse.c
index b8d5d58fe240..175727e10deb 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -564,6 +564,8 @@ void __init sparse_init(void)
 	memblocks_present();
 
 	pnum_begin = first_present_section_nr();
+	if (pnum_begin == -1)
+		panic("There is no present section in system\n");
 	nid_begin = sparse_early_nid(__nr_to_section(pnum_begin));
 
 	/* Setup pageblock_order for HUGETLB_PAGE_SIZE_VARIABLE */
-- 
2.25.1
Re: [PATCH] mm/sparse: Check the return value of first_present_section_nr()
Posted by David Hildenbrand 2 years, 7 months ago
On 19.06.23 06:44, Liam Ni wrote:
> first_present_section_nr() may return -1,
> which means there is no present section in system,or other errors,
> so we cause panic here.
> 
> Signed-off-by: Liam Ni <zhiguangni01@gmail.com>
> ---
>   mm/sparse.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/mm/sparse.c b/mm/sparse.c
> index b8d5d58fe240..175727e10deb 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -564,6 +564,8 @@ void __init sparse_init(void)
>   	memblocks_present();
>   
>   	pnum_begin = first_present_section_nr();
> +	if (pnum_begin == -1)
> +		panic("There is no present section in system\n");
>   	nid_begin = sparse_early_nid(__nr_to_section(pnum_begin));
>   
>   	/* Setup pageblock_order for HUGETLB_PAGE_SIZE_VARIABLE */

That would mean that __section_mark_present() was never called, implying 
  memory_present() / memblocks_present() was never called.

... but sparse_init() calles memblocks_present() itself.

So what's left would be, that there is absolutely no memory in the 
system such that memblocks_present() couldn't mark anything present ... 
which sound pretty much impossible, unless the arch setup code is deeply 
flawed such that everything else would be broken as well.

Don't think this will ever trigger, and, therefore, this is not really 
required.



-- 
Cheers,

David / dhildenb