Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1703269 > unrolled thread

Re: [PATCH v5 1/6] seccomp: Sysctl to display available actions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 1/6] seccomp: Sysctl to display available actions Kees Cook <keescook@chromium.org> - 2017-08-03 18:40 +0200
    Re: [PATCH v5 1/6] seccomp: Sysctl to display available actions Tyler Hicks <tyhicks@canonical.com> - 2017-08-04 02:50 +0200

#1703269 — Re: [PATCH v5 1/6] seccomp: Sysctl to display available actions

On Fri, Jul 28, 2017 at 1:55 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
> This patch creates a read-only sysctl containing an ordered list of
> seccomp actions that the kernel supports. The ordering, from left to
> right, is the lowest action value (kill) to the highest action value
> (allow). Currently, a read of the sysctl file would return "kill trap
> errno trace allow". The contents of this sysctl file can be useful for
> userspace code as well as the system administrator.
>
> The path to the sysctl is:
>
>   /proc/sys/kernel/seccomp/actions_avail
>
> libseccomp and other userspace code can easily determine which actions
> the current kernel supports. The set of actions supported by the current
> kernel may be different than the set of action macros found in kernel
> headers that were installed where the userspace code was built.
>
> In addition, this sysctl will allow system administrators to know which
> actions are supported by the kernel and make it easier to configure
> exactly what seccomp logs through the audit subsystem. Support for this
> level of logging configuration will come in a future patch.
>
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>
> * Changes since v4:
>   - move device_initcall() into CONFIG_SYSCTL ifdef
>   - mark the seccomp_actions_avail string as const
>   - adjust for new reStructuredText format
>
>  Documentation/sysctl/kernel.txt                |  1 +
>  Documentation/userspace-api/seccomp_filter.rst | 16 ++++++++
>  kernel/seccomp.c                               | 51 ++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+)
>
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index bac23c1..995c42c 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -74,6 +74,7 @@ show up in /proc/sys/kernel:
>  - reboot-cmd                  [ SPARC only ]
>  - rtsig-max
>  - rtsig-nr
> +- seccomp/                    ==> Documentation/userspace-api/seccomp_filter.rst
>  - sem
>  - sem_next_id                [ sysv ipc ]
>  - sg-big-buff                 [ generic SCSI device (sg) ]
> diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
> index f71eb5e..35fc7cb 100644
> --- a/Documentation/userspace-api/seccomp_filter.rst
> +++ b/Documentation/userspace-api/seccomp_filter.rst
> @@ -169,7 +169,23 @@ The ``samples/seccomp/`` directory contains both an x86-specific example
>  and a more generic example of a higher level macro interface for BPF
>  program generation.
>
> +Sysctls
> +=======
> +
> +Seccomp's sysctl files can be found in the ``/proc/sys/kernel/seccomp/``
> +directory. Here's a description of each file in that directory:
> +
> +``actions_avail``:
> +       A read-only ordered list of seccomp return values (refer to the
> +       ``SECCOMP_RET_*`` macros above) in string form. The ordering, from
> +       left-to-right, is the least permissive return value to the most
> +       permissive return value.
>
> +       The list represents the set of seccomp return values supported
> +       by the kernel. A userspace program may use this list to
> +       determine if the actions found in the ``seccomp.h``, when the
> +       program was built, differs from the set of actions actually
> +       supported in the current running kernel.
>
>  Adding architecture support
>  ===========================
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 98b59b5..6bff068 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -17,11 +17,13 @@
>  #include <linux/audit.h>
>  #include <linux/compat.h>
>  #include <linux/coredump.h>
> +#include <linux/kmemleak.h>
>  #include <linux/sched.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/seccomp.h>
>  #include <linux/slab.h>
>  #include <linux/syscalls.h>
> +#include <linux/sysctl.h>
>
>  #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
>  #include <asm/syscall.h>
> @@ -922,3 +924,52 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
>         return ret;
>  }
>  #endif
> +
> +#ifdef CONFIG_SYSCTL
> +
> +/* Human readable action names for friendly sysctl interaction */
> +#define SECCOMP_RET_KILL_NAME          "kill"
> +#define SECCOMP_RET_TRAP_NAME          "trap"
> +#define SECCOMP_RET_ERRNO_NAME         "errno"
> +#define SECCOMP_RET_TRACE_NAME         "trace"
> +#define SECCOMP_RET_ALLOW_NAME         "allow"
> +
> +static const char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME      " "
> +                                           SECCOMP_RET_TRAP_NAME       " "
> +                                           SECCOMP_RET_ERRNO_NAME      " "
> +                                           SECCOMP_RET_TRACE_NAME      " "
> +                                           SECCOMP_RET_ALLOW_NAME;
> +
> +static struct ctl_path seccomp_sysctl_path[] = {
> +       { .procname = "kernel", },
> +       { .procname = "seccomp", },
> +       { }
> +};
> +
> +static struct ctl_table seccomp_sysctl_table[] = {
> +       {
> +               .procname       = "actions_avail",
> +               .data           = (void *) &seccomp_actions_avail,
> +               .maxlen         = sizeof(seccomp_actions_avail),
> +               .mode           = 0444,
> +               .proc_handler   = proc_dostring,
> +       },
> +       { }
> +};
> +
> +static int __init seccomp_sysctl_init(void)
> +{
> +       struct ctl_table_header *hdr;
> +
> +       hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table);
> +       if (!hdr)
> +               pr_warn("seccomp: sysctl registration failed\n");
> +       else
> +               kmemleak_not_leak(hdr);
> +
> +       return 0;
> +}
> +
> +device_initcall(seccomp_sysctl_init)
> +
> +#endif /* CONFIG_SYSCTL */

