All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tyler Hicks <tyhicks@canonical.com>
To: linux-kernel@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>, Paul Moore <paul@paul-moore.com>,
	Eric Paris <eparis@redhat.com>, Steve Grubb <sgrubb@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-audit@redhat.com, linux-security-module@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: [PATCH v2 1/4] seccomp: Separate read and write code for actions_logged sysctl
Date: Wed,  2 May 2018 15:53:17 +0000	[thread overview]
Message-ID: <1525276400-7161-2-git-send-email-tyhicks@canonical.com> (raw)
In-Reply-To: <1525276400-7161-1-git-send-email-tyhicks@canonical.com>

Break the read and write paths of the kernel.seccomp.actions_logged
sysctl into separate functions to maintain readability. An upcoming
change will need to audit writes, but not reads, of this sysctl which
would introduce too many conditional code paths on whether or not the
'write' parameter evaluates to true.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 kernel/seccomp.c | 60 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index dc77548..f4afe67 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1199,48 +1199,64 @@ static bool seccomp_actions_logged_from_names(u32 *actions_logged, char *names)
 	return true;
 }
 
-static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,
-					  void __user *buffer, size_t *lenp,
-					  loff_t *ppos)
+static int read_actions_logged(struct ctl_table *ro_table, void __user *buffer,
+			       size_t *lenp, loff_t *ppos)
 {
 	char names[sizeof(seccomp_actions_avail)];
 	struct ctl_table table;
+
+	memset(names, 0, sizeof(names));
+
+	if (!seccomp_names_from_actions_logged(names, sizeof(names),
+					       seccomp_actions_logged))
+		return -EINVAL;
+
+	table = *ro_table;
+	table.data = names;
+	table.maxlen = sizeof(names);
+	return proc_dostring(&table, 0, buffer, lenp, ppos);
+}
+
+static int write_actions_logged(struct ctl_table *ro_table, void __user *buffer,
+				size_t *lenp, loff_t *ppos)
+{
+	char names[sizeof(seccomp_actions_avail)];
+	struct ctl_table table;
+	u32 actions_logged;
 	int ret;
 
-	if (write && !capable(CAP_SYS_ADMIN))
+	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	memset(names, 0, sizeof(names));
 
-	if (!write) {
-		if (!seccomp_names_from_actions_logged(names, sizeof(names),
-						       seccomp_actions_logged))
-			return -EINVAL;
-	}
-
 	table = *ro_table;
 	table.data = names;
 	table.maxlen = sizeof(names);
-	ret = proc_dostring(&table, write, buffer, lenp, ppos);
+	ret = proc_dostring(&table, 1, buffer, lenp, ppos);
 	if (ret)
 		return ret;
 
-	if (write) {
-		u32 actions_logged;
-
-		if (!seccomp_actions_logged_from_names(&actions_logged,
-						       table.data))
-			return -EINVAL;
-
-		if (actions_logged & SECCOMP_LOG_ALLOW)
-			return -EINVAL;
+	if (!seccomp_actions_logged_from_names(&actions_logged, table.data))
+		return -EINVAL;
 
-		seccomp_actions_logged = actions_logged;
-	}
+	if (actions_logged & SECCOMP_LOG_ALLOW)
+		return -EINVAL;
 
+	seccomp_actions_logged = actions_logged;
 	return 0;
 }
 
+static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,
+					  void __user *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	if (write)
+		return write_actions_logged(ro_table, buffer, lenp, ppos);
+	else
+		return read_actions_logged(ro_table, buffer, lenp, ppos);
+}
+
 static struct ctl_path seccomp_sysctl_path[] = {
 	{ .procname = "kernel", },
 	{ .procname = "seccomp", },
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: Tyler Hicks <tyhicks@canonical.com>
To: linux-kernel@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>, Paul Moore <paul@paul-moore.com>,
	Eric Paris <eparis@redhat.com>, Steve Grubb <sgrubb@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-audit@redhat.com, linux-security-module@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: [PATCH v2 1/4] seccomp: Separate read and write code for actions_logged sysctl
Date: Wed,  2 May 2018 15:53:17 +0000	[thread overview]
Message-ID: <1525276400-7161-2-git-send-email-tyhicks@canonical.com> (raw)
In-Reply-To: <1525276400-7161-1-git-send-email-tyhicks@canonical.com>

Break the read and write paths of the kernel.seccomp.actions_logged
sysctl into separate functions to maintain readability. An upcoming
change will need to audit writes, but not reads, of this sysctl which
would introduce too many conditional code paths on whether or not the
'write' parameter evaluates to true.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 kernel/seccomp.c | 60 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index dc77548..f4afe67 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1199,48 +1199,64 @@ static bool seccomp_actions_logged_from_names(u32 *actions_logged, char *names)
 	return true;
 }
 
