linux-kernel - Re: [PATCH v2 2/9] x86: numa: check the node id consistently for x86

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190831085539.GG2369@hirez.programming.kicks-ass.net>
Date:   Sat, 31 Aug 2019 10:55:39 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Yunsheng Lin <linyunsheng@...wei.com>
Cc:     catalin.marinas@....com, will@...nel.org, mingo@...hat.com,
        bp@...en8.de, rth@...ddle.net, ink@...assic.park.msu.ru,
        mattst88@...il.com, benh@...nel.crashing.org, paulus@...ba.org,
        mpe@...erman.id.au, heiko.carstens@...ibm.com, gor@...ux.ibm.com,
        borntraeger@...ibm.com, ysato@...rs.sourceforge.jp,
        dalias@...c.org, davem@...emloft.net, ralf@...ux-mips.org,
        paul.burton@...s.com, jhogan@...nel.org, jiaxun.yang@...goat.com,
        chenhc@...ote.com, akpm@...ux-foundation.org, rppt@...ux.ibm.com,
        anshuman.khandual@....com, tglx@...utronix.de, cai@....pw,
        robin.murphy@....com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, hpa@...or.com, x86@...nel.org,
        dave.hansen@...ux.intel.com, luto@...nel.org, len.brown@...el.com,
        axboe@...nel.dk, dledford@...hat.com, jeffrey.t.kirsher@...el.com,
        linux-alpha@...r.kernel.org, nfont@...ux.vnet.ibm.com,
        naveen.n.rao@...ux.vnet.ibm.com, mwb@...ux.vnet.ibm.com,
        linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
        tbogendoerfer@...e.de, linux-mips@...r.kernel.org,
        linuxarm@...wei.com
Subject: Re: [PATCH v2 2/9] x86: numa: check the node id consistently for x86

On Sat, Aug 31, 2019 at 01:58:16PM +0800, Yunsheng Lin wrote:
> According to Section 6.2.14 from ACPI spec 6.3 [1], the setting
> of proximity domain is optional, as below:
> 
> This optional object is used to describe proximity domain
> associations within a machine. _PXM evaluates to an integer
> that identifies a device as belonging to a Proximity Domain
> defined in the System Resource Affinity Table (SRAT).

That's just words.. what does it actually mean?

> This patch checks node id with the below case before returning
> node_to_cpumask_map[node]:
> 1. if node_id >= nr_node_ids, return cpu_none_mask
> 2. if node_id < 0, return cpu_online_mask
> 3. if node_to_cpumask_map[node_id] is NULL, return cpu_online_mask
> 
> [1] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
> 
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> ---
>  arch/x86/include/asm/topology.h | 6 ++++++
>  arch/x86/mm/numa.c              | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index 4b14d23..f36e9c8 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -69,6 +69,12 @@ extern 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)
> +		return cpu_none_mask;
> +
> +	if (node < 0 || !node_to_cpumask_map[node])
> +		return cpu_online_mask;
> +
>  	return node_to_cpumask_map[node];
>  }
>  #endif

I _reallly_ hate this. Users are expected to use valid numa ids. Now
we're adding all this checking to all users. Why do we want to do that?

Using '(unsigned)node >= nr_nods_ids' is an error.

> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index e6dad60..5e393d2 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -868,7 +868,7 @@ const struct cpumask *cpumask_of_node(int node)
>  		dump_stack();
>  		return cpu_none_mask;
>  	}
> -	if (node_to_cpumask_map[node] == NULL) {
> +	if (node < 0 || !node_to_cpumask_map[node]) {
>  		printk(KERN_WARNING
>  			"cpumask_of_node(%d): no node_to_cpumask_map!\n",
>  			node);
> -- 
> 2.8.1
>