Looks good. And for the record, the BPF return values, while not
checked in seccomp_check_filter(), are part of ABI and the kernel will
behave differently for unexpected values. For example, an older kernel
encountering the future SECCOMP_RET_LOG will treat it as
SECCOMP_RET_KILL since it's missing from the switch statement in
__seccomp_filter().

A question about patch ordering: should the new seccomp action
introspection patch maybe follow this one, so they're together in the
series (they provide the same information)?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [next] | [standalone]

#1703567

[Multipart message — attachments visible in raw view] — view raw

On 08/03/2017 11:37 AM, Kees Cook wrote:
> On Fri, Jul 28, 2017 at 1:55 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> This patch creates a read-only sysctl containing an ordered list of
>> seccomp actions that the kernel supports. The ordering, from left to
>> right, is the lowest action value (kill) to the highest action value
>> (allow). Currently, a read of the sysctl file would return "kill trap
>> errno trace allow". The contents of this sysctl file can be useful for
>> userspace code as well as the system administrator.
>>
>> The path to the sysctl is:
>>
>>   /proc/sys/kernel/seccomp/actions_avail
>>
>> libseccomp and other userspace code can easily determine which actions
>> the current kernel supports. The set of actions supported by the current
>> kernel may be different than the set of action macros found in kernel
>> headers that were installed where the userspace code was built.
>>
>> In addition, this sysctl will allow system administrators to know which
>> actions are supported by the kernel and make it easier to configure
>> exactly what seccomp logs through the audit subsystem. Support for this
>> level of logging configuration will come in a future patch.
>>
>> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
>> ---
>>
>> * Changes since v4:
>>   - move device_initcall() into CONFIG_SYSCTL ifdef
>>   - mark the seccomp_actions_avail string as const
>>   - adjust for new reStructuredText format
>>
>>  Documentation/sysctl/kernel.txt                |  1 +
>>  Documentation/userspace-api/seccomp_filter.rst | 16 ++++++++
>>  kernel/seccomp.c                               | 51 ++++++++++++++++++++++++++
>>  3 files changed, 68 insertions(+)
>>
>> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
>> index bac23c1..995c42c 100644
>> --- a/Documentation/sysctl/kernel.txt
>> +++ b/Documentation/sysctl/kernel.txt
>> @@ -74,6 +74,7 @@ show up in /proc/sys/kernel:
>>  - reboot-cmd                  [ SPARC only ]
>>  - rtsig-max
>>  - rtsig-nr
>> +- seccomp/                    ==> Documentation/userspace-api/seccomp_filter.rst
>>  - sem
>>  - sem_next_id                [ sysv ipc ]
>>  - sg-big-buff                 [ generic SCSI device (sg) ]
>> diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
>> index f71eb5e..35fc7cb 100644
>> --- a/Documentation/userspace-api/seccomp_filter.rst
>> +++ b/Documentation/userspace-api/seccomp_filter.rst
>> @@ -169,7 +169,23 @@ The ``samples/seccomp/`` directory contains both an x86-specific example
>>  and a more generic example of a higher level macro interface for BPF
>>  program generation.
>>
>> +Sysctls
>> +=======
>> +
>> +Seccomp's sysctl files can be found in the ``/proc/sys/kernel/seccomp/``
>> +directory. Here's a description of each file in that directory:
>> +
>> +``actions_avail``:
>> +       A read-only ordered list of seccomp return values (refer to the
>> +       ``SECCOMP_RET_*`` macros above) in string form. The ordering, from
>> +       left-to-right, is the least permissive return value to the most
>> +       permissive return value.
>>
>> +       The list represents the set of seccomp return values supported
>> +       by the kernel. A userspace program may use this list to
>> +       determine if the actions found in the ``seccomp.h``, when the
>> +       program was built, differs from the set of actions actually
>> +       supported in the current running kernel.
>>
>>  Adding architecture support
>>  ===========================
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index 98b59b5..6bff068 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -17,11 +17,13 @@
>>  #include <linux/audit.h>
>>  #include <linux/compat.h>
>>  #include <linux/coredump.h>
>> +#include <linux/kmemleak.h>
>>  #include <linux/sched.h>
>>  #include <linux/sched/task_stack.h>
>>  #include <linux/seccomp.h>
>>  #include <linux/slab.h>
>>  #include <linux/syscalls.h>
>> +#include <linux/sysctl.h>
>>
>>  #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
>>  #include <asm/syscall.h>
>> @@ -922,3 +924,52 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
>>         return ret;
>>  }
>>  #endif
>> +
>> +#ifdef CONFIG_SYSCTL
>> +
>> +/* Human readable action names for friendly sysctl interaction */
>> +#define SECCOMP_RET_KILL_NAME          "kill"
>> +#define SECCOMP_RET_TRAP_NAME          "trap"
>> +#define SECCOMP_RET_ERRNO_NAME         "errno"
>> +#define SECCOMP_RET_TRACE_NAME         "trace"
>> +#define SECCOMP_RET_ALLOW_NAME         "allow"
>> +
>> +static const char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME      " "
>> +                                           SECCOMP_RET_TRAP_NAME       " "
>> +                                           SECCOMP_RET_ERRNO_NAME      " "
>> +                                           SECCOMP_RET_TRACE_NAME      " "
>> +                                           SECCOMP_RET_ALLOW_NAME;
>> +
>> +static struct ctl_path seccomp_sysctl_path[] = {
>> +       { .procname = "kernel", },
>> +       { .procname = "seccomp", },
>> +       { }
>> +};
>> +
>> +static struct ctl_table seccomp_sysctl_table[] = {
>> +       {
>> +               .procname       = "actions_avail",
>> +               .data           = (void *) &seccomp_actions_avail,
>> +               .maxlen         = sizeof(seccomp_actions_avail),
>> +               .mode           = 0444,
>> +               .proc_handler   = proc_dostring,
>> +       },
>> +       { }
>> +};
>> +
>> +static int __init seccomp_sysctl_init(void)
>> +{
>> +       struct ctl_table_header *hdr;
>> +
>> +       hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table);
>> +       if (!hdr)
>> +               pr_warn("seccomp: sysctl registration failed\n");
>> +       else
>> +               kmemleak_not_leak(hdr);
>> +
>> +       return 0;
>> +}
>> +
>> +device_initcall(seccomp_sysctl_init)
>> +
>> +#endif /* CONFIG_SYSCTL */
> 
> Looks good. And for the record, the BPF return values, while not
> checked in seccomp_check_filter(), are part of ABI and the kernel will
> behave differently for unexpected values. For example, an older kernel
> encountering the future SECCOMP_RET_LOG will treat it as
> SECCOMP_RET_KILL since it's missing from the switch statement in
> __seccomp_filter().
> 
> A question about patch ordering: should the new seccomp action
> introspection patch maybe follow this one, so they're together in the
> series (they provide the same information)?

That would be fine. I'll move it to patch #2.

Tyler

> 
> -Kees
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web