-static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,
-					  void __user *buffer, size_t *lenp,
-					  loff_t *ppos)
+static int read_actions_logged(struct ctl_table *ro_table, void __user *buffer,
+			       size_t *lenp, loff_t *ppos)
 {
 	char names[sizeof(seccomp_actions_avail)];
 	struct ctl_table table;
+
+	memset(names, 0, sizeof(names));
+
+	if (!seccomp_names_from_actions_logged(names, sizeof(names),
+					       seccomp_actions_logged))
+		return -EINVAL;
+
+	table = *ro_table;
+	table.data = names;
+	table.maxlen = sizeof(names);
+	return proc_dostring(&table, 0, buffer, lenp, ppos);
+}
+
+static int write_actions_logged(struct ctl_table *ro_table, void __user *buffer,
+				size_t *lenp, loff_t *ppos)
+{
+	char names[sizeof(seccomp_actions_avail)];
+	struct ctl_table table;
+	u32 actions_logged;
 	int ret;
 
-	if (write && !capable(CAP_SYS_ADMIN))
+	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	memset(names, 0, sizeof(names));
 
-	if (!write) {
-		if (!seccomp_names_from_actions_logged(names, sizeof(names),
-						       seccomp_actions_logged))
-			return -EINVAL;
-	}
-
 	table = *ro_table;
 	table.data = names;
 	table.maxlen = sizeof(names);
-	ret = proc_dostring(&table, write, buffer, lenp, ppos);
+	ret = proc_dostring(&table, 1, buffer, lenp, ppos);
 	if (ret)
 		return ret;
 
-	if (write) {
-		u32 actions_logged;
-
-		if (!seccomp_actions_logged_from_names(&actions_logged,
-						       table.data))
-			return -EINVAL;
-
-		if (actions_logged & SECCOMP_LOG_ALLOW)
-			return -EINVAL;
+	if (!seccomp_actions_logged_from_names(&actions_logged, table.data))
+		return -EINVAL;
 
-		seccomp_actions_logged = actions_logged;
-	}
+	if (actions_logged & SECCOMP_LOG_ALLOW)
+		return -EINVAL;
 
+	seccomp_actions_logged = actions_logged;
 	return 0;
 }
 
+static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,
+					  void __user *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	if (write)
+		return write_actions_logged(ro_table, buffer, lenp, ppos);
+	else
+		return read_actions_logged(ro_table, buffer, lenp, ppos);
+}
+
 static struct ctl_path seccomp_sysctl_path[] = {
 	{ .procname = "kernel", },
 	{ .procname = "seccomp", },
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: tyhicks@canonical.com (Tyler Hicks)
To: linux-security-module@vger.kernel.org
Subject: [PATCH v2 1/4] seccomp: Separate read and write code for actions_logged sysctl
Date: Wed,  2 May 2018 15:53:17 +0000	[thread overview]
Message-ID: <1525276400-7161-2-git-send-email-tyhicks@canonical.com> (raw)
In-Reply-To: <1525276400-7161-1-git-send-email-tyhicks@canonical.com>

Break the read and write paths of the kernel.seccomp.actions_logged
sysctl into separate functions to maintain readability. An upcoming
change will need to audit writes, but not reads, of this sysctl which
would introduce too many conditional code paths on whether or not the
'write' parameter evaluates to true.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 kernel/seccomp.c | 60 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index dc77548..f4afe67 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1199,48 +1199,64 @@ static bool seccomp_actions_logged_from_names(u32 *actions_logged, char *names)
 	return true;
 }
 
