From: Haoqin Huang zstd_setup_params() creates global cdict and ddict stored in params->drv_data, shared across all per-CPU contexts. The per-CPU zstd_create() error path called zstd_release_params(), which freed those globally-shared objects. While the drv_data=NULL guard in zstd_release_params() prevents a double-free on the init failure path, this is still a layering violation: a per-CPU callback should only clean up its own context, not release resources owned by the compression lifecycle (zcomp_init / zcomp_destroy). Remove zstd_release_params() from the per-CPU error path and call only zstd_destroy(), which properly cleans up the per-CPU context without touching the global params->drv_data. Signed-off-by: Haoqin Huang Signed-off-by: Rongwei Wang --- drivers/block/zram/backend_zstd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c index d00b548056dc..2584f47c9b3c 100644 --- a/drivers/block/zram/backend_zstd.c +++ b/drivers/block/zram/backend_zstd.c @@ -161,7 +161,6 @@ static int zstd_create(struct zcomp_params *params, struct zcomp_ctx *ctx) return 0; error: - zstd_release_params(params); zstd_destroy(ctx); return -EINVAL; } -- 2.43.7 From: Haoqin Huang kernel_read_file_from_path() already rejects empty files (i_size <= 0) and returns -EINVAL, but the current implementation only checks for sz < 0 without logging any information. Use sz <= 0 to cover the zero-size case and print an error message if dictionary loading fails. Signed-off-by: Haoqin Huang Signed-off-by: Rongwei Wang --- drivers/block/zram/zram_drv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index ace65c586072..0223fd83bbba 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1709,8 +1709,11 @@ static int comp_params_store(struct zram *zram, u32 prio, s32 level, INT_MAX, NULL, READING_POLICY); - if (sz < 0) + if (sz <= 0) { + pr_err("zram: failed to load dictionary %s (err=%zd)\n", + dict_path, sz); return -EINVAL; + } } zram->params[prio].dict_sz = sz; -- 2.43.7 From: Haoqin Huang zstd_setup_params() currently accepts any level and silently clamps it via zstd_get_params(). Add explicit bounds checking using zstd_max_clevel() to reject out-of-range levels early with an error message. Since zstd_max_clevel() is a runtime function, the check is done here rather than in the generic validation path. Signed-off-by: Haoqin Huang Signed-off-by: Rongwei Wang --- drivers/block/zram/backend_zstd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c index 2584f47c9b3c..6febb366f76e 100644 --- a/drivers/block/zram/backend_zstd.c +++ b/drivers/block/zram/backend_zstd.c @@ -60,6 +60,11 @@ static int zstd_setup_params(struct zcomp_params *params) params->drv_data = zp; if (params->level == ZCOMP_PARAM_NOT_SET) params->level = zstd_default_clevel(); + else if (params->level < -(int)ZSTD_TARGETLENGTH_MAX || + params->level > zstd_max_clevel()) { + pr_err("zstd: invalid compression level %d\n", params->level); + goto error; + } zp->cprm = zstd_get_params(params->level, PAGE_SIZE); -- 2.43.7 From: Haoqin Huang Dict and level parameters are silently accepted even for backends that do not support them, e.g. "algo=lzo dict=/data/dict" succeeds but has no effect. Add per-backend caps and zcomp_validate_params() to reject invalid parameters with a specific error message before storing. For zstd, set level_max to -1 as a sentinel since its maximum level is determined at runtime by zstd_max_clevel(). Signed-off-by: Haoqin Huang Signed-off-by: Rongwei Wang --- drivers/block/zram/backend_842.c | 1 + drivers/block/zram/backend_deflate.c | 3 +++ drivers/block/zram/backend_lz4.c | 3 +++ drivers/block/zram/backend_lz4hc.c | 3 +++ drivers/block/zram/backend_lzo.c | 1 + drivers/block/zram/backend_lzorle.c | 1 + drivers/block/zram/backend_zstd.c | 3 +++ drivers/block/zram/zcomp.c | 27 +++++++++++++++++++++++++++ drivers/block/zram/zcomp.h | 7 +++++++ drivers/block/zram/zram_drv.c | 7 +++++++ 10 files changed, 56 insertions(+) diff --git a/drivers/block/zram/backend_842.c b/drivers/block/zram/backend_842.c index 10d9d5c60f53..d796ebda1fa0 100644 --- a/drivers/block/zram/backend_842.c +++ b/drivers/block/zram/backend_842.c @@ -57,5 +57,6 @@ const struct zcomp_ops backend_842 = { .destroy_ctx = destroy_842, .setup_params = setup_params_842, .release_params = release_params_842, + .caps = 0, .name = "842", }; diff --git a/drivers/block/zram/backend_deflate.c b/drivers/block/zram/backend_deflate.c index f92a52a720d1..cedc3daad33a 100644 --- a/drivers/block/zram/backend_deflate.c +++ b/drivers/block/zram/backend_deflate.c @@ -144,5 +144,8 @@ const struct zcomp_ops backend_deflate = { .destroy_ctx = deflate_destroy, .setup_params = deflate_setup_params, .release_params = deflate_release_params, + .caps = ZCOMP_CAP_LEVEL, + .level_min = Z_DEFAULT_COMPRESSION, + .level_max = Z_BEST_COMPRESSION, .name = "deflate", }; diff --git a/drivers/block/zram/backend_lz4.c b/drivers/block/zram/backend_lz4.c index c449d511ba86..bd1e5ca4d134 100644 --- a/drivers/block/zram/backend_lz4.c +++ b/drivers/block/zram/backend_lz4.c @@ -146,5 +146,8 @@ const struct zcomp_ops backend_lz4 = { .destroy_ctx = lz4_destroy, .setup_params = lz4_setup_params, .release_params = lz4_release_params, + .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL, + .level_min = LZ4_ACCELERATION_DEFAULT, + .level_max = 65535, .name = "lz4", }; diff --git a/drivers/block/zram/backend_lz4hc.c b/drivers/block/zram/backend_lz4hc.c index f6a336acfe20..0e0d7c68a7d4 100644 --- a/drivers/block/zram/backend_lz4hc.c +++ b/drivers/block/zram/backend_lz4hc.c @@ -124,5 +124,8 @@ const struct zcomp_ops backend_lz4hc = { .destroy_ctx = lz4hc_destroy, .setup_params = lz4hc_setup_params, .release_params = lz4hc_release_params, + .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL, + .level_min = LZ4HC_MIN_CLEVEL, + .level_max = LZ4HC_MAX_CLEVEL, .name = "lz4hc", }; diff --git a/drivers/block/zram/backend_lzo.c b/drivers/block/zram/backend_lzo.c index 4c906beaae6b..965f007e2ca8 100644 --- a/drivers/block/zram/backend_lzo.c +++ b/drivers/block/zram/backend_lzo.c @@ -55,5 +55,6 @@ const struct zcomp_ops backend_lzo = { .destroy_ctx = lzo_destroy, .setup_params = lzo_setup_params, .release_params = lzo_release_params, + .caps = 0, .name = "lzo", }; diff --git a/drivers/block/zram/backend_lzorle.c b/drivers/block/zram/backend_lzorle.c index 10640c96cbfc..757b4598be03 100644 --- a/drivers/block/zram/backend_lzorle.c +++ b/drivers/block/zram/backend_lzorle.c @@ -55,5 +55,6 @@ const struct zcomp_ops backend_lzorle = { .destroy_ctx = lzorle_destroy, .setup_params = lzorle_setup_params, .release_params = lzorle_release_params, + .caps = 0, .name = "lzo-rle", }; diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c index 6febb366f76e..801ee8ee4ba6 100644 --- a/drivers/block/zram/backend_zstd.c +++ b/drivers/block/zram/backend_zstd.c @@ -217,5 +217,8 @@ const struct zcomp_ops backend_zstd = { .destroy_ctx = zstd_destroy, .setup_params = zstd_setup_params, .release_params = zstd_release_params, + .caps = ZCOMP_CAP_DICT | ZCOMP_CAP_LEVEL, + .level_min = (int)-ZSTD_TARGETLENGTH_MAX, + .level_max = -1, /* validated by zstd_setup_params() */ .name = "zstd", }; diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 974c4691887e..149eff4590e1 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -94,6 +94,33 @@ const char *zcomp_lookup_backend_name(const char *comp) return NULL; } +int zcomp_validate_params(const char *comp, s32 level, const char *dict_path) +{ + const struct zcomp_ops *backend = lookup_backend_ops(comp); + + if (!backend) + return -EINVAL; + + if (dict_path && !(backend->caps & ZCOMP_CAP_DICT)) { + pr_err("zram: %s does not support dictionary\n", comp); + return -EOPNOTSUPP; + } + + if (level != ZCOMP_PARAM_NOT_SET) { + if (!(backend->caps & ZCOMP_CAP_LEVEL)) { + pr_err("zram: %s does not support level\n", comp); + return -EOPNOTSUPP; + } + /* level_max == -1 means validate in .setup_params() */ + if (backend->level_max >= 0 && + (level < backend->level_min || level > backend->level_max)) { + pr_err("zram: invalid level %d for %s\n", level, comp); + return -EINVAL; + } + } + return 0; +} + /* show available compressors */ ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at) { diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index 81a0f3f6ff48..366250050d4a 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -7,6 +7,9 @@ #define ZCOMP_PARAM_NOT_SET INT_MIN +#define ZCOMP_CAP_DICT BIT(0) /* dictionary support */ +#define ZCOMP_CAP_LEVEL BIT(1) /* adjustable compression level */ + struct deflate_params { s32 winbits; }; @@ -66,6 +69,9 @@ struct zcomp_ops { int (*setup_params)(struct zcomp_params *params); void (*release_params)(struct zcomp_params *params); + unsigned int caps; + s32 level_min; + s32 level_max; const char *name; }; @@ -81,6 +87,7 @@ int zcomp_cpu_up_prepare(unsigned int cpu, struct hlist_node *node); int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node); ssize_t zcomp_available_show(const char *comp, char *buf, ssize_t at); const char *zcomp_lookup_backend_name(const char *comp); +int zcomp_validate_params(const char *comp, s32 level, const char *dict_path); struct zcomp *zcomp_create(const char *alg, struct zcomp_params *params); void zcomp_destroy(struct zcomp *comp); diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 0223fd83bbba..0c804bb4a701 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1797,6 +1797,13 @@ static ssize_t algorithm_params_store(struct device *dev, return -EINVAL; } + if (zram->comp_algs[prio]) { + ret = zcomp_validate_params(zram->comp_algs[prio], level, + dict_path); + if (ret) + return ret; + } + ret = comp_params_store(zram, prio, level, dict_path, &deflate_params); return ret ? ret : len; } -- 2.43.7 From: Haoqin Huang Parameters validated against one algorithm may be invalid for another (e.g. lz4 accepts level=65535 but zstd does not). Although algorithm changes are blocked after disksize is set, they are allowed before device initialization. Reset per-priority params on algorithm change so that stale parameters do not silently carry over. Signed-off-by: Haoqin Huang Signed-off-by: Rongwei Wang Reviewed-by: Sergey Senozhatsky --- drivers/block/zram/zram_drv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 0c804bb4a701..b4585b3bc576 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1661,6 +1661,8 @@ static void comp_algorithm_set(struct zram *zram, u32 prio, const char *alg) zram->comp_algs[prio] = alg; } +static void comp_params_reset(struct zram *zram, u32 prio); + static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf) { const char *alg; @@ -1681,6 +1683,7 @@ static int __comp_algorithm_store(struct zram *zram, u32 prio, const char *buf) } comp_algorithm_set(zram, prio, alg); + comp_params_reset(zram, prio); return 0; } -- 2.43.7