Skip to content

Commit

Permalink
cr-dump: fix cr_imgset leak in dump_one_task
Browse files Browse the repository at this point in the history
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 <ptikhomirov@virtuozzo.com>
  • Loading branch information
Snorch authored and avagin committed Apr 29, 2022
1 parent 2747bb2 commit 7e9a9dc
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions criu/cr-dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7e9a9dc

Please sign in to comment.