Merge pull request #2723 from 2xsec/bugfix

src/lxc/Makefile.am

@@ -424,9 +424,7 @@ if HAVE_PAM
 pam_LTLIBRARIES = pam_cgfs.la
 
 pam_cgfs_la_SOURCES = pam/pam_cgfs.c \
-		      caps.c caps.h \
 		      file_utils.c file_utils.h \
-		      log.c log.h \
 		      macro.h \
 		      string_utils.c string_utils.h
 

src/lxc/cgroups/cgfsng.c

@@ -84,12 +84,6 @@ static void free_string_list(char **clist)
 	free(clist);
 }
 
-/* Allocate a pointer, do not fail. */
-static void *must_alloc(size_t sz)
-{
-	return must_realloc(NULL, sz);
-}
-
 /* Given a pointer to a null-terminated array of pointers, realloc to add one
  * entry, and point the new entry to NULL. Do not fail. Return the index to the
  * second-to-last entry - that is, the one which is now available for use
@@ -134,7 +128,7 @@ static char *cg_legacy_must_prefix_named(char *entry)
 	char *prefixed;
 
 	len = strlen(entry);
-	prefixed = must_alloc(len + 6);
+	prefixed = must_realloc(NULL, len + 6);
 
 	memcpy(prefixed, "name=", STRLITERALLEN("name="));
 	memcpy(prefixed + STRLITERALLEN("name="), entry, len);
@@ -541,7 +535,7 @@ static bool copy_parent_file(char *path, char *file)
 	if (len <= 0)
 		goto on_error;
 
-	value = must_alloc(len + 1);
+	value = must_realloc(NULL, len + 1);
 	ret = lxc_read_from_file(fpath, value, len);
 	if (ret != len)
 		goto on_error;
@@ -824,7 +818,7 @@ static struct hierarchy *add_hierarchy(struct hierarchy ***h, char **clist, char
 	struct hierarchy *new;
 	int newentry;
 
-	new = must_alloc(sizeof(*new));
+	new = must_realloc(NULL, sizeof(*new));
 	new->controllers = clist;
 	new->mountpoint = mountpoint;
 	new->container_base_path = container_base_path;
@@ -863,7 +857,7 @@ static char *cg_hybrid_get_mountpoint(char *line)
 	*p2 = '\0';
 
 	len = strlen(p);
-	sret = must_alloc(len + 1);
+	sret = must_realloc(NULL, len + 1);
 	memcpy(sret, p, len);
 	sret[len] = '\0';
 	return sret;
@@ -879,7 +873,7 @@ static char *copy_to_eol(char *p)
 		return NULL;
 
 	len = p2 - p;
-	sret = must_alloc(len + 1);
+	sret = must_realloc(NULL, len + 1);
 	memcpy(sret, p, len);
 	sret[len] = '\0';
 	return sret;
@@ -1466,7 +1460,7 @@ __cgfsng_ops static inline bool cgfsng_payload_create(struct cgroup_ops *ops,
 	}
 
 	len = strlen(tmp) + 5; /* leave room for -NNN\0 */
-	container_cgroup = must_alloc(len);
+	container_cgroup = must_realloc(NULL, len);
 	(void)strlcpy(container_cgroup, tmp, len);
 	free(tmp);
 	offset = container_cgroup + len - 5;
