Merge pull request #2347 from brauner/2018-05-24/seccomp_cleanups

src/lxc/lxccontainer.c

@@ -2684,15 +2684,6 @@ out:
 	return bret;
 }
 
-static void strip_newline(char *p)
-{
-	size_t len = strlen(p);
-	if (len < 1)
-		return;
-	if (p[len-1] == '\n')
-		p[len-1] = '\0';
-}
-
 void mod_all_rdeps(struct lxc_container *c, bool inc)
 {
 	struct lxc_container *p;
@@ -2715,8 +2706,10 @@ void mod_all_rdeps(struct lxc_container *c, bool inc)
 			ERROR("badly formatted file %s", path);
 			goto out;
 		}
-		strip_newline(lxcpath);
-		strip_newline(lxcname);
+
+		remove_trailing_newlines(lxcpath);
+		remove_trailing_newlines(lxcname);
+
 		if ((p = lxc_container_new(lxcname, lxcpath)) == NULL) {
 			ERROR("Unable to find dependent container %s:%s",
 				lxcpath, lxcname);

src/lxc/lxcseccomp.h

@@ -27,23 +27,24 @@
 #include "conf.h"
 
 #ifdef HAVE_SECCOMP
-int lxc_seccomp_load(struct lxc_conf *conf);
-int lxc_read_seccomp_config(struct lxc_conf *conf);
-void lxc_seccomp_free(struct lxc_conf *conf);
+extern int lxc_seccomp_load(struct lxc_conf *conf);
+extern int lxc_read_seccomp_config(struct lxc_conf *conf);
+extern void lxc_seccomp_free(struct lxc_conf *conf);
 #else
-static inline int lxc_seccomp_load(struct lxc_conf *conf) {
+static inline int lxc_seccomp_load(struct lxc_conf *conf)
+{
 	return 0;
 }
 
-static inline int lxc_read_seccomp_config(struct lxc_conf *conf) {
+static inline int lxc_read_seccomp_config(struct lxc_conf *conf)
+{
 	return 0;
 }
 
-static inline void lxc_seccomp_free(struct lxc_conf *conf) {
-	if (conf->seccomp) {
-		free(conf->seccomp);
-		conf->seccomp = NULL;
-	}
+static inline void lxc_seccomp_free(struct lxc_conf *conf)
+{
+	free(conf->seccomp);
+	conf->seccomp = NULL;
 }
 #endif
 

src/lxc/seccomp.c

@@ -23,9 +23,9 @@
 
 #define _GNU_SOURCE
 #include <errno.h>
+#include <seccomp.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <seccomp.h>
 #include <sys/mount.h>
 #include <sys/utsname.h>
 
@@ -34,40 +34,60 @@
 #include "lxcseccomp.h"
 #include "utils.h"
 
+#ifdef __MIPSEL__
+#define MIPS_ARCH_O32 lxc_seccomp_arch_mipsel
+#define MIPS_ARCH_N64 lxc_seccomp_arch_mipsel64
+#else
+#define MIPS_ARCH_O32 lxc_seccomp_arch_mips
+#define MIPS_ARCH_N64 lxc_seccomp_arch_mips64
+#endif
+
 lxc_log_define(lxc_seccomp, lxc);
 
 static int parse_config_v1(FILE *f, struct lxc_conf *conf)
 {
-	char line[1024];
-	int ret;
+	int ret = 0;
+	size_t line_bufsz = 0;
+	char *line = NULL;
 
-	while (fgets(line, 1024, f)) {
+	while (getline(&line, &line_bufsz, f) != -1) {
 		int nr;
+
 		ret = sscanf(line, "%d", &nr);
 		if (ret != 1)
 			return -1;
-		ret = seccomp_rule_add(
+
 #if HAVE_SCMP_FILTER_CTX
-		    conf->seccomp_ctx,
+		ret = seccomp_rule_add(conf->seccomp_ctx, SCMP_ACT_ALLOW, nr, 0);
+#else
+		ret = seccomp_rule_add(SCMP_ACT_ALLOW, nr, 0);
 #endif
-		    SCMP_ACT_ALLOW, nr, 0);
 		if (ret < 0) {
 			ERROR("Failed loading allow rule for %d", nr);
-			return ret;
+			break;
 		}
 	}
-	return 0;
+	free(line);
+
+	return ret;
 }
 
 #if HAVE_DECL_SECCOMP_SYSCALL_RESOLVE_NAME_ARCH
-static void remove_trailing_newlines(char *l)
+static const char *get_action_name(uint32_t action)
 {
-	char *p = l;
+	/* The upper 16 bits indicate the type of the seccomp action. */
+	switch (action & 0xffff0000) {
+	case SCMP_ACT_KILL:
+		return "kill";
+	case SCMP_ACT_ALLOW:
+		return "allow";
+	case SCMP_ACT_TRAP:
+		return "trap";
+	case SCMP_ACT_ERRNO(0):
+		return "errno";
+	}
 
-	while (*p)
-		p++;
-	while (--p >= l && *p == '\n')
-		*p = '\0';
+	return "invalid action";
 }
 
 static uint32_t get_v2_default_action(char *line)
@@ -76,61 +96,57 @@ static uint32_t get_v2_default_action(char *line)
 
 	while (*line == ' ')
 		line++;
+
 	/* After 'whitelist' or 'blacklist' comes default behavior. */
-	if (strncmp(line, "kill", 4) == 0)
+	if (strncmp(line, "kill", 4) == 0) {
 		ret_action = SCMP_ACT_KILL;
-	else if (strncmp(line, "errno", 5) == 0) {
-		int e;
-		if (sscanf(line + 5, "%d", &e) != 1) {
-			ERROR("Bad errno value in %s", line);
+	} else if (strncmp(line, "errno", 5) == 0) {
+		int e, ret;
+
+		ret = sscanf(line + 5, "%d", &e);
+		if (ret != 1) {
+			ERROR("Failed to parse errno value from %s", line);
 			return -2;
 		}
+
 		ret_action = SCMP_ACT_ERRNO(e);
-	} else if (strncmp(line, "allow", 5) == 0)
+	} else if (strncmp(line, "allow", 5) == 0) {
 		ret_action = SCMP_ACT_ALLOW;
-	else if (strncmp(line, "trap", 4) == 0)
+	} else if (strncmp(line, "trap", 4) == 0) {
 		ret_action = SCMP_ACT_TRAP;
-	return ret_action;
-}
-
-static const char *get_action_name(uint32_t action)
-{
-	/* The upper 16 bits indicate the type of the seccomp action. */
-	switch(action & 0xffff0000){
-	case SCMP_ACT_KILL:
-		return "kill";
-	case SCMP_ACT_ALLOW:
-		return "allow";
-	case SCMP_ACT_TRAP:
-		return "trap";
-	case SCMP_ACT_ERRNO(0):
-		return "errno";
-	default:
-		return "invalid action";
 	}
+
+	return ret_action;
 }
 
 static uint32_t get_v2_action(char *line, uint32_t def_action)
 {
-	char *p = strchr(line, ' ');
+	char *p;
 	uint32_t ret;
 
+	p = strchr(line, ' ');
 	if (!p)
 		return def_action;
 	p++;
+
 	while (*p == ' ')
 		p++;
+
 	if (!*p || *p == '#')
 		return def_action;
+
 	ret = get_v2_default_action(p);
-	switch(ret) {
-	case -2: return -1;
-	case -1: return def_action;
-	default: return ret;
+	switch (ret) {
+	case -2:
+		return -1;
+	case -1:
+		return def_action;
 	}
+
+	return ret;
 }
 
-struct v2_rule_args {
+struct seccomp_v2_rule_args {
 	uint32_t index;
 	uint64_t value;
 	uint64_t mask;
@@ -140,7 +156,7 @@ struct v2_rule_args {
 struct seccomp_v2_rule {
 	uint32_t action;
 	uint32_t args_num;
-	struct v2_rule_args args_value[6];
+	struct seccomp_v2_rule_args args_value[6];
 };
 
 static enum scmp_compare parse_v2_rule_op(char *s)
@@ -163,7 +179,8 @@ static enum scmp_compare parse_v2_rule_op(char *s)
 	return _SCMP_CMP_MAX;
 }
 
-/* This function is used to parse the args string into the structure.
+/*
+ * This function is used to parse the args string into the structure.
  * args string format:[index,value,op,valueTwo] or [index,value,op]
  * index: the index for syscall arguments (type uint)
  * value: the value for syscall arguments (type uint64)
@@ -174,21 +191,21 @@ static enum scmp_compare parse_v2_rule_op(char *s)
  * valueTwo: the value for syscall arguments only used for mask eq (type uint64, optional)
  * Returns 0 on success, < 0 otherwise.
  */
-static int get_seccomp_arg_value(char *key, struct v2_rule_args *rule_args)
+static int get_seccomp_arg_value(char *key, struct seccomp_v2_rule_args *rule_args)
 {
 	int ret = 0;
-	uint64_t value = 0;
-	uint64_t mask = 0;
-	enum scmp_compare op = 0;
 	uint32_t index = 0;
-	char s[31] = {0}, v[24] = {0}, m[24] = {0};
+	uint64_t mask = 0, value = 0;
+	enum scmp_compare op = 0;
 	char *tmp = NULL;
+	char s[31] = {0}, v[24] = {0}, m[24] = {0};
 
 	tmp = strchr(key, '[');
 	if (!tmp) {
 		ERROR("Failed to interpret args");
 		return -1;
 	}
+
 	ret = sscanf(tmp, "[%i,%23[^,],%30[^0-9^,],%23[^,]", &index, v, s, m);
 	if ((ret != 3 && ret != 4) || index >= 6) {
 		ERROR("Failed to interpret args value");
@@ -201,7 +218,7 @@ static int get_seccomp_arg_value(char *key, struct v2_rule_args *rule_args)
 		return -1;
 	}
 
-	ret = lxc_safe_uint64(v, &mask);
+	ret = lxc_safe_uint64(m, &mask);
 	if (ret < 0) {
 		ERROR("Invalid argument mask");
 		return -1;
@@ -226,13 +243,11 @@ static int get_seccomp_arg_value(char *key, struct v2_rule_args *rule_args)
  * @rules	: output struct.
  * Returns 0 on success, < 0 otherwise.
  */
-static int parse_v2_rules(char *line, uint32_t def_action, struct seccomp_v2_rule *rules)
+static int parse_v2_rules(char *line, uint32_t def_action,
+			  struct seccomp_v2_rule *rules)
 {
-	int ret = 0 ;
-	int i = 0;
-	char *tmp = NULL;
-	char *key = NULL;
-	char *saveptr = NULL;
+	int i = 0, ret = -1;
+	char *key = NULL, *saveptr = NULL, *tmp = NULL;
 
 	tmp = strdup(line);
 	if (!tmp)
@@ -240,33 +255,29 @@ static int parse_v2_rules(char *line, uint32_t def_action, struct seccomp_v2_rul
 
 	/* read optional action which follows the syscall */
 	rules->action = get_v2_action(tmp, def_action);
-	if (rules->action == -1) {
-		ERROR("Failed to interpret action");
-		ret = -1;
-		goto out;
-	}
 
+	ret = 0;
 	rules->args_num = 0;
-	if (!strchr(tmp, '[')) {
-		ret = 0;
+	if (!strchr(tmp, '['))
 		goto out;
-	}
 
-	for ((key = strtok_r(tmp, "]", &saveptr)), i = 0; key && i < 6; (key = strtok_r(NULL, "]", &saveptr)), i++) {
+	ret = -1;
+	for ((key = strtok_r(tmp, "]", &saveptr)), i = 0; key && i < 6;
+	     (key = strtok_r(NULL, "]", &saveptr)), i++) {
 		ret = get_seccomp_arg_value(key, &rules->args_value[i]);
-		if (ret < 0) {
-			ret = -1;
+		if (ret < 0)
 			goto out;
-		}
+
 		rules->args_num++;
 	}
 
 	ret = 0;
+
 out:
 	free(tmp);
+
 	return ret;
 }
-
 #endif
 
 #if HAVE_DECL_SECCOMP_SYSCALL_RESOLVE_NAME_ARCH
@@ -291,14 +302,6 @@ enum lxc_hostarch_t {
 	lxc_seccomp_arch_unknown = 999,
 };
 
-#ifdef __MIPSEL__
-# define MIPS_ARCH_O32 lxc_seccomp_arch_mipsel
-# define MIPS_ARCH_N64 lxc_seccomp_arch_mipsel64
-#else
-# define MIPS_ARCH_O32 lxc_seccomp_arch_mips
-# define MIPS_ARCH_N64 lxc_seccomp_arch_mips64
-#endif
-
 int get_hostarch(void)
 {
 	struct utsname uts;
@@ -306,6 +309,7 @@ int get_hostarch(void)
 		SYSERROR("Failed to read host arch");
 		return -1;
 	}
+
 	if (strcmp(uts.machine, "i686") == 0)
 		return lxc_seccomp_arch_i386;
 	/* no x32 kernels */
@@ -327,59 +331,96 @@ int get_hostarch(void)
 		return MIPS_ARCH_O32;
 	else if (strncmp(uts.machine, "s390x", 5) == 0)
 		return lxc_seccomp_arch_s390x;
+
 	return lxc_seccomp_arch_unknown;
 }
 
-scmp_filter_ctx get_new_ctx(enum lxc_hostarch_t n_arch, uint32_t default_policy_action, bool *needs_merge)
+scmp_filter_ctx get_new_ctx(enum lxc_hostarch_t n_arch,
+			    uint32_t default_policy_action, bool *needs_merge)
 {
-	scmp_filter_ctx ctx;
 	int ret;
 	uint32_t arch;
+	scmp_filter_ctx ctx;
 
-	switch(n_arch) {
-	case lxc_seccomp_arch_i386: arch = SCMP_ARCH_X86; break;
-	case lxc_seccomp_arch_x32: arch = SCMP_ARCH_X32; break;
-	case lxc_seccomp_arch_amd64: arch = SCMP_ARCH_X86_64; break;
-	case lxc_seccomp_arch_arm: arch = SCMP_ARCH_ARM; break;
+	switch (n_arch) {
+	case lxc_seccomp_arch_i386:
+		arch = SCMP_ARCH_X86;
+		break;
+	case lxc_seccomp_arch_x32:
+		arch = SCMP_ARCH_X32;
+		break;
+	case lxc_seccomp_arch_amd64:
+		arch = SCMP_ARCH_X86_64;
+		break;
+	case lxc_seccomp_arch_arm:
+		arch = SCMP_ARCH_ARM;
+		break;
 #ifdef SCMP_ARCH_AARCH64
-	case lxc_seccomp_arch_arm64: arch = SCMP_ARCH_AARCH64; break;
+	case lxc_seccomp_arch_arm64:
+		arch = SCMP_ARCH_AARCH64;
+		break;
 #endif
 #ifdef SCMP_ARCH_PPC64LE
-	case lxc_seccomp_arch_ppc64le: arch = SCMP_ARCH_PPC64LE; break;
+	case lxc_seccomp_arch_ppc64le:
+		arch = SCMP_ARCH_PPC64LE;
+		break;
 #endif
 #ifdef SCMP_ARCH_PPC64
-	case lxc_seccomp_arch_ppc64: arch = SCMP_ARCH_PPC64; break;
+	case lxc_seccomp_arch_ppc64:
+		arch = SCMP_ARCH_PPC64;
+		break;
 #endif
 #ifdef SCMP_ARCH_PPC
-	case lxc_seccomp_arch_ppc: arch = SCMP_ARCH_PPC; break;
+	case lxc_seccomp_arch_ppc:
+		arch = SCMP_ARCH_PPC;
+		break;
 #endif
 #ifdef SCMP_ARCH_MIPS
-	case lxc_seccomp_arch_mips: arch = SCMP_ARCH_MIPS; break;
-	case lxc_seccomp_arch_mips64: arch = SCMP_ARCH_MIPS64; break;
-	case lxc_seccomp_arch_mips64n32: arch = SCMP_ARCH_MIPS64N32; break;
-	case lxc_seccomp_arch_mipsel: arch = SCMP_ARCH_MIPSEL; break;
-	case lxc_seccomp_arch_mipsel64: arch = SCMP_ARCH_MIPSEL64; break;
-	case lxc_seccomp_arch_mipsel64n32: arch = SCMP_ARCH_MIPSEL64N32; break;
+	case lxc_seccomp_arch_mips:
+		arch = SCMP_ARCH_MIPS;
+		break;
+	case lxc_seccomp_arch_mips64:
+		arch = SCMP_ARCH_MIPS64;
+		break;
+	case lxc_seccomp_arch_mips64n32:
+		arch = SCMP_ARCH_MIPS64N32;
+		break;
+	case lxc_seccomp_arch_mipsel:
+		arch = SCMP_ARCH_MIPSEL;
+		break;
+	case lxc_seccomp_arch_mipsel64:
+		arch = SCMP_ARCH_MIPSEL64;
+		break;
+	case lxc_seccomp_arch_mipsel64n32:
+		arch = SCMP_ARCH_MIPSEL64N32;
+		break;
 #endif
 #ifdef SCMP_ARCH_S390X
-	case lxc_seccomp_arch_s390x: arch = SCMP_ARCH_S390X; break;
+	case lxc_seccomp_arch_s390x:
+		arch = SCMP_ARCH_S390X;
+		break;
 #endif
-	default: return NULL;
+	default:
+		return NULL;
 	}
 
-	if ((ctx = seccomp_init(default_policy_action)) == NULL) {
+	ctx = seccomp_init(default_policy_action);
+	if (!ctx) {
 		ERROR("Error initializing seccomp context");
 		return NULL;
 	}
-	if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_NNP, 0)) {
-		ERROR("Failed to turn off no-new-privs");
+
+	ret = seccomp_attr_set(ctx, SCMP_FLTATR_CTL_NNP, 0);
+	if (ret < 0) {
+		ERROR("%s - Failed to turn off no-new-privs", strerror(-ret));
 		seccomp_release(ctx);
 		return NULL;
 	}
+
 #ifdef SCMP_FLTATR_ATL_TSKIP
-	if (seccomp_attr_set(ctx, SCMP_FLTATR_ATL_TSKIP, 1)) {
-		WARN("Failed to turn on seccomp nop-skip, continuing");
-	}
+	ret = seccomp_attr_set(ctx, SCMP_FLTATR_ATL_TSKIP, 1);
+	if (ret < 0)
+		WARN("%s - Failed to turn on seccomp nop-skip, continuing", strerror(-ret));
 #endif
 
 	ret = seccomp_arch_exist(ctx, arch);
@@ -387,7 +428,7 @@ scmp_filter_ctx get_new_ctx(enum lxc_hostarch_t n_arch, uint32_t default_policy_
 		if (ret != -EEXIST) {
 			ERROR("%s - Failed to determine whether arch %d is "
 			      "already present in the main seccomp context",
-			       strerror(-ret), (int)n_arch);
+			      strerror(-ret), (int)n_arch);
 			seccomp_release(ctx);
 			return NULL;
 		}
@@ -419,18 +460,14 @@ scmp_filter_ctx get_new_ctx(enum lxc_hostarch_t n_arch, uint32_t default_policy_
 }
 
 bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
-			struct seccomp_v2_rule *rule)
+			 struct seccomp_v2_rule *rule)
 {
-	int nr, ret, i;
+	int i, nr, ret;
 	struct scmp_arg_cmp arg_cmp[6];
 
-	memset(arg_cmp, 0 ,sizeof(arg_cmp));
-
 	ret = seccomp_arch_exist(ctx, arch);
 	if (arch && ret != 0) {
-		ERROR("BUG: Seccomp: rule and context arch do not match (arch "
-		      "%d): %s",
-		      arch, strerror(-ret));
+		ERROR("%s - Seccomp: rule and context arch do not match (arch %d)", strerror(-ret), arch);
 		return false;
 	}
 
@@ -440,49 +477,59 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
 		*p = '\0';
 
 	if (strncmp(line, "reject_force_umount", 19) == 0) {
-		INFO("Setting Seccomp rule to reject force umounts");
-		ret = seccomp_rule_add_exact(ctx, SCMP_ACT_ERRNO(EACCES), SCMP_SYS(umount2),
-				1, SCMP_A1(SCMP_CMP_MASKED_EQ , MNT_FORCE , MNT_FORCE ));
+		ret = seccomp_rule_add_exact(ctx, SCMP_ACT_ERRNO(EACCES),
+					     SCMP_SYS(umount2), 1,
+					     SCMP_A1(SCMP_CMP_MASKED_EQ, MNT_FORCE, MNT_FORCE));
 		if (ret < 0) {
-			ERROR("Failed (%d) loading rule to reject force "
-			      "umount: %s",
-			      ret, strerror(-ret));
+			ERROR("%s - Failed loading rule to reject force umount", strerror(-ret));
 			return false;
 		}
+
+		INFO("Set seccomp rule to reject force umounts");
 		return true;
 	}
 
 	nr = seccomp_syscall_resolve_name(line);
 	if (nr == __NR_SCMP_ERROR) {
-		WARN("Seccomp: failed to resolve syscall: %s", line);
+		WARN("Failed to resolve syscall \"%s\"", line);
 		WARN("This syscall will NOT be blacklisted");
 		return true;
 	}
+
 	if (nr < 0) {
-		WARN("Seccomp: got negative for syscall: %d: %s", nr, line);
+		WARN("Got negative return value %d for syscall \"%s\"", nr, line);
 		WARN("This syscall will NOT be blacklisted");
 		return true;
 	}
 
+	memset(&arg_cmp, 0, sizeof(arg_cmp));
 	for (i = 0; i < rule->args_num; i++) {
-		INFO("arg_cmp[%d]:SCMP_CMP(%u, %llu, %llu, %llu)", i,
-		      rule->args_value[i].index,
-		      (long long unsigned int)rule->args_value[i].op,
-		      (long long unsigned int)rule->args_value[i].mask,
-		      (long long unsigned int)rule->args_value[i].value);
+		INFO("arg_cmp[%d]: SCMP_CMP(%u, %llu, %llu, %llu)", i,
+		     rule->args_value[i].index,
+		     (long long unsigned int)rule->args_value[i].op,
+		     (long long unsigned int)rule->args_value[i].mask,
+		     (long long unsigned int)rule->args_value[i].value);
 
 		if (SCMP_CMP_MASKED_EQ == rule->args_value[i].op)
-			arg_cmp[i] = SCMP_CMP(rule->args_value[i].index, rule->args_value[i].op, rule->args_value[i].mask, rule->args_value[i].value);
+			arg_cmp[i] = SCMP_CMP(rule->args_value[i].index,
+					      rule->args_value[i].op,
+					      rule->args_value[i].mask,
+					      rule->args_value[i].value);
 		else
-			arg_cmp[i] = SCMP_CMP(rule->args_value[i].index, rule->args_value[i].op, rule->args_value[i].value);
+			arg_cmp[i] = SCMP_CMP(rule->args_value[i].index,
+					      rule->args_value[i].op,
+					      rule->args_value[i].value);
 	}
 
-	ret = seccomp_rule_add_exact_array(ctx, rule->action, nr, rule->args_num, arg_cmp);
+	ret = seccomp_rule_add_exact_array(ctx, rule->action, nr,
+					   rule->args_num, arg_cmp);
 	if (ret < 0) {
-		ERROR("Failed (%d) loading rule for %s (nr %d action %d(%s)): %s",
-		      ret, line, nr, rule->action, get_action_name(rule->action), strerror(-ret));
+		ERROR("%s - Failed loading rule for %s (nr %d action %d (%s))",
+		      strerror(-ret), line, nr, rule->action,
+		      get_action_name(rule->action));
 		return false;
 	}
+
 	return true;
 }
 
@@ -502,12 +549,13 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
  */
 static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 {
-	char *p;
 	int ret;
+	char *p;
+	enum lxc_hostarch_t cur_rule_arch, native_arch;
+	size_t line_bufsz = 0;
 	bool blacklist = false;
+	char *rule_line = NULL;
 	uint32_t default_policy_action = -1, default_rule_action = -1;
-	enum lxc_hostarch_t native_arch = get_hostarch(),
-			    cur_rule_arch = native_arch;
 	struct seccomp_v2_rule rule;
 	struct scmp_ctx_info {
 		uint32_t architectures[3];
@@ -518,11 +566,12 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 	if (strncmp(line, "blacklist", 9) == 0)
 		blacklist = true;
 	else if (strncmp(line, "whitelist", 9) != 0) {
-		ERROR("Bad seccomp policy style: %s", line);
+		ERROR("Bad seccomp policy style \"%s\"", line);
 		return -1;
 	}
 
-	if ((p = strchr(line, ' '))) {
+	p = strchr(line, ' ');
+	if (p) {
 		default_policy_action = get_v2_default_action(p + 1);
 		if (default_policy_action == -2)
 			return -1;
@@ -532,11 +581,13 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 	if (blacklist) {
 		if (default_policy_action == -1)
 			default_policy_action = SCMP_ACT_ALLOW;
+
 		if (default_rule_action == -1)
 			default_rule_action = SCMP_ACT_KILL;
 	} else {
 		if (default_policy_action == -1)
 			default_policy_action = SCMP_ACT_KILL;
+
 		if (default_rule_action == -1)
 			default_rule_action = SCMP_ACT_ALLOW;
 	}
@@ -545,6 +596,8 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 	ctx.architectures[0] = SCMP_ARCH_NATIVE;
 	ctx.architectures[1] = SCMP_ARCH_NATIVE;
 	ctx.architectures[2] = SCMP_ARCH_NATIVE;
+	native_arch = get_hostarch();
+	cur_rule_arch = native_arch;
 	if (native_arch == lxc_seccomp_arch_amd64) {
 		cur_rule_arch = lxc_seccomp_arch_all;
 
@@ -591,17 +644,17 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 		cur_rule_arch = lxc_seccomp_arch_all;
 
 		ctx.architectures[0] = SCMP_ARCH_ARM;
-		ctx.contexts[0] =
-		    get_new_ctx(lxc_seccomp_arch_arm, default_policy_action,
-				&ctx.needs_merge[0]);
+		ctx.contexts[0] = get_new_ctx(lxc_seccomp_arch_arm,
+					      default_policy_action,
+					      &ctx.needs_merge[0]);
 		if (!ctx.contexts[0])
 			goto bad;
 
 #ifdef SCMP_ARCH_AARCH64
 		ctx.architectures[2] = SCMP_ARCH_AARCH64;
-		ctx.contexts[2] =
-		    get_new_ctx(lxc_seccomp_arch_arm64, default_policy_action,
-				&ctx.needs_merge[2]);
+		ctx.contexts[2] = get_new_ctx(lxc_seccomp_arch_arm64,
+					      default_policy_action,
+					      &ctx.needs_merge[2]);
 		if (!ctx.contexts[2])
 			goto bad;
 #endif
@@ -662,25 +715,30 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 			ERROR("Error re-initializing Seccomp");
 			return -1;
 		}
-		if (seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_CTL_NNP, 0)) {
-			ERROR("Failed to turn off no-new-privs");
+
+		ret = seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_CTL_NNP, 0);
+		if (ret < 0) {
+			ERROR("%s - Failed to turn off no-new-privs", strerror(-ret));
 			return -1;
 		}
+
 #ifdef SCMP_FLTATR_ATL_TSKIP
-		if (seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_ATL_TSKIP, 1)) {
-			WARN("Failed to turn on seccomp nop-skip, continuing");
-		}
+		ret = seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_ATL_TSKIP, 1);
+		if (ret < 0)
+			WARN("%s - Failed to turn on seccomp nop-skip, continuing", strerror(-ret));
 #endif
 	}
 
-	while (fgets(line, 1024, f)) {
-
+	while (getline(&rule_line, &line_bufsz, f) != -1) {
 		if (line[0] == '#')
 			continue;
-		if (strlen(line) == 0)
+
+		if (line[0] == '\0')
 			continue;
+
 		remove_trailing_newlines(line);
-		INFO("processing: .%s", line);
+
+		INFO("Processing \"%s\"", line);
 		if (line[0] == '[') {
 			/* Read the architecture for next set of rules. */
 			if (strcmp(line, "[x86]") == 0 ||
@@ -690,6 +748,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_i386;
 			} else if (strcmp(line, "[x32]") == 0 ||
 				   strcmp(line, "[X32]") == 0) {
@@ -697,6 +756,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_x32;
 			} else if (strcmp(line, "[X86_64]") == 0 ||
 				   strcmp(line, "[x86_64]") == 0) {
@@ -704,6 +764,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_amd64;
 			} else if (strcmp(line, "[all]") == 0 ||
 				   strcmp(line, "[ALL]") == 0) {
@@ -717,6 +778,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_arm;
 			}
 #endif
@@ -727,6 +789,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_arm64;
 			}
 #endif
@@ -737,6 +800,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_ppc64le;
 			}
 #endif
@@ -747,6 +811,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_ppc64;
 			}
 #endif
@@ -758,6 +823,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_ppc;
 			}
 #endif
@@ -768,6 +834,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_mips64;
 			} else if (strcmp(line, "[mips64n32]") == 0 ||
 				   strcmp(line, "[MIPS64N32]") == 0) {
@@ -775,6 +842,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_mips64n32;
 			} else if (strcmp(line, "[mips]") == 0 ||
 				   strcmp(line, "[MIPS]") == 0) {
@@ -783,6 +851,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_mips;
 			} else if (strcmp(line, "[mipsel64]") == 0 ||
 				   strcmp(line, "[MIPSEL64]") == 0) {
@@ -790,6 +859,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_mipsel64;
 			} else if (strcmp(line, "[mipsel64n32]") == 0 ||
 				   strcmp(line, "[MIPSEL64N32]") == 0) {
@@ -797,6 +867,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_mipsel64n32;
 			} else if (strcmp(line, "[mipsel]") == 0 ||
 				   strcmp(line, "[MIPSEL]") == 0) {
@@ -805,6 +876,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_mipsel;
 			}
 #endif
@@ -815,11 +887,12 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 					cur_rule_arch = lxc_seccomp_arch_unknown;
 					continue;
 				}
+
 				cur_rule_arch = lxc_seccomp_arch_s390x;
-			}
 #endif
-			else
+			} else {
 				goto bad_arch;
+			}
 
 			continue;
 		}
@@ -839,6 +912,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 		if (!do_resolve_add_rule(SCMP_ARCH_NATIVE, line,
 					 conf->seccomp_ctx, &rule))
 			goto bad_rule;
+
 		INFO("Added native rule for arch %d for %s action %d(%s)",
 		     SCMP_ARCH_NATIVE, line, rule.action,
 		     get_action_name(rule.action));
@@ -847,6 +921,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 			if (!do_resolve_add_rule(ctx.architectures[0], line,
 						 ctx.contexts[0], &rule))
 				goto bad_rule;
+
 			INFO("Added compat rule for arch %d for %s action %d(%s)",
 			     ctx.architectures[0], line, rule.action,
 			     get_action_name(rule.action));
@@ -856,6 +931,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 			if (!do_resolve_add_rule(ctx.architectures[1], line,
 						 ctx.contexts[1], &rule))
 				goto bad_rule;
+
 			INFO("Added compat rule for arch %d for %s action %d(%s)",
 			     ctx.architectures[1], line, rule.action,
 			     get_action_name(rule.action));
@@ -865,6 +941,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 			if (!do_resolve_add_rule(ctx.architectures[2], line,
 						ctx.contexts[2], &rule))
 				goto bad_rule;
+
 			INFO("Added native rule for arch %d for %s action %d(%s)",
 			     ctx.architectures[2], line, rule.action,
 			     get_action_name(rule.action));
@@ -880,6 +957,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 				      "context into main context");
 				goto bad;
 			}
+
 			TRACE("Merged first compat seccomp context into main context");
 		} else {
 			seccomp_release(ctx.contexts[0]);
@@ -895,6 +973,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 				      "context into main context");
 				goto bad;
 			}
+
 			TRACE("Merged second compat seccomp context into main context");
 		} else {
 			seccomp_release(ctx.contexts[1]);
@@ -910,6 +989,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 				      "context into main context");
 				goto bad;
 			}
