[lxc-devel] [lxc/master] Seccomp fixup part 2

[Blub on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 529 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180525/1f0e19f2/attachment.bin>
-------------- next part --------------
From f858dd50cff72855f4fe305c150eeb02387f8fb5 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 25 May 2018 11:44:42 +0200
Subject: [PATCH 1/3] seccomp: re-add action parse error handling

This can happen when the 'errno' action can't parse its
supplied number.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
Fixes: f67c94d00a0d ("seccomp: parse_v2_rules()")
---
 src/lxc/seccomp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 057e57082..dcf37447f 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -257,6 +257,11 @@ static int parse_v2_rules(char *line, uint32_t def_action,
 
 	/* 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;

From 9dbd8ff383804094dc4059cc052d56b504ad3121 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 25 May 2018 12:04:13 +0200
Subject: [PATCH 2/3] seccomp: refactor line handling of parse_config

Moving parse_config_v2 to use getline accidentally parsed
the wrong buffer. Since both _v1 and _v2 now use getline it
seems to be simpler to also use getline() for the first line
before entering the version specific parsers and pass along
the pointer and size so they can reuse them.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
Fixes: 9c3798eba41c ("seccomp: parse_config_v2()")
---
 src/lxc/seccomp.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index dcf37447f..44862983c 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -44,13 +44,11 @@
 
 lxc_log_define(lxc_seccomp, lxc);
 
-static int parse_config_v1(FILE *f, struct lxc_conf *conf)
+static int parse_config_v1(FILE *f, char *line, size_t *line_bufsz, struct lxc_conf *conf)
 {
 	int ret = 0;
-	size_t line_bufsz = 0;
-	char *line = NULL;
 
-	while (getline(&line, &line_bufsz, f) != -1) {
+	while (getline(&line, line_bufsz, f) != -1) {
 		int nr;
 
 		ret = sscanf(line, "%d", &nr);
@@ -554,14 +552,12 @@ bool do_resolve_add_rule(uint32_t arch, char *line, scmp_filter_ctx ctx,
  * write
  * close
  */
-static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
+static int parse_config_v2(FILE *f, char *line, size_t *line_bufsz, struct lxc_conf *conf)
 {
 	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;
 	struct seccomp_v2_rule rule;
 	struct scmp_ctx_info {
@@ -736,7 +732,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 #endif
 	}
 
-	while (getline(&rule_line, &line_bufsz, f) != -1) {
+	while (getline(&line, line_bufsz, f) != -1) {
 		if (line[0] == '#')
 			continue;
 
@@ -1004,7 +1000,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 		}
 	}
 
-	free(rule_line);
+	free(line);
 	return 0;
 
 bad_arch:
@@ -1021,7 +1017,7 @@ static int parse_config_v2(FILE *f, char *line, struct lxc_conf *conf)
 	if (ctx.contexts[2])
 		seccomp_release(ctx.contexts[2]);
 
-	free(rule_line);
+	free(line);
 
 	return -1;
 }
@@ -1042,7 +1038,8 @@ 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[MAXPATHLEN];
+	char *line = NULL;
+	size_t line_bufsz = 0;
 	int ret, version;
 
 	ret = fscanf(f, "%d\n", &version);
@@ -1051,25 +1048,29 @@ static int parse_config(FILE *f, struct lxc_conf *conf)
 		return -1;
 	}
 
-	if (!fgets(line, MAXPATHLEN, f)) {
+	if (getline(&line, &line_bufsz, f) == -1) {
 		ERROR("Invalid config file");
-		return -1;
+		goto bad_line;
 	}
 
 	if (version == 1 && !strstr(line, "whitelist")) {
 		ERROR("Only whitelist policy is supported");
-		return -1;
+		goto bad_line;
 	}
 
 	if (strstr(line, "debug")) {
 		ERROR("Debug not yet implemented");
-		return -1;
+		goto bad_line;
 	}
 
 	if (version == 1)
-		return parse_config_v1(f, conf);
+		return parse_config_v1(f, line, &line_bufsz, conf);
 
-	return parse_config_v2(f, line, conf);
+	return parse_config_v2(f, line, &line_bufsz, conf);
+
+bad_line:
+	free(line);
+	return -1;
 }
 
 /*

From 7474b5b33f15e7769608df2a36f7f86274028c01 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 25 May 2018 12:07:12 +0200
Subject: [PATCH 3/3] seccomp: error on unrecognized actions

Be more strict about unrecognized actions. Previously the
parser would happily accept lines with typos like:

  kexec_load errrno 1

(note the extra 'r')

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/seccomp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 44862983c..4ae981203 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -114,6 +114,9 @@ static uint32_t get_v2_default_action(char *line)
 		ret_action = SCMP_ACT_ALLOW;
 	} else if (strncmp(line, "trap", 4) == 0) {
 		ret_action = SCMP_ACT_TRAP;
+	} else if (line[0]) {
+		ERROR("Unrecognized seccomp action: %s", line);
+		return -2;
 	}
 
 	return ret_action;