@@ -2110,7 +2104,7 @@ static int __cg_unified_attach(const struct hierarchy *h, const char *name,
 
 	len = strlen(base_path) + STRLITERALLEN("/lxc-1000") +
 	      STRLITERALLEN("/cgroup-procs");
-	full_path = must_alloc(len + 1);
+	full_path = must_realloc(NULL, len + 1);
 	do {
 		if (idx)
 			ret = snprintf(full_path, len + 1, "%s/lxc-%d",

src/lxc/file_utils.c

@@ -30,12 +30,9 @@
 
 #include "config.h"
 #include "file_utils.h"
-#include "log.h"
 #include "macro.h"
 #include "string.h"
 
-lxc_log_define(file_utils, lxc);
-
 int lxc_write_to_file(const char *filename, const void *buf, size_t count,
 		      bool add_newline, mode_t mode)
 {

src/lxc/initutils.c

@@ -321,7 +321,7 @@ int setproctitle(char *title)
 	if (ret == 0)
 		(void)strlcpy((char*)arg_start, title, len);
 	else
-		SYSINFO("setting cmdline failed");
+		SYSWARN("Failed to set cmdline");
 
 	return ret;
 }

src/lxc/pam/pam_cgfs.c

@@ -57,8 +57,9 @@
 #include <unistd.h>
 
 #include "config.h"
+#include "file_utils.h"
 #include "macro.h"
-#include "utils.h"
+#include "string_utils.h"
 
 #define PAM_SM_SESSION
 #include <security/_pam_macros.h>
@@ -119,14 +120,12 @@ static inline bool is_set(unsigned bit, uint32_t *bitarr)
 static bool is_lxcfs(const char *line);
 static bool is_cgv1(char *line);
 static bool is_cgv2(char *line);
-static void *must_alloc(size_t sz);
 static void must_add_to_list(char ***clist, char *entry);
 static void must_append_controller(char **klist, char **nlist, char ***clist,
 				   char *entry);
 static void must_append_string(char ***list, char *entry);
 static void mysyslog(int err, const char *format, ...) __attribute__((sentinel));
 static char *read_file(char *fnam);
-static int read_from_file(const char *filename, void* buf, size_t count);
 static int recursive_rmdir(char *dirname);
 static inline void set_bit(unsigned bit, uint32_t *bitarr)
 {
@@ -136,9 +135,6 @@ static bool string_in_list(char **list, const char *entry);
 static char *string_join(const char *sep, const char **parts, bool use_as_prefix);
 static void trim(char *s);
 static bool write_int(char *path, int v);
-static ssize_t write_nointr(int fd, const void* buf, size_t count);
-static int write_to_file(const char *filename, const void *buf, size_t count,
-			 bool add_newline);
 
 /* cgroupfs prototypes. */
 static bool cg_belongs_to_uid_gid(const char *path, uid_t uid, gid_t gid);
@@ -392,12 +388,6 @@ static void trim(char *s)
 		s[--len] = '\0';
 }
 
-/* Allocate pointer; do not fail. */
-static void *must_alloc(size_t sz)
-{
-	return must_realloc(NULL, sz);
-}
-
 /* Make allocated copy of string. End of string is taken to be '\n'. */
 static char *copy_to_eol(char *s)
 {
@@ -409,7 +399,7 @@ static char *copy_to_eol(char *s)
 		return NULL;
 
 	len = newline - s;
-	sret = must_alloc(len + 1);
+	sret = must_realloc(NULL, len + 1);
 	memcpy(sret, s, len);
 	sret[len] = '\0';
 
@@ -607,7 +597,7 @@ static char *get_mountpoint(char *line)
 		*p2 = '\0';
 
 	len = strlen(p);
-	sret = must_alloc(len + 1);
+	sret = must_realloc(NULL, len + 1);
 	memcpy(sret, p, len);
 	sret[len] = '\0';
 
@@ -779,7 +769,7 @@ static char *cgv1_must_prefix_named(char *entry)
 	size_t len;
 
 	len = strlen(entry);
-	s = must_alloc(len + 6);
+	s = must_realloc(NULL, len + 6);
 
 	ret = snprintf(s, len + 6, "name=%s", entry);
 	if (ret < 0 || (size_t)ret >= (len + 6)) {
@@ -941,7 +931,7 @@ static void cgv1_add_controller(char **clist, char *mountpoint, char *base_cgrou
 	struct cgv1_hierarchy *new;
 	int newentry;
 
-	new = must_alloc(sizeof(*new));
+	new = must_realloc(NULL, sizeof(*new));
 
 	new->controllers = clist;
 	new->mountpoint = mountpoint;
@@ -968,7 +958,7 @@ static void cgv2_add_controller(char **clist, char *mountpoint, char *base_cgrou
 	struct cgv2_hierarchy *new;
 	int newentry;
 
-	new = must_alloc(sizeof(*new));
+	new = must_realloc(NULL, sizeof(*new));
 
 	new->controllers = clist;
 	new->mountpoint = mountpoint;
@@ -1738,49 +1728,6 @@ static ssize_t cg_get_max_cpus(char *cpulist)
 	return cpus;
 }
 
-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;
-}
-
-static int write_to_file(const char *filename, const void* buf, size_t count, bool add_newline)
-{
-	int fd, saved_errno;
-	ssize_t ret;
-
-	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;
-
-out_error:
-	saved_errno = errno;
-	close(fd);
-	errno = saved_errno;
-	return -1;
-}
-
 #define __ISOL_CPUS "/sys/devices/system/cpu/isolated"
 static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
 {
@@ -1905,7 +1852,7 @@ copy_parent:
 	free(fpath);
 
 	fpath = must_make_path(path, "cpuset.cpus", NULL);
-	ret = write_to_file(fpath, cpulist, strlen(cpulist), false);
+	ret = lxc_write_to_file(fpath, cpulist, strlen(cpulist), false, 0660);
 	if (ret < 0) {
 		pam_cgfs_debug("Could not write cpu list to: %s\n", fpath);
 		goto on_error;
@@ -1929,37 +1876,6 @@ on_error:
 	return bret;
 }
 
-int read_from_file(const char *filename, void* buf, size_t count)
-{
-	int fd = -1, saved_errno;
-	ssize_t ret;
-
-	fd = open(filename, O_RDONLY | O_CLOEXEC);
-	if (fd < 0)
-		return -1;
-
-	if (!buf || !count) {
-		char buf2[100];
-		size_t count2 = 0;
-
-		while ((ret = read(fd, buf2, 100)) > 0)
-			count2 += ret;
-		if (ret >= 0)
-			ret = count2;
-	} else {
-		memset(buf, 0, count);
-		ret = read(fd, buf, count);
-	}
-
-	if (ret < 0)
-		pam_cgfs_debug("read %s: %s", filename, strerror(errno));
-
-	saved_errno = errno;
-	close(fd);
-	errno = saved_errno;
-	return ret;
-}
-
 /* Copy contents of parent(@path)/@file to @path/@file */
 static bool cg_copy_parent_file(char *path, char *file)
 {
@@ -1977,19 +1893,23 @@ static bool cg_copy_parent_file(char *path, char *file)
 	*lastslash = '\0';
 
 	fpath = must_make_path(path, file, NULL);
-	len = read_from_file(fpath, NULL, 0);
-	if (len <= 0)
+	len = lxc_read_from_file(fpath, NULL, 0);
+	if (len <= 0) {
+		pam_cgfs_debug("Failed to read %s: %s", fpath, strerror(errno));
 		goto bad;
+	}
 
-	value = must_alloc(len + 1);
-	if (read_from_file(fpath, value, len) != len)
+	value = must_realloc(NULL, len + 1);
+	if (lxc_read_from_file(fpath, value, len) != len) {
+		pam_cgfs_debug("Failed to read %s: %s", fpath, strerror(errno));
 		goto bad;
+	}
 	free(fpath);
 
 	*lastslash = oldv;
 
 	fpath = must_make_path(path, file, NULL);
-	ret = write_to_file(fpath, value, len, false);
+	ret = lxc_write_to_file(fpath, value, len, false, 0660);
 	if (ret < 0)
 		pam_cgfs_debug("Unable to write %s to %s", value, fpath);
 
@@ -2018,8 +1938,8 @@ static bool cgv1_handle_root_cpuset_hierarchy(struct cgv1_hierarchy *h)
 
 	clonechildrenpath = must_make_path(h->mountpoint, "cgroup.clone_children", NULL);
 
-	if (read_from_file(clonechildrenpath, &v, 1) < 0) {
-		pam_cgfs_debug("Failed to read '%s'", clonechildrenpath);
+	if (lxc_read_from_file(clonechildrenpath, &v, 1) < 0) {
+		pam_cgfs_debug("Failed to read %s: %s", clonechildrenpath, strerror(errno));
 		free(clonechildrenpath);
 		return false;
 	}
@@ -2029,7 +1949,7 @@ static bool cgv1_handle_root_cpuset_hierarchy(struct cgv1_hierarchy *h)
 		return true;
 	}
 
-	if (write_to_file(clonechildrenpath, "1", 1, false) < 0) {
+	if (lxc_write_to_file(clonechildrenpath, "1", 1, false, 0660) < 0) {
 		/* Set clone_children so children inherit our settings */
 		pam_cgfs_debug("Failed to write 1 to %s", clonechildrenpath);
 		free(clonechildrenpath);
@@ -2077,8 +1997,8 @@ static bool cgv1_handle_cpuset_hierarchy(struct cgv1_hierarchy *h,
 		return true;
 	}
 
-	if (read_from_file(clonechildrenpath, &v, 1) < 0) {
-		pam_cgfs_debug("Failed to read '%s'", clonechildrenpath);
+	if (lxc_read_from_file(clonechildrenpath, &v, 1) < 0) {
+		pam_cgfs_debug("Failed to read %s: %s", clonechildrenpath, strerror(errno));
 		free(clonechildrenpath);
 		free(cgpath);
 		return false;
@@ -2108,7 +2028,7 @@ static bool cgv1_handle_cpuset_hierarchy(struct cgv1_hierarchy *h,
 	}
 	free(cgpath);
 
-	if (write_to_file(clonechildrenpath, "1", 1, false) < 0) {
+	if (lxc_write_to_file(clonechildrenpath, "1", 1, false, 0660) < 0) {
 		/* Set clone_children so children inherit our settings */
 		pam_cgfs_debug("Failed to write 1 to %s", clonechildrenpath);
 		free(clonechildrenpath);

src/lxc/string_utils.c

@@ -29,6 +29,7 @@
 #include <inttypes.h>
 #include <libgen.h>
 #include <pthread.h>
+#include <stdarg.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -43,7 +44,6 @@
 #include <unistd.h>
 
 #include "config.h"
-#include "log.h"
 #include "lxclock.h"
 #include "macro.h"
 #include "namespace.h"
@@ -58,8 +58,6 @@
 #include "include/strlcat.h"
 #endif
 
-lxc_log_define(string_utils, lxc);
-
 char **lxc_va_arg_list_to_argv(va_list ap, size_t skip, int do_strdup)
 {
 	va_list ap2;