+
 			TRACE("Merged third compat seccomp context into main context");
 		} else {
 			seccomp_release(ctx.contexts[2]);
@@ -917,19 +997,25 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 		}
 	}
 
+	free(rule_line);
 	return 0;
 
 bad_arch:
-	ERROR("Unsupported arch: %s.", line);
+	ERROR("Unsupported architecture \"%s\"", line);
+
 bad_rule:
 bad:
 	if (ctx.contexts[0])
 		seccomp_release(ctx.contexts[0]);
+
 	if (ctx.contexts[1])
 		seccomp_release(ctx.contexts[1]);
+
 	if (ctx.contexts[2])
 		seccomp_release(ctx.contexts[2]);
 
+	free(rule_line);
+
 	return -1;
 }
 #else /* HAVE_DECL_SECCOMP_SYSCALL_RESOLVE_NAME_ARCH */
@@ -949,7 +1035,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
  */
 static int parse_config(FILE *f, struct lxc_conf *conf)
 {
-	char line[1024];
+	char line[MAXPATHLEN];
 	int ret, version;
 
 	ret = fscanf(f, "%d\n", &version);
@@ -957,10 +1043,12 @@ static int parse_config(FILE *f, struct lxc_conf *conf)
 		ERROR("Invalid version");
 		return -1;
 	}
