From: Hengqi Chen <hengqi.chen@gmail.com>
To: Daniel Borkmann <daniel@iogearbox.net>,
Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
ast@kernel.org, andrii@kernel.org, luto@amacapital.net,
wad@chromium.org, alexyonghe@tencent.com
Subject: Re: [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation
Date: Mon, 23 Oct 2023 12:17:35 +0800 [thread overview]
Message-ID: <CAEyhmHSoGFRjpkoRQxRSaqe9U0ttbf51uKNbE6YkcaiGQc_2FA@mail.gmail.com> (raw)
In-Reply-To: <0df30939-1ba1-5703-58cc-54058fbb1df5@iogearbox.net>
On Mon, Oct 16, 2023 at 8:44 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> 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.
>
The intention is to use bpffs, so we need a bpf prog fd.
I prefer option 2, though it requires a bit of work.
That way, we could also write seccomp filter in eBPF language.
Kees, could you share your opinions ? If you have no objection,
I will continue this work.
> > + 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 {
> >
>
next prev parent reply other threads:[~2023-10-23 4:19 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-15 23:29 [PATCH v2 0/5] seccomp: Make seccomp filter reusable Hengqi Chen
2023-10-15 23:29 ` [PATCH v2 1/5] seccomp: Refactor filter copy/create for reuse Hengqi Chen
2023-10-15 23:29 ` [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation Hengqi Chen
2023-10-16 12:44 ` Daniel Borkmann
2023-10-23 4:17 ` Hengqi Chen [this message]
2023-10-22 23:42 ` kernel test robot
2023-10-15 23:29 ` [PATCH v2 3/5] seccomp: Introduce new flag SECCOMP_FILTER_FLAG_BPF_PROG_FD Hengqi Chen
2023-10-15 23:29 ` [PATCH v2 4/5] selftests/seccomp: Test seccomp filter load and attach Hengqi Chen
2023-10-15 23:29 ` [PATCH v2 5/5] selftests/bpf: Skip BPF_PROG_TYPE_SECCOMP-related tests Hengqi Chen
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=CAEyhmHSoGFRjpkoRQxRSaqe9U0ttbf51uKNbE6YkcaiGQc_2FA@mail.gmail.com \
--to=hengqi.chen@gmail.com \
--cc=alexyonghe@tencent.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--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.