Re: [PATCH] arm64: numa: check the node id before accessing node_to_cpumask_map

[Yunsheng Lin]

On 2019/8/30 13:55, Michal Hocko wrote:
> On Fri 30-08-19 10:26:31, Yunsheng Lin wrote:
>> Some buggy bios may not set the device' numa id, and dev_to_node
>> will return -1, which may cause global-out-of-bounds error
>> detected by KASAN.
> 
> Why should we workaround a buggy bios like that? Is it so widespread and
> no BIOS update available? Also, why is this arm64 specific?

For our case, there is BIOS update available. I just thought it might
be better to protect from this case when BIOS has not implemented the
device' numa id setting feature or the feature from BIOS has some bug.

It is not arm64 specific, right now I only have arm64 board. If it is
ok to protect this from the buggy BIOS, maybe all other arch can be
changed too.

> 
>> This patch changes cpumask_of_node to return cpu_none_mask if the
>> node is not valid, and sync the cpumask_of_node between the
>> cpumask_of_node function in numa.h and numa.c.
> 
> Why?

When CONFIG_DEBUG_PER_CPU_MAPS is defined, the cpumask_of_node() in
numa.c is used, if not, the cpumask_of_node() in numa.h is used.

I am not sure why there is difference between them, and it is there
when since the below commit:
1a2db300348b ("arm64, numa: Add NUMA support for arm64 platforms.")

I synced them to keep them consistent whether CONFIG_DEBUG_PER_CPU_MAPS
is defined.

> 
>> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx>
>> ---
>>  arch/arm64/include/asm/numa.h | 6 ++++++
>>  arch/arm64/mm/numa.c          | 2 +-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
>> index 626ad01..da891ed 100644
>> --- a/arch/arm64/include/asm/numa.h
>> +++ b/arch/arm64/include/asm/numa.h
>> @@ -25,6 +25,12 @@ const struct cpumask *cpumask_of_node(int node);
>>  /* Returns a pointer to the cpumask of CPUs on Node 'node'. */
>>  static inline const struct cpumask *cpumask_of_node(int node)
>>  {
>> +	if (node >= nr_node_ids || node < 0)
>> +		return cpu_none_mask;
>> +
>> +	if (!node_to_cpumask_map[node])
>> +		return cpu_online_mask;
>> +
>>  	return node_to_cpumask_map[node];
>>  }
>>  #endif
>> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
>> index 4f241cc..3846313 100644
>> --- a/arch/arm64/mm/numa.c
>> +++ b/arch/arm64/mm/numa.c
>> @@ -46,7 +46,7 @@ EXPORT_SYMBOL(node_to_cpumask_map);
>>   */
>>  const struct cpumask *cpumask_of_node(int node)
>>  {
>> -	if (WARN_ON(node >= nr_node_ids))
>> +	if (WARN_ON(node >= nr_node_ids || node < 0))
>>  		return cpu_none_mask;
>>  
>>  	if (WARN_ON(node_to_cpumask_map[node] == NULL))
>> -- 
>> 2.8.1
>