-	if (!fgets(line, 1024, f)) {
+
+	if (!fgets(line, MAXPATHLEN, f)) {
 		ERROR("Invalid config file");
 		return -1;
 	}
+
 	if (version == 1 && !strstr(line, "whitelist")) {
 		ERROR("Only whitelist policy is supported");
 		return -1;
@@ -973,6 +1061,7 @@ static int parse_config(FILE *f, struct lxc_conf *conf)
 
 	if (version == 1)
 		return parse_config_v1(f, conf);
+
 	return parse_config_v2(f, line, conf);
 }
 
@@ -985,48 +1074,54 @@ static int parse_config(FILE *f, struct lxc_conf *conf)
  */
 static bool use_seccomp(void)
 {
-	FILE *f = fopen("/proc/self/status", "r");
-	char line[1024];
-	bool already_enabled = false;
-	bool found = false;
 	int ret, v;
+	FILE *f;
+	size_t line_bufsz = 0;
+	char *line = NULL;
+	bool already_enabled = false, found = false;
 
+	f = fopen("/proc/self/status", "r");
 	if (!f)
 		return true;
 
-	while (fgets(line, 1024, f)) {
+	while (getline(&line, &line_bufsz, f) != -1) {
 		if (strncmp(line, "Seccomp:", 8) == 0) {
 			found = true;
+
 			ret = sscanf(line + 8, "%d", &v);
 			if (ret == 1 && v != 0)
 				already_enabled = true;
+
 			break;
 		}
 	}
-
+	free(line);
 	fclose(f);
-	if (!found) { /* no Seccomp line, no seccomp in kernel */
+
+	if (!found) {
 		INFO("Seccomp is not enabled in the kernel");
 		return false;
 	}
-	if (already_enabled) { /* already seccomp-confined */
+
+	if (already_enabled) {
 		INFO("Already seccomp-confined, not loading new policy");
 		return false;
 	}
+
 	return true;
 }
 
 int lxc_read_seccomp_config(struct lxc_conf *conf)
 {
+	int check_seccomp_attr_set, ret;
 	FILE *f;
-	int ret;
-	int check_seccomp_attr_set;
 
 	if (!conf->seccomp)
 		return 0;
 
 	if (!use_seccomp())
 		return 0;
+
 #if HAVE_SCMP_FILTER_CTX
 	/* XXX for debug, pass in SCMP_ACT_TRAP */
 	conf->seccomp_ctx = seccomp_init(SCMP_ACT_KILL);
@@ -1039,7 +1134,7 @@ int lxc_read_seccomp_config(struct lxc_conf *conf)
 		return -1;
 	}
 
-/* turn off no-new-privs.  We don't want it in lxc, and it breaks
+/* turn off no-new-privs. We don't want it in lxc, and it breaks
  * with apparmor */
 #if HAVE_SCMP_FILTER_CTX
 	check_seccomp_attr_set = seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_CTL_NNP, 0);
@@ -1047,13 +1142,14 @@ int lxc_read_seccomp_config(struct lxc_conf *conf)
 	check_seccomp_attr_set = seccomp_attr_set(SCMP_FLTATR_CTL_NNP, 0);
 #endif
 	if (check_seccomp_attr_set) {
-		ERROR("Failed to turn off no-new-privs");
+		ERROR("%s - Failed to turn off no-new-privs", strerror(-check_seccomp_attr_set));
 		return -1;
 	}
 #ifdef SCMP_FLTATR_ATL_TSKIP
-	if (seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_ATL_TSKIP, 1)) {
-		WARN("Failed to turn on seccomp nop-skip, continuing");
-	}
+	check_seccomp_attr_set = seccomp_attr_set(conf->seccomp_ctx, SCMP_FLTATR_ATL_TSKIP, 1);
+	if (check_seccomp_attr_set < 0)
+		WARN("%s - Failed to turn on seccomp nop-skip, continuing",
+		     strerror(-check_seccomp_attr_set));
 #endif
 
 	f = fopen(conf->seccomp, "r");
@@ -1061,39 +1157,46 @@ int lxc_read_seccomp_config(struct lxc_conf *conf)
 		SYSERROR("Failed to open seccomp policy file %s", conf->seccomp);
 		return -1;
 	}
+
 	ret = parse_config(f, conf);
 	fclose(f);
+
 	return ret;
 }
 
 int lxc_seccomp_load(struct lxc_conf *conf)
 {
 	int ret;
+
 	if (!conf->seccomp)
 		return 0;
+
 	if (!use_seccomp())
 		return 0;
-	ret = seccomp_load(
+
 #if HAVE_SCMP_FILTER_CTX
-	    conf->seccomp_ctx
+	ret = seccomp_load(conf->seccomp_ctx);
+#else
+	ret = seccomp_load();
 #endif
-	    );
 	if (ret < 0) {
-		ERROR("Error loading the seccomp policy: %s", strerror(-ret));
+		ERROR("%s- Error loading the seccomp policy", strerror(-ret));
 		return -1;
 	}
 
 /* After load seccomp filter into the kernel successfully, export the current seccomp
  * filter to log file */
 #if HAVE_SCMP_FILTER_CTX
-	if ((lxc_log_get_level() <= LXC_LOG_LEVEL_TRACE || conf->loglevel <= LXC_LOG_LEVEL_TRACE) &&
+	if ((lxc_log_get_level() <= LXC_LOG_LEVEL_TRACE ||
+	     conf->loglevel <= LXC_LOG_LEVEL_TRACE) &&
 	    lxc_log_fd >= 0) {
 		ret = seccomp_export_pfc(conf->seccomp_ctx, lxc_log_fd);
 		/* Just give an warning when export error */
 		if (ret < 0)
-			WARN("Failed to export seccomp filter to log file: %s", strerror(-ret));
+			WARN("%s - Failed to export seccomp filter to log file", strerror(-ret));
 	}
 #endif
+
 	return 0;
 }
 
@@ -1101,6 +1204,7 @@ void lxc_seccomp_free(struct lxc_conf *conf)
 {
 	free(conf->seccomp);
 	conf->seccomp = NULL;
+
 #if HAVE_SCMP_FILTER_CTX
 	if (conf->seccomp_ctx) {
 		seccomp_release(conf->seccomp_ctx);

src/lxc/utils.c

@@ -2533,3 +2533,14 @@ int lxc_set_death_signal(int signal)
 
 	return 0;
 }
+
+void remove_trailing_newlines(char *l)
+{
+	char *p = l;
+
+	while (*p)
+		p++;
+
+	while (--p >= l && *p == '\n')
+		*p = '\0';
+}

src/lxc/utils.h

@@ -453,6 +453,7 @@ extern void lxc_free_array(void **array, lxc_free_fn element_free_fn);
 extern size_t lxc_array_len(void **array);
 
 extern void **lxc_append_null_to_array(void **array, size_t count);
+extern void remove_trailing_newlines(char *l);
 
 /* initialize rand with urandom */
 extern int randseed(bool);