The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2511
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Hello, coverity issues are fixed. Thanks.
From 8ddce7df134615b1f7c1db296f8088fe3be60631 Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 13:12:00 +0900 Subject: [PATCH 1/7] coverity: #1438236 Resource leak Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index e9e01e32b..3ba423c6a 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -776,8 +776,10 @@ static char *cgv1_must_prefix_named(char *entry) s = must_alloc(len + 6); ret = snprintf(s, len + 6, "name=%s", entry); - if (ret < 0 || (size_t)ret >= (len + 6)) + if (ret < 0 || (size_t)ret >= (len + 6)) { + free(s); return NULL; + } return s; } From 9159b38c434bf0bcc11f62c96a9aa6cfe727ab14 Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 13:19:53 +0900 Subject: [PATCH 2/7] coverity: #1438235 Resource leak Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index 3ba423c6a..307b5840c 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -1185,8 +1185,7 @@ static bool cgv1_init(uid_t uid, gid_t gid) if (!controller_list) continue; - if (cgv1_controller_list_is_dup(cgv1_hierarchies, - controller_list)) { + if (cgv1_controller_list_is_dup(cgv1_hierarchies, controller_list)) { free(controller_list); continue; } @@ -1203,13 +1202,14 @@ static bool cgv1_init(uid_t uid, gid_t gid) free(mountpoint); continue; } + trim(base_cgroup); pam_cgfs_debug("Detected cgroupfs v1 controller \"%s\" with " - "mountpoint \"%s\" and cgroup \"%s\".\n", - controller_list[0], mountpoint, base_cgroup); - cgv1_add_controller(controller_list, mountpoint, base_cgroup, - NULL); + "mountpoint \"%s\" and cgroup \"%s\"\n", + controller_list[0], mountpoint, base_cgroup); + cgv1_add_controller(controller_list, mountpoint, base_cgroup, NULL); } + free_string_list(klist); free_string_list(nlist); free(basecginfo); @@ -1224,6 +1224,7 @@ static bool cgv1_init(uid_t uid, gid_t gid) for (it = cgv1_hierarchies; it && *it; it++) { if ((*it)->controllers) { char *init_cgroup, *user_slice; + /* We've already stored the controller and received its * current cgroup. If we now fail to retrieve its init * cgroup, we should probably fail. @@ -1233,17 +1234,20 @@ static bool cgv1_init(uid_t uid, gid_t gid) free(basecginfo); return false; } + cg_systemd_prune_init_scope(init_cgroup); (*it)->init_cgroup = init_cgroup; pam_cgfs_debug("cgroupfs v1 controller \"%s\" has init " - "cgroup \"%s\".\n", - (*(*it)->controllers), init_cgroup); + "cgroup \"%s\".\n", + (*(*it)->controllers), init_cgroup); /* Check whether systemd has already created a cgroup * for us. */ user_slice = must_make_path((*it)->mountpoint, (*it)->base_cgroup, NULL); if (cg_systemd_created_user_slice((*it)->base_cgroup, (*it)->init_cgroup, user_slice, uid)) (*it)->systemd_user_slice = true; + + free(user_slice); } } free(basecginfo); From d97c3a345a7b474714333ddc670a405e9cf35844 Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 13:44:46 +0900 Subject: [PATCH 3/7] coverity: #1438234 Resource leak Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index 307b5840c..072c9a41f 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -1351,14 +1351,17 @@ static bool cgv2_init(uid_t uid, gid_t gid) } pam_cgfs_debug("Detected cgroupfs v2 hierarchy at mountpoint \"%s\" with " - "current cgroup \"%s\" and init cgroup \"%s\".\n", - mountpoint, current_cgroup, init_cgroup); + "current cgroup \"%s\" and init cgroup \"%s\"\n", + mountpoint, current_cgroup, init_cgroup); cleanup: if (f) fclose(f); free(line); + if (!ret) + free(current_cgroup); + return ret; } From 90a170d8bed6e7f5b2f84260a666b16c011aaddd Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 13:54:34 +0900 Subject: [PATCH 4/7] coverity: #1438233 Resource leak Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index 072c9a41f..cb3286408 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -1295,6 +1295,7 @@ static bool cgv2_init(uid_t uid, gid_t gid) */ goto cleanup; } + cg_systemd_prune_init_scope(init_cgroup); /* Check if the v2 hierarchy is mounted at its standard location. @@ -1329,6 +1330,7 @@ static bool cgv2_init(uid_t uid, gid_t gid) while (getline(&line, &len, f) != -1) { char *user_slice; bool has_user_slice = false; + if (!is_cgv2(line)) continue; @@ -1342,6 +1344,7 @@ static bool cgv2_init(uid_t uid, gid_t gid) free(user_slice); cgv2_add_controller(NULL, mountpoint, current_cgroup, init_cgroup, has_user_slice); + /* Although the unified hierarchy can be mounted multiple times, * each of those mountpoints will expose identical information. * So let the first mountpoint we find, win. @@ -1359,8 +1362,10 @@ static bool cgv2_init(uid_t uid, gid_t gid) fclose(f); free(line); - if (!ret) + if (!ret) { + free(init_cgroup); free(current_cgroup); + } return ret; } From 8ae3983ed23b429234c57de368b05759b69fa19b Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 14:01:33 +0900 Subject: [PATCH 5/7] coverity: #1438229 Resource leak Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index cb3286408..285842946 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -1772,8 +1772,10 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized) pam_cgfs_debug("Invalid path: %s.\n", path); return bret; } + oldv = *lastslash; *lastslash = '\0'; + fpath = must_make_path(path, "cpuset.cpus", NULL); posscpus = read_file(fpath); if (!posscpus) { @@ -1790,6 +1792,7 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized) /* This system doesn't expose isolated cpus. */ pam_cgfs_debug("%s", "Path: "__ISOL_CPUS" to read isolated cpus from does not exist.\n"); cpulist = posscpus; + /* No isolated cpus but we weren't already initialized by * someone. We should simply copy the parents cpuset.cpus * values. @@ -1798,6 +1801,7 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized) pam_cgfs_debug("%s", "Copying cpuset of parent cgroup.\n"); goto copy_parent; } + /* No isolated cpus but we were already initialized by someone. * Nothing more to do for us. */ @@ -1809,9 +1813,11 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized) pam_cgfs_debug("%s", "Could not read file "__ISOL_CPUS"\n"); goto on_error; } + if (!isdigit(isolcpus[0])) { pam_cgfs_debug("%s", "No isolated cpus detected.\n"); cpulist = posscpus; + /* No isolated cpus but we weren't already initialized by * someone. We should simply copy the parents cpuset.cpus * values. @@ -1820,6 +1826,7 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized) pam_cgfs_debug("%s", "Copying cpuset of parent cgroup.\n"); goto copy_parent; } + /* No isolated cpus but we were already initialized by someone. * Nothing more to do for us. */ @@ -1868,6 +1875,10 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized) copy_parent: *lastslash = oldv; + + if (fpath) + free(fpath); + fpath = must_make_path(path, "cpuset.cpus", NULL); ret = write_to_file(fpath, cpulist, strlen(cpulist), false); if (ret < 0) { From ea8bb2a9947053f075a13f3d4cc70b1003fc7f1c Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 14:03:22 +0900 Subject: [PATCH 6/7] coverity: #1438230 Logically dead code Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index 285842946..bd928dc4d 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -825,8 +825,6 @@ static char **cgv1_get_proc_mountinfo_controllers(char **klist, char **nlist, ch return NULL; p++; } - if (!p) - return NULL; if (strncmp(p, "/sys/fs/cgroup/", 15) != 0) return NULL; From 03e7d72aeb93a3f51ede7326659de46b6d003465 Mon Sep 17 00:00:00 2001 From: 2xsec <dh48.je...@samsung.com> Date: Mon, 6 Aug 2018 14:11:46 +0900 Subject: [PATCH 7/7] coverity: #1438231 Dereference after null check Signed-off-by: 2xsec <dh48.je...@samsung.com> --- src/lxc/pam/pam_cgfs.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index bd928dc4d..a36380cbe 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -1558,7 +1558,7 @@ static bool get_uid_gid(const char *user, uid_t *uid, gid_t *gid) if (!pwentp) { if (ret == 0) mysyslog(LOG_ERR, - "Could not find matched password record\n", NULL); + "Could not find matched password record\n", NULL); free(buf); return false; @@ -1610,6 +1610,7 @@ static uint32_t *cg_cpumask(char *buf, size_t nbits) char *range = strchr(token, '-'); if (range) end = strtoul(range + 1, NULL, 0); + if (!(start <= end)) { free(bitarr); return NULL; @@ -1678,9 +1679,11 @@ static char *cg_cpumask_to_cpulist(uint32_t *bitarr, size_t nbits) free_string_list(cpulist); return NULL; } + must_append_string(&cpulist, numstr); } } + return string_join(",", (const char **)cpulist, false); } @@ -1703,10 +1706,12 @@ static ssize_t cg_get_max_cpus(char *cpulist) else if (c1 < c2) c1 = c2; + if (!c1) + return -1; + /* If the above logic is correct, c1 should always hold a valid string * here. */ - errno = 0; cpus = strtoul(c1, NULL, 0); if (errno != 0) @@ -1718,10 +1723,12 @@ static ssize_t cg_get_max_cpus(char *cpulist) static ssize_t write_nointr(int fd, const void* buf, size_t count) { ssize_t ret; + again: ret = write(fd, buf, count); if (ret < 0 && errno == EINTR) goto again; + return ret; } @@ -1733,16 +1740,19 @@ static int write_to_file(const char *filename, const void* buf, size_t count, bo fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, 0666); if (fd < 0) return -1; + ret = write_nointr(fd, buf, count); if (ret < 0) goto out_error; if ((size_t)ret != count) goto out_error; + if (add_newline) { ret = write_nointr(fd, "\n", 1); if (ret != 1) goto out_error; } + close(fd); return 0;
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel