[PATCH] mm: zswap: set zswap_ever_enabled when enabling zswap at runtime

[PATCH] mm: zswap: set zswap_ever_enabled when enabling zswap at runtime

[Wenwu Hou]

zswap can end up enabled without zswap_ever_enabled being set, which
leads to data corruption. With CONFIG_ZSWAP_DEFAULT_ON=n:

1. zswap_setup() fails to create the initial pool. Setup still counts
   as successful, but zswap_has_pool stays false.
2. The user writes a compressor name to
   /sys/module/zswap/parameters/compressor.
   zswap_compressor_param_set() creates a pool and sets
   zswap_has_pool = true, but never enables the zswap_ever_enabled
   static key.
3. The user runs "echo Y > /sys/module/zswap/parameters/enabled".
4. zswap_store() now stores pages in zswap, and swap_writeout() skips
   writing them to the swap device.
5. On swap-in, zswap_load() first checks zswap_never_enabled(). It
   returns true, so zswap_load() reports that the page is not in zswap.
   The page is then read from the swap device, which holds stale data.

Fix this by enabling zswap_ever_enabled in zswap_enabled_param_set()
before zswap_enabled becomes true.

Fixes: 2d4d2b1cfb85 ("mm: zswap: add zswap_never_enabled()")
Cc: stable@vger.kernel.org
Signed-off-by: Wenwu Hou <hwenwur@gmail.com>
---
 mm/zswap.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 37f34e406c8e3..d2b4c248baa67 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -629,12 +629,29 @@ static int zswap_enabled_param_set(const char *val,
 		if (zswap_setup())
 			break;
 		fallthrough;
-	case ZSWAP_INIT_SUCCEED:
-		if (!zswap_has_pool)
+	case ZSWAP_INIT_SUCCEED: {
+		bool enable;
+
+		if (!zswap_has_pool) {
 			pr_err("can't enable, no pool configured\n");
-		else
-			ret = param_set_bool(val, kp);
+			break;
+		}
+
+		/*
+		 * The initial pool may have been created later by the
+		 * compressor param rather than by zswap_setup(), in which
+		 * case zswap_ever_enabled is still off. Flip it before
+		 * zswap_enabled becomes visible to zswap_store(), otherwise
+		 * zswap_load() would skip stored entries and read stale data
+		 * from the backing swap device. param_set_bool() treats a
+		 * NULL value as true.
+		 */
+		if (!val || (!kstrtobool(val, &enable) && enable))
+			static_branch_enable(&zswap_ever_enabled);
+
+		ret = param_set_bool(val, kp);
 		break;
+	}
 	case ZSWAP_INIT_FAILED:
 		pr_err("can't enable, initialization failed\n");
 	}
-- 
2.43.0

Re: [PATCH] mm: zswap: set zswap_ever_enabled when enabling zswap at runtime

[Yosry Ahmed]

On Tue, Oct 20, 2026 at 5:30 AM Wenwu Hou <hwenwur@gmail.com> wrote:
>
> zswap can end up enabled without zswap_ever_enabled being set, which
> leads to data corruption. With CONFIG_ZSWAP_DEFAULT_ON=n:
>
> 1. zswap_setup() fails to create the initial pool. Setup still counts
>    as successful, but zswap_has_pool stays false.
> 2. The user writes a compressor name to
>    /sys/module/zswap/parameters/compressor.
>    zswap_compressor_param_set() creates a pool and sets
>    zswap_has_pool = true, but never enables the zswap_ever_enabled
>    static key.
> 3. The user runs "echo Y > /sys/module/zswap/parameters/enabled".
> 4. zswap_store() now stores pages in zswap, and swap_writeout() skips
>    writing them to the swap device.
> 5. On swap-in, zswap_load() first checks zswap_never_enabled(). It
>    returns true, so zswap_load() reports that the page is not in zswap.
>    The page is then read from the swap device, which holds stale data.
>
> Fix this by enabling zswap_ever_enabled in zswap_enabled_param_set()
> before zswap_enabled becomes true.

