* [bug report] mm/zswap: use only one pool in zswap
@ 2024-06-20 8:52 Dan Carpenter
2024-06-20 9:12 ` Chengming Zhou
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2024-06-20 8:52 UTC (permalink / raw)
To: Chengming Zhou; +Cc: linux-mm
Hello Chengming Zhou,
Commit 6193f190fe0a ("mm/zswap: use only one pool in zswap") from Jun
17, 2024 (linux-next), leads to the following Smatch static checker
warning:
mm/zswap.c:306 zswap_pool_create()
error: potential null dereference 'pool->zpool'. (zpool_create_pool returns null)
mm/zswap.c
244 static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
245 {
246 struct zswap_pool *pool;
247 char name[38]; /* 'zswap' + 32 char (max) num + \0 */
248 gfp_t gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM;
249 int ret;
250
251 if (!zswap_has_pool) {
252 /* if either are unset, pool initialization failed, and we
253 * need both params to be set correctly before trying to
254 * create a pool.
255 */
256 if (!strcmp(type, ZSWAP_PARAM_UNSET))
257 return NULL;
258 if (!strcmp(compressor, ZSWAP_PARAM_UNSET))
259 return NULL;
260 }
261
262 pool = kzalloc(sizeof(*pool), GFP_KERNEL);
263 if (!pool)
264 return NULL;
265
266 /* unique name for each pool specifically required by zsmalloc */
267 snprintf(name, 38, "zswap%x", atomic_inc_return(&zswap_pools_count));
268 pool->zpool = zpool_create_pool(type, name, gfp);
269 if (!pool->zpool) {
270 pr_err("%s zpool not available\n", type);
271 goto error;
pool->zpool is NULL
272 }
273 pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
274
275 strscpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
276
277 pool->acomp_ctx = alloc_percpu(*pool->acomp_ctx);
278 if (!pool->acomp_ctx) {
279 pr_err("percpu alloc failed\n");
280 goto error;
281 }
282
283 ret = cpuhp_state_add_instance(CPUHP_MM_ZSWP_POOL_PREPARE,
284 &pool->node);
285 if (ret)
286 goto error;
287
288 /* being the current pool takes 1 ref; this func expects the
289 * caller to always add the new pool as the current pool
290 */
291 ret = percpu_ref_init(&pool->ref, __zswap_pool_empty,
292 PERCPU_REF_ALLOW_REINIT, GFP_KERNEL);
293 if (ret)
294 goto ref_fail;
295 INIT_LIST_HEAD(&pool->list);
296
297 zswap_pool_debug("created", pool);
298
299 return pool;
300
301 ref_fail:
302 cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
303 error:
304 if (pool->acomp_ctx)
305 free_percpu(pool->acomp_ctx);
--> 306 zpool_destroy_pool(pool->zpool);
^^^^^^^^^^^
NULL dereference
307 kfree(pool);
308 return NULL;
309 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] mm/zswap: use only one pool in zswap
2024-06-20 8:52 [bug report] mm/zswap: use only one pool in zswap Dan Carpenter
@ 2024-06-20 9:12 ` Chengming Zhou
0 siblings, 0 replies; 2+ messages in thread
From: Chengming Zhou @ 2024-06-20 9:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mm
On 2024/6/20 16:52, Dan Carpenter wrote:
> Hello Chengming Zhou,
>
> Commit 6193f190fe0a ("mm/zswap: use only one pool in zswap") from Jun
> 17, 2024 (linux-next), leads to the following Smatch static checker
> warning:
Thanks much for your report and explanation!
I will fix in the next version.
>
> mm/zswap.c:306 zswap_pool_create()
> error: potential null dereference 'pool->zpool'. (zpool_create_pool returns null)
>
> mm/zswap.c
> 244 static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
> 245 {
> 246 struct zswap_pool *pool;
> 247 char name[38]; /* 'zswap' + 32 char (max) num + \0 */
> 248 gfp_t gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM;
> 249 int ret;
> 250
> 251 if (!zswap_has_pool) {
> 252 /* if either are unset, pool initialization failed, and we
> 253 * need both params to be set correctly before trying to
> 254 * create a pool.
> 255 */
> 256 if (!strcmp(type, ZSWAP_PARAM_UNSET))
> 257 return NULL;
> 258 if (!strcmp(compressor, ZSWAP_PARAM_UNSET))
> 259 return NULL;
> 260 }
> 261
> 262 pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> 263 if (!pool)
> 264 return NULL;
> 265
> 266 /* unique name for each pool specifically required by zsmalloc */
> 267 snprintf(name, 38, "zswap%x", atomic_inc_return(&zswap_pools_count));
> 268 pool->zpool = zpool_create_pool(type, name, gfp);
> 269 if (!pool->zpool) {
> 270 pr_err("%s zpool not available\n", type);
> 271 goto error;
>
> pool->zpool is NULL
>
> 272 }
> 273 pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
> 274
> 275 strscpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
> 276
> 277 pool->acomp_ctx = alloc_percpu(*pool->acomp_ctx);
> 278 if (!pool->acomp_ctx) {
> 279 pr_err("percpu alloc failed\n");
> 280 goto error;
> 281 }
> 282
> 283 ret = cpuhp_state_add_instance(CPUHP_MM_ZSWP_POOL_PREPARE,
> 284 &pool->node);
> 285 if (ret)
> 286 goto error;
> 287
> 288 /* being the current pool takes 1 ref; this func expects the
> 289 * caller to always add the new pool as the current pool
> 290 */
> 291 ret = percpu_ref_init(&pool->ref, __zswap_pool_empty,
> 292 PERCPU_REF_ALLOW_REINIT, GFP_KERNEL);
> 293 if (ret)
> 294 goto ref_fail;
> 295 INIT_LIST_HEAD(&pool->list);
> 296
> 297 zswap_pool_debug("created", pool);
> 298
> 299 return pool;
> 300
> 301 ref_fail:
> 302 cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
> 303 error:
> 304 if (pool->acomp_ctx)
> 305 free_percpu(pool->acomp_ctx);
> --> 306 zpool_destroy_pool(pool->zpool);
> ^^^^^^^^^^^
> NULL dereference
>
> 307 kfree(pool);
> 308 return NULL;
> 309 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-06-20 9:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-20 8:52 [bug report] mm/zswap: use only one pool in zswap Dan Carpenter
2024-06-20 9:12 ` Chengming Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox