linux-kernel - Re: [PATCH] mm/slab: protect cache_reap() against CPU and memory hot plug operations

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: <20190312145813.GS5721@dhcp22.suse.cz>
Date:   Tue, 12 Mar 2019 15:58:13 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Laurent Dufour <ldufour@...ux.ibm.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        stable@...r.kernel.org, Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] mm/slab: protect cache_reap() against CPU and memory hot
 plug operations

On Mon 11-03-19 20:17:01, Laurent Dufour wrote:
> The commit 95402b382901 ("cpu-hotplug: replace per-subsystem mutexes with
> get_online_cpus()") remove the CPU_LOCK_ACQUIRE operation which was use to
> grap the cache_chain_mutex lock which was protecting cache_reap() against
> CPU hot plug operations.
> 
> Later the commit 18004c5d4084 ("mm, sl[aou]b: Use a common mutex
> definition") changed cache_chain_mutex to slab_mutex but this didn't help
> fixing the missing the cache_reap() protection against CPU hot plug
> operations.
> 
> Here we are stopping the per cpu worker while holding the slab_mutex to
> ensure that cache_reap() is not running in our back and will not be
> triggered anymore for this cpu.
> 
> This patch fixes that race leading to SLAB's data corruption when CPU
> hotplug are triggered. We hit it while doing partition migration on PowerVM
> leading to CPU reconfiguration through the CPU hotplug mechanism.

What is the actual race? slab_offline_cpu calls cancel_delayed_work_sync
so it removes a pending item and waits for the item to finish if they run
concurently. So why do we need an additional lock?

> This fix is covering kernel containing to the commit 6731d4f12315 ("slab:
> Convert to hotplug state machine"), ie 4.9.1, earlier kernel needs a
> slightly different patch.
> 
> Cc: stable@...r.kernel.org
> Cc: Christoph Lameter <cl@...ux.com>
> Cc: Pekka Enberg <penberg@...nel.org>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@....com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Laurent Dufour <ldufour@...ux.ibm.com>
> ---
>  mm/slab.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 28652e4218e0..ba499d90f27f 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1103,6 +1103,7 @@ static int slab_online_cpu(unsigned int cpu)
>  
>  static int slab_offline_cpu(unsigned int cpu)
>  {
> +	mutex_lock(&slab_mutex);
>  	/*
>  	 * Shutdown cache reaper. Note that the slab_mutex is held so
>  	 * that if cache_reap() is invoked it cannot do anything
> @@ -1112,6 +1113,7 @@ static int slab_offline_cpu(unsigned int cpu)
>  	cancel_delayed_work_sync(&per_cpu(slab_reap_work, cpu));
>  	/* Now the cache_reaper is guaranteed to be not running. */
>  	per_cpu(slab_reap_work, cpu).work.func = NULL;
> +	mutex_unlock(&slab_mutex);
>  	return 0;
>  }
>  
> -- 
> 2.21.0

-- 
Michal Hocko
SUSE Labs