From 223dce83c2e337cd2fdb2affb9deb6e4553c0ae4 Mon Sep 17 00:00:00 2001 From: Huang Qiang Date: Wed, 24 Oct 2012 16:51:50 +0400 Subject: [PATCH] fix many unclosed file opened by open_image_ro Many image files opened by open_image_ro weren't closed before return, fix them all in this patch. Signed-off-by: Huang Qiang Acked-by: Andrew Vagin Signed-off-by: Pavel Emelyanov --- cr-restore.c | 8 ++++++-- cr-show.c | 1 + files-reg.c | 14 +++++++++----- files.c | 20 ++++++++++++-------- ipc_ns.c | 46 +++++++++++++++++++++++++++++++--------------- mount.c | 11 +++++++---- net.c | 1 + sk-queue.c | 15 +++++++++------ 8 files changed, 76 insertions(+), 40 deletions(-) diff --git a/cr-restore.c b/cr-restore.c index 9c8a7297c..a4a324256 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 b4a112bc4..8e8858433 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 f5cb4f228..8d1018de3 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 617a89148..9d25f1630 100644 --- a/files.c +++ b/files.c @@ -576,20 +576,21 @@ 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) - goto err; - - if (fchdir(cwd) < 0) { - pr_perror("Can't change root"); + if (cwd < 0) { + close_safe(&ifd); goto err; } - close(cwd); - close(ifd); + if (fchdir(cwd) < 0) { + pr_perror("Can't change root"); + goto close; + } /* * FIXME: restore task's root. Don't want to do it now, since @@ -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 9e259fefc..d006b3da0 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,14 @@ 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 +733,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 +741,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 +759,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 +830,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 +838,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 +856,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 +878,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 +889,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 b8258e0c9..0f3429828 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 4770054c8..2763e13bf 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 f8394b874..992e74e52 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; }