linux-kernel - Re: [PATCH] mm: vmalloc: Check if a hash-index is in cpu_possible

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] [day] [month] [year] [list]

Message-ID: <ZnwlaGtDpMz8rALN@MiWiFi-R3L-srv>
Date: Wed, 26 Jun 2024 22:27:52 +0800
From: Baoquan He <bhe@...hat.com>
To: "Uladzislau Rezki (Sony)" <urezki@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Hailong Liu <hailong.liu@...o.com>,
	Christoph Hellwig <hch@...radead.org>,
	Nick Bowler <nbowler@...conx.ca>,
	Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>
Subject: Re: [PATCH] mm: vmalloc: Check if a hash-index is in
 cpu_possible_mask

On 06/26/24 at 01:43pm, Uladzislau Rezki (Sony) wrote:
> The problem is that there are systems where cpu_possible_mask
> has gaps between set CPUs, for example SPARC. In this scenario
> addr_to_vb_xa() hash function can return an index which accesses
> to not-possible and not setup CPU area using per_cpu() macro.
> 
> A per-cpu vmap_block_queue is also used as hash table assuming
                                                        ~~~~~
> that incorrectly assumes the cpu_possible_mask has not gaps.
                   ~~~~~~
                         Typo of duplicated word?

Other than above nit, this looks good to me. Thanks.

Reviewed-by: Baoquan He <bhe@...hat.com>

> 
> Fix it by adjusting an index to a next possible CPU.
> 
> Fixes: 062eacf57ad9 ("mm: vmalloc: remove a global vmap_blocks xarray")
> Reported-by: Nick Bowler <nbowler@...conx.ca>
> Closes: https://lore.kernel.org/linux-kernel/ZntjIE6msJbF8zTa@MiWiFi-R3L-srv/T/
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@...il.com>
> ---
>  mm/vmalloc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index b4c42da9f3901..6b783baf12a14 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2544,7 +2544,15 @@ static DEFINE_PER_CPU(struct vmap_block_queue, vmap_block_queue);
>  static struct xarray *
>  addr_to_vb_xa(unsigned long addr)
>  {
> -	int index = (addr / VMAP_BLOCK_SIZE) % num_possible_cpus();
> +	int index = (addr / VMAP_BLOCK_SIZE) % nr_cpu_ids;
> +
> +	/*
> +	 * Please note, nr_cpu_ids points on a highest set
> +	 * possible bit, i.e. we never invoke cpumask_next()
> +	 * if an index points on it which is nr_cpu_ids - 1.
> +	 */
> +	if (!cpu_possible(index))
> +		index = cpumask_next(index, cpu_possible_mask);
>  
>  	return &per_cpu(vmap_block_queue, index).vmap_blocks;
>  }
> -- 
> 2.39.2
>