Skip to content

Commit c9b94ff

Browse files
author
Rafael Aquini
committed
mm: zswap: properly synchronize freeing resources during CPU hotunplug
JIRA: https://issues.redhat.com/browse/RHEL-78678 CVE: CVE-2025-21693 Conflicts: * the order and context of several hunks in this backport does not match their upstream commit of reference because RHEL-9 misses the full series (and dependencies) that introduces commit fa9ad6e ("mm: zswap: break out zwap_compress()"). These churny changesets, however, are mostly cosmetical and none of them are required for the purpose of this backport. commit 12dcb0e Author: Yosry Ahmed <[email protected]> Date: Wed Jan 8 22:24:41 2025 +0000 mm: zswap: properly synchronize freeing resources during CPU hotunplug 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 some of the resources attached to the acomp_ctx are freed during hotunplug in zswap_cpu_comp_dead() (i.e. acomp_ctx.buffer, acomp_ctx.req, or acomp_ctx.acomp). The problem was introduced in commit 1ec3b5f ("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. Use the acomp_ctx.mutex to synchronize CPU hotplug callbacks allocating and freeing resources with compression/decompression paths. Make sure that acomp_ctx.req is NULL when the resources are freed. In the compression/decompression paths, check if acomp_ctx.req is NULL after acquiring the mutex (meaning the CPU was offlined) and retry on the new CPU. The initialization of acomp_ctx.mutex is moved from the CPU hotplug callback to the pool initialization where it belongs (where the mutex is allocated). In addition to adding clarity, this makes sure that CPU hotplug cannot reinitialize a mutex that is already locked by compression/decompression. Previously a fix was attempted by holding cpus_read_lock() [1]. This would have caused a potential deadlock as it is possible for code already holding the lock to fall into reclaim and enter zswap (causing a deadlock). A fix was also attempted using SRCU for synchronization, but Johannes pointed out that synchronize_srcu() cannot be used in CPU hotplug notifiers [2]. Alternative fixes that were considered/attempted and could have worked: - Refcounting the per-CPU acomp_ctx. This involves complexity in handling the race between the refcount dropping to zero in zswap_[de]compress() and the refcount being re-initialized when the CPU is onlined. - Disabling migration before getting the per-CPU acomp_ctx [3], but that's discouraged and is a much bigger hammer than needed, and could result in subtle performance issues. [1]https://lkml.kernel.org/[email protected]/ [2]https://lkml.kernel.org/[email protected]/ [3]https://lkml.kernel.org/[email protected]/ [[email protected]: remove comment] Link: https://lkml.kernel.org/r/CAJD7tkaxS1wjn+swugt8QCvQ-rVF5RZnjxwPGX17k8x9zSManA@mail.gmail.com Link: https://lkml.kernel.org/r/[email protected] Fixes: 1ec3b5f ("mm/zswap: move to use crypto_acomp API for hardware acceleration") Signed-off-by: Yosry Ahmed <[email protected]> Reported-by: Johannes Weiner <[email protected]> Closes: https://lore.kernel.org/lkml/[email protected]/ Reported-by: Sam Sun <[email protected]> Closes: https://lore.kernel.org/lkml/CAEkJfYMtSdM5HceNsXUDf5haghD5+o2e7Qv4OcuruL4tPg6OaQ@mail.gmail.com/ Cc: Barry Song <[email protected]> Cc: Chengming Zhou <[email protected]> Cc: Kanchana P Sridhar <[email protected]> Cc: Nhat Pham <[email protected]> Cc: Vitaly Wool <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Rafael Aquini <[email protected]>
1 parent 302e852 commit c9b94ff

File tree

1 file changed

+45
-14
lines changed

1 file changed

+45
-14
lines changed

mm/zswap.c

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -714,11 +714,12 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
714714
struct acomp_req *req;
715715
int ret;
716716

717-
mutex_init(&acomp_ctx->mutex);
718-
717+
mutex_lock(&acomp_ctx->mutex);
719718
acomp_ctx->buffer = kmalloc_node(PAGE_SIZE * 2, GFP_KERNEL, cpu_to_node(cpu));
720-
if (!acomp_ctx->buffer)
721-
return -ENOMEM;
719+
if (!acomp_ctx->buffer) {
720+
ret = -ENOMEM;
721+
goto buffer_fail;
722+
}
722723

723724
acomp = crypto_alloc_acomp_node(pool->tfm_name, 0, 0, cpu_to_node(cpu));
724725
if (IS_ERR(acomp)) {
@@ -747,12 +748,15 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
747748
acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
748749
crypto_req_done, &acomp_ctx->wait);
749750

751+
mutex_unlock(&acomp_ctx->mutex);
750752
return 0;
751753

752754
req_fail:
753755
crypto_free_acomp(acomp_ctx->acomp);
754756
acomp_fail:
755757
kfree(acomp_ctx->buffer);
758+
buffer_fail:
759+
mutex_unlock(&acomp_ctx->mutex);
756760
return ret;
757761
}
758762

@@ -761,17 +765,45 @@ static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
761765
struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
762766
struct crypto_acomp_ctx *acomp_ctx = per_cpu_ptr(pool->acomp_ctx, cpu);
763767

