2012-11-01 18:35:48

by Kees Cook

[permalink] [raw]
Subject: [PATCH v2] proc: add "Seccomp" to status

It is currently impossible to examine the state of seccomp for
a given process. While attaching with gdb and attempting "call
prctl(PR_GET_SECCOMP,...)" will work with some situations, it is not
reliable. If the process is in seccomp mode 1, this query will kill the
process (prctl not allowed), if the process is in mode 2 with prctl not
allowed, it will similarly be killed, and in weird cases, if prctl is
filtered to return errno 0, it can look like seccomp is disabled.

When reviewing the state of running processes, there should be a way to
externally examine the seccomp mode. ("Did this build of Chrome end up
using seccomp?" "Did my distro ship ssh with seccomp enabled?")

This adds the "Seccomp" line to /proc/$pid/status.

Signed-off-by: Kees Cook <[email protected]>
Reviewed-by: Cyrill Gorcunov <[email protected]>

---
v2:
- improve commit message, add documentation, as suggested by akpm.
---
Documentation/filesystems/proc.txt | 2 ++
fs/proc/array.c | 8 ++++++++
2 files changed, 10 insertions(+)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index a1793d6..557891d 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -181,6 +181,7 @@ read the file /proc/PID/status:
CapPrm: 0000000000000000
CapEff: 0000000000000000
CapBnd: ffffffffffffffff
+ Seccomp: 0
voluntary_ctxt_switches: 0
nonvoluntary_ctxt_switches: 1

@@ -237,6 +238,7 @@ Table 1-2: Contents of the status files (as of 2.6.30-rc7)
CapPrm bitmap of permitted capabilities
CapEff bitmap of effective capabilities
CapBnd bitmap of capabilities bounding set
+ Seccomp seccomp mode, like prctl(PR_GET_SECCOMP, ...)
Cpus_allowed mask of CPUs on which this process may run
Cpus_allowed_list Same as previous, but in "list format"
Mems_allowed mask of memory nodes allowed to this process
diff --git a/fs/proc/array.c b/fs/proc/array.c
index c1c207c..135d6ac 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -327,6 +327,13 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
render_cap_t(m, "CapBnd:\t", &cap_bset);
}

+static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
+{
+#ifdef CONFIG_SECCOMP
+ seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
+#endif
+}
+
static inline void task_context_switch_counts(struct seq_file *m,
struct task_struct *p)
{
@@ -360,6 +367,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
}
task_sig(m, task);
task_cap(m, task);
+ task_seccomp(m, task);
task_cpus_allowed(m, task);
cpuset_task_status_allowed(m, task);
task_context_switch_counts(m, task);
--
1.7.9.5


--
Kees Cook
Chrome OS Security


2012-11-03 11:51:38

by Vasily Kulikov

[permalink] [raw]
Subject: Re: [PATCH v2] proc: add "Seccomp" to status

On Thu, Nov 01, 2012 at 11:35 -0700, Kees Cook wrote:
> @@ -327,6 +327,13 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> render_cap_t(m, "CapBnd:\t", &cap_bset);
> }
>
> +static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> +{
> +#ifdef CONFIG_SECCOMP
> + seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
> +#endif

Hmm, probably it's better to always show this line, not only on
SECCOMP'ed kernel? If it is disabled just print "0". It will simplify
parsing of /proc/pid/status.

> +}
> +
> static inline void task_context_switch_counts(struct seq_file *m,
> struct task_struct *p)
> {

Thanks,

--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

2012-11-03 16:07:53

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2] proc: add "Seccomp" to status

On Sat, Nov 3, 2012 at 4:51 AM, Vasiliy Kulikov <[email protected]> wrote:
> On Thu, Nov 01, 2012 at 11:35 -0700, Kees Cook wrote:
>> @@ -327,6 +327,13 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
>> render_cap_t(m, "CapBnd:\t", &cap_bset);
>> }
>>
>> +static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
>> +{
>> +#ifdef CONFIG_SECCOMP
>> + seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
>> +#endif
>
> Hmm, probably it's better to always show this line, not only on
> SECCOMP'ed kernel? If it is disabled just print "0". It will simplify
> parsing of /proc/pid/status.

I disagree -- if the line is missing it means the kernel doesn't
support it, which is a different state from seccomp being inactive in
the process.

And since other things when not built into the kernel are similarly
missing from status, this is the consistent behavior for such
situations.

-Kees

--
Kees Cook
Chrome OS Security

2012-11-05 14:43:41

by Serge Hallyn

[permalink] [raw]
Subject: Re: [PATCH v2] proc: add "Seccomp" to status

Quoting Kees Cook ([email protected]):
> It is currently impossible to examine the state of seccomp for
> a given process. While attaching with gdb and attempting "call
> prctl(PR_GET_SECCOMP,...)" will work with some situations, it is not
> reliable. If the process is in seccomp mode 1, this query will kill the
> process (prctl not allowed), if the process is in mode 2 with prctl not
> allowed, it will similarly be killed, and in weird cases, if prctl is
> filtered to return errno 0, it can look like seccomp is disabled.
>
> When reviewing the state of running processes, there should be a way to
> externally examine the seccomp mode. ("Did this build of Chrome end up
> using seccomp?" "Did my distro ship ssh with seccomp enabled?")
>
> This adds the "Seccomp" line to /proc/$pid/status.
>
> Signed-off-by: Kees Cook <[email protected]>
> Reviewed-by: Cyrill Gorcunov <[email protected]>

Acked-by: Serge E. Hallyn <[email protected]>

One nit:

>
> ---
> v2:
> - improve commit message, add documentation, as suggested by akpm.
> ---
> Documentation/filesystems/proc.txt | 2 ++
> fs/proc/array.c | 8 ++++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
> index a1793d6..557891d 100644
> --- a/Documentation/filesystems/proc.txt
> +++ b/Documentation/filesystems/proc.txt
> @@ -181,6 +181,7 @@ read the file /proc/PID/status:
> CapPrm: 0000000000000000
> CapEff: 0000000000000000
> CapBnd: ffffffffffffffff
> + Seccomp: 0

Unless my mailer has messed with it, i notice that here there are 8 spaces,
whereas the code introduces a tab. Not sure if that might confuse some
people writing simple parsers.

> voluntary_ctxt_switches: 0
> nonvoluntary_ctxt_switches: 1
>
> @@ -237,6 +238,7 @@ Table 1-2: Contents of the status files (as of 2.6.30-rc7)
> CapPrm bitmap of permitted capabilities
> CapEff bitmap of effective capabilities
> CapBnd bitmap of capabilities bounding set
> + Seccomp seccomp mode, like prctl(PR_GET_SECCOMP, ...)
> Cpus_allowed mask of CPUs on which this process may run
> Cpus_allowed_list Same as previous, but in "list format"
> Mems_allowed mask of memory nodes allowed to this process
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index c1c207c..135d6ac 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -327,6 +327,13 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> render_cap_t(m, "CapBnd:\t", &cap_bset);
> }
>
> +static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> +{
> +#ifdef CONFIG_SECCOMP
> + seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
> +#endif
> +}
> +
> static inline void task_context_switch_counts(struct seq_file *m,
> struct task_struct *p)
> {
> @@ -360,6 +367,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
> }
> task_sig(m, task);
> task_cap(m, task);
> + task_seccomp(m, task);
> task_cpus_allowed(m, task);
> cpuset_task_status_allowed(m, task);
> task_context_switch_counts(m, task);
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security