Wasn't this fixed by
https://lore.kernel.org/linux-mm/20260906133601.3563324-1-xialonglong2025@163.com/?

>
> Fixes: 2d4d2b1cfb85 ("mm: zswap: add zswap_never_enabled()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wenwu Hou <hwenwur@gmail.com>
> ---
>  mm/zswap.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 37f34e406c8e3..d2b4c248baa67 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -629,12 +629,29 @@ static int zswap_enabled_param_set(const char *val,
>                 if (zswap_setup())
>                         break;
>                 fallthrough;
> -       case ZSWAP_INIT_SUCCEED:
> -               if (!zswap_has_pool)
> +       case ZSWAP_INIT_SUCCEED: {
> +               bool enable;
> +
> +               if (!zswap_has_pool) {
>                         pr_err("can't enable, no pool configured\n");
> -               else
> -                       ret = param_set_bool(val, kp);
> +                       break;
> +               }
> +
> +               /*
> +                * The initial pool may have been created later by the
> +                * compressor param rather than by zswap_setup(), in which
> +                * case zswap_ever_enabled is still off. Flip it before
> +                * zswap_enabled becomes visible to zswap_store(), otherwise
> +                * zswap_load() would skip stored entries and read stale data
> +                * from the backing swap device. param_set_bool() treats a
> +                * NULL value as true.
> +                */
> +               if (!val || (!kstrtobool(val, &enable) && enable))
> +                       static_branch_enable(&zswap_ever_enabled);
> +
> +               ret = param_set_bool(val, kp);
>                 break;
> +       }
>         case ZSWAP_INIT_FAILED:
>                 pr_err("can't enable, initialization failed\n");
>         }
> --
> 2.43.0
>

Re: [PATCH] mm: zswap: set zswap_ever_enabled when enabling zswap at runtime

[Wenwu Hou]

On Tue, Oct 20, 2026 at 08:57:15AM -0700, Yosry Ahmed wrote:
> On Tue, Oct 20, 2026 at 5:30 AM Wenwu Hou <hwenwur@gmail.com> wrote:
> >
> > zswap can end up enabled without zswap_ever_enabled being set, which
> > leads to data corruption. With CONFIG_ZSWAP_DEFAULT_ON=n:
> >
> > 1. zswap_setup() fails to create the initial pool. Setup still counts
> >    as successful, but zswap_has_pool stays false.
> > 2. The user writes a compressor name to
> >    /sys/module/zswap/parameters/compressor.
> >    zswap_compressor_param_set() creates a pool and sets
> >    zswap_has_pool = true, but never enables the zswap_ever_enabled
> >    static key.
> > 3. The user runs "echo Y > /sys/module/zswap/parameters/enabled".
> > 4. zswap_store() now stores pages in zswap, and swap_writeout() skips
> >    writing them to the swap device.
> > 5. On swap-in, zswap_load() first checks zswap_never_enabled(). It
> >    returns true, so zswap_load() reports that the page is not in zswap.
> >    The page is then read from the swap device, which holds stale data.
> >
> > Fix this by enabling zswap_ever_enabled in zswap_enabled_param_set()
> > before zswap_enabled becomes true.
> 
> Wasn't this fixed by
> https://lore.kernel.org/linux-mm/20260906133601.3563324-1-xialonglong2025@163.com/?

Yes, please drop this one, sorry for the noise.

Re: [PATCH] mm: zswap: set zswap_ever_enabled when enabling zswap at runtime

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: zswap: redundant zswap_ever_enabled enablement in zswap_enabled_param_set()
--

commit 4d07b90085374936c4c6222ca58eb86f3db68b14
Author: Wenwu Hou <hwenwur@gmail.com>

mm: zswap: set zswap_ever_enabled when enabling zswap at runtime

This commit enables the zswap_ever_enabled static branch in
zswap_enabled_param_set() when zswap is enabled at runtime after the
initial pool was created via the compressor parameter.

>     mm: zswap: set zswap_ever_enabled when enabling zswap at runtime
>     
>     zswap can end up enabled without zswap_ever_enabled being set, which
>     leads to data corruption. With CONFIG_ZSWAP_DEFAULT_ON=n:
>     
>     1. zswap_setup() fails to create the initial pool. Setup still counts
>        as successful, but zswap_has_pool stays false.
>     2. The user writes a compressor name to
>        /sys/module/zswap/parameters/compressor.
>        zswap_compressor_param_set() creates a pool and sets
>        zswap_has_pool = true, but never enables the zswap_ever_enabled
>        static key.
>     3. The user runs "echo Y > /sys/module/zswap/parameters/enabled".
>     4. zswap_store() now stores pages in zswap, and swap_writeout() skips
>        writing them to the swap device.
>     5. On swap-in, zswap_load() first checks zswap_never_enabled(). It
>        returns true, so zswap_load() reports that the page is not in zswap.
>        The page is then read from the swap device, which holds stale data.
>     
>     Fix this by enabling zswap_ever_enabled in zswap_enabled_param_set()
>     before zswap_enabled becomes true.
>     
>     Fixes: 2d4d2b1cfb85 ("mm: zswap: add zswap_never_enabled()")
>     Cc: stable@vger.kernel.org
>     Signed-off-by: Wenwu Hou <hwenwur@gmail.com>
> 
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 572075bb1c1c1..bd97652b682de 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -661,12 +661,29 @@ static int zswap_enabled_param_set(const char *val,
>  		if (zswap_setup())
>  			break;
>  		fallthrough;
> -	case ZSWAP_INIT_SUCCEED:
> -		if (!zswap_has_pool)
> +	case ZSWAP_INIT_SUCCEED: {
> +		bool enable;
> +
> +		if (!zswap_has_pool) {
>  			pr_err("can't enable, no pool configured\n");
> -		else
> -			ret = param_set_bool(val, kp);
> +			break;
> +		}
> +
> +		/*
> +		 * The initial pool may have been created later by the
> +		 * compressor param rather than by zswap_setup(), in which
> +		 * case zswap_ever_enabled is still off. Flip it before
> +		 * zswap_enabled becomes visible to zswap_store(), otherwise
> +		 * zswap_load() would skip stored entries and read stale data
> +		 * from the backing swap device. param_set_bool() treats a
> +		 * NULL value as true.
> +		 */
> +		if (!val || (!kstrtobool(val, &enable) && enable))
> +			static_branch_enable(&zswap_ever_enabled);

[Severity: Medium]
Can zswap_ever_enabled actually still be off when reaching this check in
zswap_enabled_param_set()?

Commit 91b4edb993e1 ("mm/zswap: enable zswap_ever_enabled in
zswap_pool_create()") already moved static_branch_enable(&zswap_ever_enabled)
into zswap_pool_create():

mm/zswap.c:zswap_pool_create() {
	...
	zswap_pool_debug("created", pool);

	static_branch_enable(&zswap_ever_enabled);

	return pool;
	...
}

When a user writes to /sys/module/zswap/parameters/compressor,
zswap_compressor_param_set() creates the pool via zswap_pool_create() before
setting zswap_has_pool = true:

mm/zswap.c:zswap_compressor_param_set() {
	...
	pool = zswap_pool_find_get(s);
	if (!pool) {
		pool = zswap_pool_create(s);
	}
	...
	if (!ret) {
		put_pool = rcu_access_pointer(zswap_current_pool);
		rcu_assign_pointer(zswap_current_pool, pool);
		zswap_has_pool = true;
	}
	...
}

Since zswap_has_pool is only set to true after zswap_pool_create() succeeds
(either in zswap_setup()->__zswap_pool_create_fallback()->zswap_pool_create()
or in zswap_compressor_param_set()->zswap_pool_create()), and
zswap_enabled_param_set() checks !zswap_has_pool and breaks out above, isn't
zswap_ever_enabled guaranteed to already be enabled here?

> +
> +		ret = param_set_bool(val, kp);
>  		break;
> +	}
>  	case ZSWAP_INIT_FAILED:
>  		pr_err("can't enable, initialization failed\n");
>  	}

([ ♦♦ ])

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020122942.3477541-1-hwenwur@gmail.com?part=1