[lxc-devel] [lxc/master] fix coverity issues
2xsec on Github
lxc-bot at linuxcontainers.org
Mon Aug 6 05:37:09 UTC 2018
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 348 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180806/689d9380/attachment.bin>
-------------- next part --------------
From 8ddce7df134615b1f7c1db296f8088fe3be60631 Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.jeong at samsung.com>
Date: Mon, 6 Aug 2018 13:12:00 +0900
Subject: [PATCH 1/7] coverity: #1438236
Resource leak
Signed-off-by: 2xsec <dh48.jeong at 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.jeong at samsung.com>
Date: Mon, 6 Aug 2018 13:19:53 +0900
Subject: [PATCH 2/7] coverity: #1438235
Resource leak
Signed-off-by: 2xsec <dh48.jeong at 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.jeong at samsung.com>
Date: Mon, 6 Aug 2018 13:44:46 +0900
Subject: [PATCH 3/7] coverity: #1438234
Resource leak
Signed-off-by: 2xsec <dh48.jeong at 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.jeong at samsung.com>
Date: Mon, 6 Aug 2018 13:54:34 +0900
Subject: [PATCH 4/7] coverity: #1438233
Resource leak
Signed-off-by: 2xsec <dh48.jeong at 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.jeong at samsung.com>
Date: Mon, 6 Aug 2018 14:01:33 +0900
Subject: [PATCH 5/7] coverity: #1438229
Resource leak
Signed-off-by: 2xsec <dh48.jeong at 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.jeong at 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.jeong at 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.jeong at 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.jeong at 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;
More information about the lxc-devel
mailing list