On Mon, Jan 13, 2025 at 6:04 AM Sasha Levin <sashal@xxxxxxxxxx> 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;a=summary > > 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@xxxxxxxxxxxxxxx> 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-iPqbPdyXCpS=2AGQ@xxxxxxxxxxxxxx/ [2]https://lore.kernel.org/lkml/20250113000543.862792e948c2646032a477e0@xxxxxxxxxxxxxxxxxxxx/ > > > > commit c56e79d453ef5b5fc6ded252bc6ba461f12946ba > Author: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > 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@xxxxxxxxxx > Fixes: 1ec3b5fe6eec ("mm/zswap: move to use crypto_acomp API for hardware acceleration") > Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > Reported-by: Johannes Weiner <hannes@xxxxxxxxxxx> > Closes: https://lore.kernel.org/lkml/20241113213007.GB1564047@xxxxxxxxxxx/ > Reported-by: Sam Sun <samsun1006219@xxxxxxxxx> > Closes: https://lore.kernel.org/lkml/CAEkJfYMtSdM5HceNsXUDf5haghD5+o2e7Qv4OcuruL4tPg6OaQ@xxxxxxxxxxxxxx/ > Reviewed-by: Chengming Zhou <chengming.zhou@xxxxxxxxx> > Acked-by: Barry Song <baohua@xxxxxxxxxx> > Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx> > Cc: Vitaly Wool <vitalywool@xxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > > 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(); > } > > /*********************************