From 7e9a9dc34258fa087d12a9063a36cf84d8fee63f Mon Sep 17 00:00:00 2001 From: Pavel Tikhomirov Date: Wed, 9 Mar 2022 15:14:50 +0300 Subject: [PATCH] cr-dump: fix cr_imgset leak in dump_one_task coverity CID 389194: 1238static int dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie) 1239{ ... 1245 struct cr_imgset *cr_imgset = NULL; ... 11. alloc_fn: Storage is returned from allocation function cr_task_imgset_open. [show details] 12. var_assign: Assigning: cr_imgset = storage returned from cr_task_imgset_open(vpid(item), 577). 1355 cr_imgset = cr_task_imgset_open(vpid(item), O_DUMP); 13. Condition !cr_imgset, taking false branch. 1356 if (!cr_imgset) 1357 goto err_cure; 1358 ... 25. Condition opts.lazy_pages, taking false branch. 1427 if (opts.lazy_pages) 1428 ret = compel_cure_remote(parasite_ctl); 1429 else 1430 ret = compel_cure(parasite_ctl); 26. Condition ret, taking true branch. 1431 if (ret) { 1432 pr_err("Can't cure (pid: %d) from parasite\n", pid); 27. Jumping to label err. 1433 goto err; 1434 } ... 1448 close_cr_imgset(&cr_imgset); 1449 exit_code = 0; 1450err: 1451 close_pid_proc(); 1452 free_mappings(&vmas); 1453 xfree(dfds); CID 389194 (#1 of 1): Resource leak (RESOURCE_LEAK)28. leaked_storage: Variable cr_imgset going out of scope leaks the storage it points to. 1454 return exit_code; 1455 1456err_cure: 1457 close_cr_imgset(&cr_imgset); 1458err_cure_imgset: 1459 ret = compel_cure(parasite_ctl); 1460 if (ret) 1461 pr_err("Can't cure (pid: %d) from parasite\n", pid); 1462 goto err; 1463} On compel_cure() error path we do not do close_cr_imgset() thich leads to leaked cr_imgset, let's move corresponding close_cr_imgset below err label. Also now we can merge remove close_cr_imgset() in err_cure label as it goes to err label later anyway. Separate err_cure_imgset label is not needed as close_cr_imgset() is ready for cr_imgset == NULL. v2: remove excess close_cr_imgset() in label err_cure (@adrianreber) Signed-off-by: Pavel Tikhomirov --- criu/cr-dump.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/criu/cr-dump.c b/criu/cr-dump.c index c972e343aa..eb1fb5e9a7 100644 --- a/criu/cr-dump.c +++ b/criu/cr-dump.c @@ -1315,29 +1315,29 @@ static int dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie) pfd = parasite_get_proc_fd_seized(parasite_ctl); if (pfd < 0) { pr_err("Can't get proc fd (pid: %d)\n", pid); - goto err_cure_imgset; + goto err_cure; } if (install_service_fd(CR_PROC_FD_OFF, pfd) < 0) - goto err_cure_imgset; + goto err_cure; } ret = parasite_fixup_vdso(parasite_ctl, pid, &vmas); if (ret) { pr_err("Can't fixup vdso VMAs (pid: %d)\n", pid); - goto err_cure_imgset; + goto err_cure; } ret = parasite_collect_aios(parasite_ctl, &vmas); /* FIXME -- merge with above */ if (ret) { pr_err("Failed to check aio rings (pid: %d)\n", pid); - goto err_cure_imgset; + goto err_cure; } ret = parasite_dump_misc_seized(parasite_ctl, &misc); if (ret) { pr_err("Can't dump misc (pid: %d)\n", pid); - goto err_cure_imgset; + goto err_cure; } item->pid->ns[0].virt = misc.pid; @@ -1445,17 +1445,15 @@ static int dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie) goto err; } - close_cr_imgset(&cr_imgset); exit_code = 0; err: + close_cr_imgset(&cr_imgset); close_pid_proc(); free_mappings(&vmas); xfree(dfds); return exit_code; err_cure: - close_cr_imgset(&cr_imgset); -err_cure_imgset: ret = compel_cure(parasite_ctl); if (ret) pr_err("Can't cure (pid: %d) from parasite\n", pid);