768+
mutex_lock(&acomp_ctx->mutex);
764769
if (!IS_ERR_OR_NULL(acomp_ctx)) {
765770
if (!IS_ERR_OR_NULL(acomp_ctx->req))
766771
acomp_request_free(acomp_ctx->req);
772+
acomp_ctx->req = NULL;
767773
if (!IS_ERR_OR_NULL(acomp_ctx->acomp))
768774
crypto_free_acomp(acomp_ctx->acomp);
769775
kfree(acomp_ctx->buffer);
770776
}
777+
mutex_unlock(&acomp_ctx->mutex);
771778

772779
return 0;
773780
}
774781

782+
static struct crypto_acomp_ctx *acomp_ctx_get_cpu_lock(struct zswap_pool *pool)
783+
{
784+
struct crypto_acomp_ctx *acomp_ctx;
785+
786+
for (;;) {
787+
acomp_ctx = raw_cpu_ptr(pool->acomp_ctx);
788+
mutex_lock(&acomp_ctx->mutex);
789+
if (likely(acomp_ctx->req))
790+
return acomp_ctx;
791+
/*
792+
* It is possible that we were migrated to a different CPU after
793+
* getting the per-CPU ctx but before the mutex was acquired. If
794+
* the old CPU got offlined, zswap_cpu_comp_dead() could have
795+
* already freed ctx->req (among other things) and set it to
796+
* NULL. Just try again on the new CPU that we ended up on.
797+
*/
798+
mutex_unlock(&acomp_ctx->mutex);
799+
}
800+
}
801+
802+
static void acomp_ctx_put_unlock(struct crypto_acomp_ctx *acomp_ctx)
803+
{
804+
mutex_unlock(&acomp_ctx->mutex);
805+
}
806+
775807
/*********************************
776808
* pool functions
777809
**********************************/
@@ -1029,7 +1061,7 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
10291061
struct zswap_pool *pool;
10301062
char name[38]; /* 'zswap' + 32 char (max) num + \0 */
10311063
gfp_t gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM;
1032-
int ret;
1064+
int ret, cpu;
10331065

10341066
if (!zswap_has_pool) {
10351067
/* if either are unset, pool initialization failed, and we
@@ -1067,6 +1099,9 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
10671099
goto error;
10681100
}
10691101

1102+
for_each_possible_cpu(cpu)
1103+
mutex_init(&per_cpu_ptr(pool->acomp_ctx, cpu)->mutex);
1104+
10701105
ret = cpuhp_state_add_instance(CPUHP_MM_ZSWP_POOL_PREPARE,
10711106
&pool->node);
10721107
if (ret)
@@ -1376,9 +1411,7 @@ static void __zswap_load(struct zswap_entry *entry, struct page *page)
13761411
struct crypto_acomp_ctx *acomp_ctx;
13771412
u8 *src;
13781413

1379-
acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
1380-
mutex_lock(&acomp_ctx->mutex);
1381-
1414+
acomp_ctx = acomp_ctx_get_cpu_lock(entry->pool);
13821415
src = zpool_map_handle(zpool, entry->handle, ZPOOL_MM_RO);
13831416
if (!zpool_can_sleep_mapped(zpool)) {
13841417
memcpy(acomp_ctx->buffer, src, entry->length);
@@ -1392,10 +1425,10 @@ static void __zswap_load(struct zswap_entry *entry, struct page *page)
13921425
acomp_request_set_params(acomp_ctx->req, &input, &output, entry->length, PAGE_SIZE);
13931426
BUG_ON(crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req), &acomp_ctx->wait));
13941427
BUG_ON(acomp_ctx->req->dlen != PAGE_SIZE);
1395-
mutex_unlock(&acomp_ctx->mutex);
13961428

13971429
if (zpool_can_sleep_mapped(zpool))
13981430
zpool_unmap_handle(zpool, entry->handle);
1431+
acomp_ctx_put_unlock(acomp_ctx);
13991432
}
14001433

14011434
/*********************************
@@ -1612,9 +1645,7 @@ bool zswap_store(struct folio *folio)
16121645
}
16131646

16141647
/* compress */
1615-
acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
1616-
1617-
mutex_lock(&acomp_ctx->mutex);
1648+
acomp_ctx = acomp_ctx_get_cpu_lock(entry->pool);
16181649

16191650
dst = acomp_ctx->buffer;
16201651
sg_init_table(&input, 1);
@@ -1662,7 +1693,7 @@ bool zswap_store(struct folio *folio)
16621693
buf = zpool_map_handle(zpool, handle, ZPOOL_MM_WO);
16631694
memcpy(buf, dst, dlen);
16641695
zpool_unmap_handle(zpool, handle);
1665-
mutex_unlock(&acomp_ctx->mutex);
1696+
acomp_ctx_put_unlock(acomp_ctx);
16661697

16671698
/* populate entry */
16681699
entry->swpentry = swp_entry(type, offset);
@@ -1705,7 +1736,7 @@ bool zswap_store(struct folio *folio)
17051736
return true;
17061737

17071738
put_dstmem:
1708-
mutex_unlock(&acomp_ctx->mutex);
1739+
acomp_ctx_put_unlock(acomp_ctx);
17091740
put_pool:
17101741
zswap_pool_put(entry->pool);
17111742
freepage:

0 commit comments

Comments
 (0)