lkml.org 
[lkml]   [2023]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation
From
Date
On 10/16/23 1:29 AM, Hengqi Chen wrote:
> This patch adds a new operation named SECCOMP_LOAD_FILTER.
> It accepts a sock_fprog the same as SECCOMP_SET_MODE_FILTER
> but only performs the loading process. If succeed, return a
> new fd associated with the JITed BPF program (the filter).
> The filter can then be pinned to bpffs using the returned
> fd and reused for different processes. To distinguish the
> filter from other BPF progs, BPF_PROG_TYPE_SECCOMP is added.
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
> include/uapi/linux/bpf.h | 1 +
> include/uapi/linux/seccomp.h | 1 +
> kernel/seccomp.c | 43 ++++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 1 +
> 4 files changed, 46 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 7ba61b75bc0e..61c80ffb1724 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -995,6 +995,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_SK_LOOKUP,
> BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
> BPF_PROG_TYPE_NETFILTER,
> + BPF_PROG_TYPE_SECCOMP,

Please don't extend UAPI surface if this is not reachable/usable from user
space anyway.

> enum bpf_attach_type {
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index dbfc9b37fcae..ee2c83697810 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -16,6 +16,7 @@
> #define SECCOMP_SET_MODE_FILTER 1
> #define SECCOMP_GET_ACTION_AVAIL 2
> #define SECCOMP_GET_NOTIF_SIZES 3
> +#define SECCOMP_LOAD_FILTER 4
>
> /* Valid flags for SECCOMP_SET_MODE_FILTER */
> #define SECCOMP_FILTER_FLAG_TSYNC (1UL << 0)
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index faf84fc892eb..c9f6a19f7a4e 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -17,6 +17,7 @@
>
> #include <linux/refcount.h>
> #include <linux/audit.h>
> +#include <linux/bpf.h>
> #include <linux/compat.h>
> #include <linux/coredump.h>
> #include <linux/kmemleak.h>
> @@ -25,6 +26,7 @@
> #include <linux/sched.h>
> #include <linux/sched/task_stack.h>
> #include <linux/seccomp.h>
> +#include <linux/security.h>
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> #include <linux/sysctl.h>
> @@ -2032,12 +2034,48 @@ static long seccomp_set_mode_filter(unsigned int flags,
> seccomp_filter_free(prepared);
> return ret;
> }
> +
> +static long seccomp_load_filter(const char __user *filter)
> +{
> + struct sock_fprog fprog;
> + struct bpf_prog *prog;
> + int ret;
> +
> + ret = seccomp_copy_user_filter(filter, &fprog);
> + if (ret)
> + return ret;
> +
> + ret = seccomp_prepare_prog(&prog, &fprog);
> + if (ret)
> + return ret;
> +
> + ret = security_bpf_prog_alloc(prog->aux);
> + if (ret) {
> + bpf_prog_free(prog);
> + return ret;
> + }
> +
> + prog->aux->user = get_current_user();
> + atomic64_set(&prog->aux->refcnt, 1);
> + prog->type = BPF_PROG_TYPE_SECCOMP;
> +
> + ret = bpf_prog_new_fd(prog);
> + if (ret < 0)
> + bpf_prog_put(prog);

My bigger concern here is that bpf_prog_new_fd() is only used by eBPF (not cBPF).

Then you get an 'eBPF'-like fd back to user space which you can pass to various
other bpf(2) commands like BPF_OBJ_GET_INFO_BY_FD etc which all have the assumption
that this is a proper looking eBPF prog fd.

There may be breakage/undefined behavior in subtle ways.

I would suggest two potential alternatives :

1) Build a seccomp-specific fd via anon_inode_getfd() so that BPF side does not
confuse it with bpf_prog_fops and therefore does not recognize it in bpf(2)
as a prog fd.

2) Extend seccomp where proper eBPF could be supported.

If option 2) is not realistic (where you would get this out of the box), then I
think 1) could be however.

> + return ret;
> +}
> #else
> static inline long seccomp_set_mode_filter(unsigned int flags,
> const char __user *filter)
> {
> return -EINVAL;
> }
> +
> +static inline long seccomp_load_filter(const char __user *filter)
> +{
> + return -EINVAL;
> +}
> #endif
>
> static long seccomp_get_action_avail(const char __user *uaction)
> @@ -2099,6 +2137,11 @@ static long do_seccomp(unsigned int op, unsigned int flags,
> return -EINVAL;
>
> return seccomp_get_notif_sizes(uargs);
> + case SECCOMP_LOAD_FILTER:
> + if (flags != 0)
> + return -EINVAL;
> +
> + return seccomp_load_filter(uargs);
> default:
> return -EINVAL;
> }
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 7ba61b75bc0e..61c80ffb1724 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -995,6 +995,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_SK_LOOKUP,
> BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
> BPF_PROG_TYPE_NETFILTER,
> + BPF_PROG_TYPE_SECCOMP,
> };
>
> enum bpf_attach_type {
>

\
 
 \ /
  Last update: 2023-10-16 14:45    [W:0.133 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site