On Mon, Jan 13, 2025 at 6:04 AM Sasha Levin sashal@kernel.org wrote:

This is a note to let you know that I've just added the patch titled

mm: zswap: fix race between [de]compression and CPU hotunplug

to the 6.12-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...

The filename of the patch is: mm-zswap-fix-race-between-de-compression-and-cpu-hot.patch and it can be found in the queue-6.12 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.

Please drop this patch from v6.12 (and all stable trees) as it has a bug, as I pointed out in [1] (was that the correct place to surface this?).

Andrew's latest PR contains a revert of this patch (and alternative fix) [2].

Thanks!

[1]https://lore.kernel.org/stable/CAJD7tkaiUA6J1nb0=1ELpUD0OgjoD+tft-iPqbPdyXCp... [2]https://lore.kernel.org/lkml/20250113000543.862792e948c2646032a477e0@linux-f...

commit c56e79d453ef5b5fc6ded252bc6ba461f12946ba Author: Yosry Ahmed yosryahmed@google.com Date: Thu Dec 19 21:24:37 2024 +0000

mm: zswap: fix race between [de]compression and CPU hotunplug

[ Upstream commit eaebeb93922ca6ab0dd92027b73d0112701706ef ]

In zswap_compress() and zswap_decompress(), the per-CPU acomp_ctx of the
current CPU at the beginning of the operation is retrieved and used
throughout.  However, since neither preemption nor migration are disabled,
it is possible that the operation continues on a different CPU.

If the original CPU is hotunplugged while the acomp_ctx is still in use,
we run into a UAF bug as the resources attached to the acomp_ctx are freed
during hotunplug in zswap_cpu_comp_dead().

The problem was introduced in commit 1ec3b5fe6eec ("mm/zswap: move to use
crypto_acomp API for hardware acceleration") when the switch to the
crypto_acomp API was made.  Prior to that, the per-CPU crypto_comp was
retrieved using get_cpu_ptr() which disables preemption and makes sure the
CPU cannot go away from under us.  Preemption cannot be disabled with the
crypto_acomp API as a sleepable context is needed.

Commit 8ba2f844f050 ("mm/zswap: change per-cpu mutex and buffer to
per-acomp_ctx") increased the UAF surface area by making the per-CPU
buffers dynamic, adding yet another resource that can be freed from under
zswap compression/decompression by CPU hotunplug.

There are a few ways to fix this:
(a) Add a refcount for acomp_ctx.
(b) Disable migration while using the per-CPU acomp_ctx.
(c) Disable CPU hotunplug while using the per-CPU acomp_ctx by holding
the CPUs read lock.

Implement (c) since it's simpler than (a), and (b) involves using
migrate_disable() which is apparently undesired (see huge comment in
include/linux/preempt.h).

Link: https://lkml.kernel.org/r/20241219212437.2714151-1-yosryahmed@google.com
Fixes: 1ec3b5fe6eec ("mm/zswap: move to use crypto_acomp API for hardware acceleration")
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Reported-by: Johannes Weiner <hannes@cmpxchg.org>
Closes: https://lore.kernel.org/lkml/20241113213007.GB1564047@cmpxchg.org/
Reported-by: Sam Sun <samsun1006219@gmail.com>
Closes: https://lore.kernel.org/lkml/CAEkJfYMtSdM5HceNsXUDf5haghD5+o2e7Qv4OcuruL4tPg6OaQ@mail.gmail.com/
Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev>
Acked-by: Barry Song <baohua@kernel.org>
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Cc: Vitaly Wool <vitalywool@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/mm/zswap.c b/mm/zswap.c index 0030ce8fecfc..c86d4bcbb447 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -875,6 +875,18 @@ static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node) return 0; }

+/* Prevent CPU hotplug from freeing up the per-CPU acomp_ctx resources */ +static struct crypto_acomp_ctx *acomp_ctx_get_cpu(struct crypto_acomp_ctx __percpu *acomp_ctx) +{

•   cpus_read_lock();
  

•   return raw_cpu_ptr(acomp_ctx);
  

+}

+static void acomp_ctx_put_cpu(void) +{

•   cpus_read_unlock();
  

+}

static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) { struct crypto_acomp_ctx *acomp_ctx; @@ -887,8 +899,7 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) gfp_t gfp; u8 *dst;

•   acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
  

•   acomp_ctx = acomp_ctx_get_cpu(entry->pool->acomp_ctx);
    mutex_lock(&acomp_ctx->mutex);
  
    dst = acomp_ctx->buffer;
  

@@ -944,6 +955,7 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) zswap_reject_alloc_fail++;

    mutex_unlock(&acomp_ctx->mutex);

•   acomp_ctx_put_cpu();
    return comp_ret == 0 && alloc_ret == 0;
  

}

@@ -954,7 +966,7 @@ static void zswap_decompress(struct zswap_entry *entry, struct folio *folio) struct crypto_acomp_ctx *acomp_ctx; u8 *src;

•   acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
  

•   acomp_ctx = acomp_ctx_get_cpu(entry->pool->acomp_ctx);
    mutex_lock(&acomp_ctx->mutex);
  
    src = zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO);
  

@@ -984,6 +996,7 @@ static void zswap_decompress(struct zswap_entry *entry, struct folio *folio)

    if (src != acomp_ctx->buffer)
            zpool_unmap_handle(zpool, entry->handle);

•   acomp_ctx_put_cpu();
  

}

/*********************************