[PATCH] zram: do not permit params change after init

[PATCH] zram: do not permit params change after init

[Sergey Senozhatsky]

First, algorithm_params_store(), like any sysfs handler,
should grab device init lock.

Second, like any write() sysfs handler, it should grab
device init lock in exclusive mode.

Third, it should not permit change of algos' parameters
after device init, as this doesn't make sense - we cannot
compress with one C/D dict and then just change C/D dict
to a different one, for example.

Fixes: 4eac932103a5d ("zram: introduce algorithm_params device attribute")
Cc: gao xu <gaoxu2@honor.com>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/block/zram/zram_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index c41e1257243f..7551c5e664a9 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1769,6 +1769,10 @@ static ssize_t algorithm_params_store(struct device *dev,
 	if (prio < ZRAM_PRIMARY_COMP || prio >= ZRAM_MAX_COMPS)
 		return -EINVAL;
 
+	guard(rwsem_write)(&zram->dev_lock);
+	if (init_done(zram))
+		return -EBUSY;
+
 	ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
 	return ret ? ret : len;
 }
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH] zram: do not permit params change after init

[Brian Geffon]

On Tue, Mar 10, 2026 at 2:04 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> First, algorithm_params_store(), like any sysfs handler,
> should grab device init lock.
>
> Second, like any write() sysfs handler, it should grab
> device init lock in exclusive mode.

comp_algorithm_store() and recomp_algorithm_store()?

>
> Third, it should not permit change of algos' parameters
> after device init, as this doesn't make sense - we cannot
> compress with one C/D dict and then just change C/D dict
> to a different one, for example.
>
> Fixes: 4eac932103a5d ("zram: introduce algorithm_params device attribute")
> Cc: gao xu <gaoxu2@honor.com>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Acked-by: Brian Geffon <bgeffon@google.com>

> ---
>  drivers/block/zram/zram_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index c41e1257243f..7551c5e664a9 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1769,6 +1769,10 @@ static ssize_t algorithm_params_store(struct device *dev,
>         if (prio < ZRAM_PRIMARY_COMP || prio >= ZRAM_MAX_COMPS)
>                 return -EINVAL;
>
> +       guard(rwsem_write)(&zram->dev_lock);
> +       if (init_done(zram))
> +               return -EBUSY;
> +
>         ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
>         return ret ? ret : len;
>  }
> --
> 2.53.0.473.g4a7958ca14-goog
>

Re: [PATCH] zram: do not permit params change after init

[Brian Geffon]

On Tue, Mar 10, 2026 at 10:01 AM Brian Geffon <bgeffon@google.com> wrote:
>
> On Tue, Mar 10, 2026 at 2:04 AM Sergey Senozhatsky
> <senozhatsky@chromium.org> wrote:
> >
> > First, algorithm_params_store(), like any sysfs handler,
> > should grab device init lock.
> >
> > Second, like any write() sysfs handler, it should grab
> > device init lock in exclusive mode.
>
> comp_algorithm_store() and recomp_algorithm_store()?

disregard, that happens in __comp_algorithm_store().

>
> >
> > Third, it should not permit change of algos' parameters
> > after device init, as this doesn't make sense - we cannot
> > compress with one C/D dict and then just change C/D dict
> > to a different one, for example.
> >
> > Fixes: 4eac932103a5d ("zram: introduce algorithm_params device attribute")
> > Cc: gao xu <gaoxu2@honor.com>
> > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>
> Acked-by: Brian Geffon <bgeffon@google.com>
>
> > ---
> >  drivers/block/zram/zram_drv.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index c41e1257243f..7551c5e664a9 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -1769,6 +1769,10 @@ static ssize_t algorithm_params_store(struct device *dev,
> >         if (prio < ZRAM_PRIMARY_COMP || prio >= ZRAM_MAX_COMPS)
> >                 return -EINVAL;
> >
> > +       guard(rwsem_write)(&zram->dev_lock);
> > +       if (init_done(zram))
> > +               return -EBUSY;
> > +
> >         ret = comp_params_store(zram, prio, level, dict_path, &deflate_params);
> >         return ret ? ret : len;
> >  }
> > --
> > 2.53.0.473.g4a7958ca14-goog
> >

Re: [PATCH] zram: do not permit params change after init

[Sergey Senozhatsky]

On (26/03/10 15:01), Sergey Senozhatsky wrote:
> First, algorithm_params_store(), like any sysfs handler,
> should grab device init lock.
> 
> Second, like any write() sysfs handler, it should grab
> device init lock in exclusive mode.
> 
> Third, it should not permit change of algos' parameters
> after device init, as this doesn't make sense - we cannot
> compress with one C/D dict and then just change C/D dict
> to a different one, for example.
> 
> Fixes: 4eac932103a5d ("zram: introduce algorithm_params device attribute")
> Cc: gao xu <gaoxu2@honor.com>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Andrew, I will include this patch into v2 of the zram fixups,
can you please drop it for now?

We need to protect algo lookup path as well, which accesses
->comp_algs.  I'll fix that in v2.