-static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,
-					  void __user *buffer, size_t *lenp,
-					  loff_t *ppos)
+static int read_actions_logged(struct ctl_table *ro_table, void __user *buffer,
+			       size_t *lenp, loff_t *ppos)
 {
 	char names[sizeof(seccomp_actions_avail)];
 	struct ctl_table table;
+
+	memset(names, 0, sizeof(names));
+
+	if (!seccomp_names_from_actions_logged(names, sizeof(names),
+					       seccomp_actions_logged))
+		return -EINVAL;
+
+	table = *ro_table;
+	table.data = names;
+	table.maxlen = sizeof(names);
+	return proc_dostring(&table, 0, buffer, lenp, ppos);
+}
+
+static int write_actions_logged(struct ctl_table *ro_table, void __user *buffer,
+				size_t *lenp, loff_t *ppos)
+{
+	char names[sizeof(seccomp_actions_avail)];
+	struct ctl_table table;
+	u32 actions_logged;
 	int ret;
 
-	if (write && !capable(CAP_SYS_ADMIN))
+	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	memset(names, 0, sizeof(names));
 
-	if (!write) {
-		if (!seccomp_names_from_actions_logged(names, sizeof(names),
-						       seccomp_actions_logged))
-			return -EINVAL;
-	}
-
 	table = *ro_table;
 	table.data = names;
 	table.maxlen = sizeof(names);
-	ret = proc_dostring(&table, write, buffer, lenp, ppos);
+	ret = proc_dostring(&table, 1, buffer, lenp, ppos);
 	if (ret)
 		return ret;
 
-	if (write) {
-		u32 actions_logged;
-
-		if (!seccomp_actions_logged_from_names(&actions_logged,
-						       table.data))
-			return -EINVAL;
-
-		if (actions_logged & SECCOMP_LOG_ALLOW)
-			return -EINVAL;
+	if (!seccomp_actions_logged_from_names(&actions_logged, table.data))
+		return -EINVAL;
 
-		seccomp_actions_logged = actions_logged;
-	}
+	if (actions_logged & SECCOMP_LOG_ALLOW)
+		return -EINVAL;
 
+	seccomp_actions_logged = actions_logged;
 	return 0;
 }
 
+static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,
+					  void __user *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	if (write)
+		return write_actions_logged(ro_table, buffer, lenp, ppos);
+	else
+		return read_actions_logged(ro_table, buffer, lenp, ppos);
+}
+
 static struct ctl_path seccomp_sysctl_path[] = {
 	{ .procname = "kernel", },
 	{ .procname = "seccomp", },
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2018-05-02 15:54 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-02 15:53 [PATCH v2 0/4] Better integrate seccomp logging and auditing Tyler Hicks
2018-05-02 15:53 ` Tyler Hicks
2018-05-02 15:53 ` Tyler Hicks
2018-05-02 15:53 ` Tyler Hicks [this message]
2018-05-02 15:53   ` [PATCH v2 1/4] seccomp: Separate read and write code for actions_logged sysctl Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 21:01   ` James Morris
2018-05-02 21:01     ` James Morris
2018-05-02 21:01     ` James Morris
2018-05-02 15:53 ` [PATCH v2 2/4] seccomp: Configurable separator for the actions_logged string Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 21:11   ` James Morris
2018-05-02 21:11     ` James Morris
2018-05-02 21:11     ` James Morris
2018-05-02 15:53 ` [PATCH v2 3/4] seccomp: Audit attempts to modify the actions_logged sysctl Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 18:18   ` Steve Grubb
2018-05-02 18:18     ` Steve Grubb
2018-05-02 18:18     ` Steve Grubb
2018-05-03 20:18     ` Paul Moore
2018-05-03 20:18       ` Paul Moore
2018-05-03 20:18       ` Paul Moore
2018-05-03 20:42       ` Steve Grubb
2018-05-03 20:42         ` Steve Grubb
2018-05-03 20:42         ` Steve Grubb
2018-05-03 20:42         ` Steve Grubb
2018-05-03 20:48         ` Paul Moore
2018-05-03 20:48           ` Paul Moore
2018-05-03 20:48           ` Paul Moore
2018-05-03 20:51           ` Tyler Hicks
2018-05-03 21:12             ` Steve Grubb
2018-05-03 21:12               ` Steve Grubb
2018-05-03 21:12               ` Steve Grubb
2018-05-03 22:36               ` Tyler Hicks
2018-05-03 23:18                 ` Steve Grubb
2018-05-03 23:18                   ` Steve Grubb
2018-05-03 23:18                   ` Steve Grubb
2018-05-02 21:17   ` James Morris
2018-05-02 21:17     ` James Morris
2018-05-02 21:17     ` James Morris
2018-05-02 15:53 ` [PATCH v2 4/4] seccomp: Don't special case audited processes when logging Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 15:53   ` Tyler Hicks
2018-05-02 16:57   ` Kees Cook
2018-05-02 16:57     ` Kees Cook
2018-05-02 16:57     ` Kees Cook
2018-05-03  2:32     ` Paul Moore
2018-05-03  2:32       ` Paul Moore
2018-05-03  2:32       ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1525276400-7161-2-git-send-email-tyhicks@canonical.com \
    --to=tyhicks@canonical.com \
    --cc=corbet@lwn.net \
    --cc=eparis@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=paul@paul-moore.com \
    --cc=sgrubb@redhat.com \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.