[CRIU] [PATCH] fix many unclosed file opened by open_image_ro
Huang Qiang
h.huangqiang at huawei.com
Wed Oct 24 05:13:12 EDT 2012
Many image files opened by open_image_ro weren't closed before return, fix
them all in this patch.
Signed-off-by: Huang Qiang <h.huangqiang at huawei.com>
---
cr-restore.c | 8 ++++++--
cr-show.c | 1 +
files-reg.c | 14 +++++++++-----
files.c | 16 ++++++++++------
ipc_ns.c | 47 ++++++++++++++++++++++++++++++++---------------
mount.c | 11 +++++++----
net.c | 1 +
sk-queue.c | 15 +++++++++------
8 files changed, 75 insertions(+), 38 deletions(-)
diff --git a/cr-restore.c b/cr-restore.c
index 9c8a729..a4a3242 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -1199,8 +1199,10 @@ static int prepare_mm(pid_t pid, struct task_restore_core_args *args)
if (fd < 0)
return -1;
- if (pb_read_one(fd, &mm, PB_MM) < 0)
+ if (pb_read_one(fd, &mm, PB_MM) < 0) {
+ close_safe(&fd);
return -1;
+ }
args->mm = *mm;
args->mm.n_mm_saved_auxv = 0;
@@ -1349,7 +1351,7 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core, struct list_head *tgt_v
fd_pages = open_image_ro(CR_FD_PAGES, pid);
if (fd_pages < 0) {
pr_perror("Can't open pages-%d", pid);
- goto err;
+ goto free;
}
restore_task_vma_len = round_up(sizeof(*task_args), PAGE_SIZE);
@@ -1587,6 +1589,8 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core, struct list_head *tgt_v
: "rsp", "rdi", "rsi", "rbx", "rax", "memory");
err:
+ close_safe(&fd_pages);
+free:
free_mappings(&self_vma_list);
/* Just to be sure */
diff --git a/cr-show.c b/cr-show.c
index b4a112b..8e88584 100644
--- a/cr-show.c
+++ b/cr-show.c
@@ -476,6 +476,7 @@ static int cr_show_all(struct cr_options *opts)
pr_msg("----------------------------------------\n");
show_core(fd_th, opts);
+ close_safe(&fd_th);
pr_msg("----------------------------------------\n");
diff --git a/files-reg.c b/files-reg.c
index f5cb4f2..8d1018d 100644
--- a/files-reg.c
+++ b/files-reg.c
@@ -86,14 +86,14 @@ static int open_remap_ghost(struct reg_file_info *rfi,
goto err;
if (pb_read_one(ifd, &gfe, PB_GHOST_FILE) < 0)
- goto err;
+ goto close_ifd;
snprintf(gf->remap.path, PATH_MAX, "%s.cr.%x.ghost", rfi->path, rfe->remap_id);
if (S_ISFIFO(gfe->mode)) {
if (mknod(gf->remap.path, gfe->mode, 0)) {
pr_perror("Can't create node for ghost file\n");
- goto err;
+ goto close_ifd;
}
ghost_flags = O_RDWR; /* To not block */
} else
@@ -102,17 +102,17 @@ static int open_remap_ghost(struct reg_file_info *rfi,
gfd = open(gf->remap.path, ghost_flags, gfe->mode);
if (gfd < 0) {
pr_perror("Can't open ghost file");
- goto err;
+ goto close_ifd;
}
if (fchown(gfd, gfe->uid, gfe->gid) < 0) {
pr_perror("Can't reset user/group on ghost %#x\n", rfe->remap_id);
- goto err;
+ goto close_all;
}
if (S_ISREG(gfe->mode)) {
if (copy_file(ifd, gfd, 0) < 0)
- goto err;
+ goto close_all;
}
ghost_file_entry__free_unpacked(gfe, NULL);
@@ -127,6 +127,10 @@ gf_found:
rfi->remap = &gf->remap;
return 0;
+close_all:
+ close_safe(&gfd);
+close_ifd:
+ close_safe(&ifd);
err:
if (gfe)
ghost_file_entry__free_unpacked(gfe, NULL);
diff --git a/files.c b/files.c
index 617a891..9d25f16 100644
--- a/files.c
+++ b/files.c
@@ -576,21 +576,22 @@ int prepare_fs(int pid)
if (ifd < 0)
return -1;
- if (pb_read_one(ifd, &fe, PB_FS) < 0)
+ if (pb_read_one(ifd, &fe, PB_FS) < 0) {
+ close_safe(&ifd);
return -1;
+ }
cwd = open_reg_by_id(fe->cwd_id);
- if (cwd < 0)
+ if (cwd < 0) {
+ close_safe(&ifd);
goto err;
+ }
if (fchdir(cwd) < 0) {
pr_perror("Can't change root");
- goto err;
+ goto close;
}
- close(cwd);
- close(ifd);
-
/*
* FIXME: restore task's root. Don't want to do it now, since
* it's not yet clean how we're going to resolve tasks' paths
@@ -601,6 +602,9 @@ int prepare_fs(int pid)
*/
ret = 0;
+close:
+ close_safe(&cwd);
+ close_safe(&ifd);
err:
fs_entry__free_unpacked(fe, NULL);
return ret;
diff --git a/ipc_ns.c b/ipc_ns.c
index 9e259fe..b72b0e1 100644
--- a/ipc_ns.c
+++ b/ipc_ns.c
@@ -611,7 +611,7 @@ static int prepare_ipc_sem_desc(int fd, const IpcSemEntry *entry)
static int prepare_ipc_sem(int pid)
{
- int fd;
+ int fd, ret;
pr_info("Restoring IPC semaphores sets\n");
fd = open_image_ro(CR_FD_IPCNS_SEM, pid);
@@ -619,12 +619,13 @@ static int prepare_ipc_sem(int pid)
return -1;
while (1) {
- int ret;
IpcSemEntry *entry;
ret = pb_read_one_eof(fd, &entry, PB_IPCNS_SEM);
- if (ret < 0)
- return -EIO;
+ if (ret < 0) {
+ ret = -EIO;
+ goto err;
+ }
if (ret == 0)
break;
@@ -635,10 +636,15 @@ static int prepare_ipc_sem(int pid)
if (ret < 0) {
pr_err("Failed to prepare semaphores set\n");
- return ret;
+ goto err;
}
}
+
return close_safe(&fd);
+err:
+ close_safe(&fd);
+ return ret;
+
}
static int prepare_ipc_msg_queue_messages(int fd, const IpcMsgEntry *entry)
@@ -728,7 +734,7 @@ static int prepare_ipc_msg_queue(int fd, const IpcMsgEntry *entry)
static int prepare_ipc_msg(int pid)
{
- int fd;
+ int fd, ret;
pr_info("Restoring IPC message queues\n");
fd = open_image_ro(CR_FD_IPCNS_MSG, pid);
@@ -736,13 +742,13 @@ static int prepare_ipc_msg(int pid)
return -1;
while (1) {
- int ret;
IpcMsgEntry *entry;
ret = pb_read_one_eof(fd, &entry, PB_IPCNS_MSG_ENT);
if (ret < 0) {
pr_err("Failed to read IPC messages queue\n");
- return -EIO;
+ ret = -EIO;
+ goto err;
}
if (ret == 0)
break;
@@ -754,10 +760,13 @@ static int prepare_ipc_msg(int pid)
if (ret < 0) {
pr_err("Failed to prepare messages queue\n");
- return ret;
+ goto err;
}
}
return close_safe(&fd);
+err:
+ close_safe(&fd);
+ return ret;
}
static int prepare_ipc_shm_pages(int fd, const IpcShmEntry *shm)
@@ -822,7 +831,7 @@ static int prepare_ipc_shm_seg(int fd, const IpcShmEntry *shm)
static int prepare_ipc_shm(int pid)
{
- int fd;
+ int fd, ret;
pr_info("Restoring IPC shared memory\n");
fd = open_image_ro(CR_FD_IPCNS_SHM, pid);
@@ -830,13 +839,13 @@ static int prepare_ipc_shm(int pid)
return -1;
while (1) {
- int ret;
IpcShmEntry *shm;
ret = pb_read_one_eof(fd, &shm, PB_IPCNS_SHM);
if (ret < 0) {
pr_err("Failed to read IPC shared memory segment\n");
- return -EIO;
+ ret = -EIO;
+ goto err;
}
if (ret == 0)
break;
@@ -848,10 +857,13 @@ static int prepare_ipc_shm(int pid)
if (ret < 0) {
pr_err("Failed to prepare shm segment\n");
- return ret;
+ goto err;
}
}
return close_safe(&fd);
+err:
+ close_safe(&fd);
+ return ret;
}
static int prepare_ipc_var(int pid)
@@ -867,7 +879,8 @@ static int prepare_ipc_var(int pid)
ret = pb_read_one(fd, &var, PB_IPCNS_VAR);
if (ret <= 0) {
pr_err("Failed to read IPC namespace variables\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto err;
}
ipc_sysctl_req(var, CTL_PRINT);
@@ -877,9 +890,13 @@ static int prepare_ipc_var(int pid)
if (ret < 0) {
pr_err("Failed to prepare IPC namespace variables\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto err;
}
return close_safe(&fd);
+err:
+ close_safe(&fd);
+ return ret;
}
int prepare_ipc_ns(int pid)
diff --git a/mount.c b/mount.c
index b8258e0..0f34298 100644
--- a/mount.c
+++ b/mount.c
@@ -646,22 +646,22 @@ static int populate_mnt_ns(int ns_pid)
pr_debug("\t\tGetting root for %d\n", pm->mnt_id);
pm->root = xstrdup(me->root);
if (!pm->root)
- return -1;
+ goto err;
pr_debug("\t\tGetting mpt for %d\n", pm->mnt_id);
pm->mountpoint = xstrdup(me->mountpoint);
if (!pm->mountpoint)
- return -1;
+ goto err;
pr_debug("\t\tGetting source for %d\n", pm->mnt_id);
pm->source = xstrdup(me->source);
if (!pm->source)
- return -1;
+ goto err;
pr_debug("\t\tGetting opts for %d\n", pm->mnt_id);
pm->options = xstrdup(me->options);
if (!pm->options)
- return -1;
+ goto err;
pr_debug("\tRead %d mp @ %s\n", pm->mnt_id, pm->mountpoint);
pm->next = pms;
@@ -679,6 +679,9 @@ static int populate_mnt_ns(int ns_pid)
return -1;
return mnt_tree_for_each(pms, do_mount_one);
+err:
+ close_safe(&img);
+ return -1;
}
int prepare_mnt_ns(int ns_pid)
diff --git a/net.c b/net.c
index 4770054..2763e13 100644
--- a/net.c
+++ b/net.c
@@ -265,6 +265,7 @@ static int restore_links(int pid)
nlsk = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (nlsk < 0) {
pr_perror("Can't create nlk socket");
+ close_safe(&fd);
return -1;
}
diff --git a/sk-queue.c b/sk-queue.c
index f8394b8..992e74e 100644
--- a/sk-queue.c
+++ b/sk-queue.c
@@ -227,28 +227,28 @@ int restore_sk_queue(int fd, unsigned int peer_id)
buf = xmalloc(entry->length);
if (buf ==NULL)
- return -1;
+ goto err;
if (lseek(img_fd, pkt->img_off, SEEK_SET) == -1) {
pr_perror("lseek() failed");
xfree(buf);
- return -1;
+ goto err;
}
if (read_img_buf(img_fd, buf, entry->length) != 1) {
xfree(buf);
- return -1;
+ goto err;
}
ret = write(fd, buf, entry->length);
xfree(buf);
if (ret < 0) {
pr_perror("Failed to send packet");
- return -1;
+ goto err;
}
if (ret != entry->length) {
pr_err("Restored skb trimmed to %d/%d\n",
ret, (unsigned int)entry->length);
- return -1;
+ goto err;
}
list_del(&pkt->list);
sk_packet_entry__free_unpacked(entry, NULL);
@@ -257,9 +257,12 @@ int restore_sk_queue(int fd, unsigned int peer_id)
if (fcntl(fd, F_SETFL, flags) ) {
pr_perror("Unable to restore flags for %d", fd);
- return -1;
+ goto err;
}
close(img_fd);
return 0;
+err:
+ close_safe(&img_fd);
+ return -1;
}
--
1.7.1
More information about the CRIU
mailing list