eBPF is often used for kernel debugging, and one of the widely used and
powerful debugging techniques is post-mortem debugging with a full memory dump.
Triggering a panic at exactly the right moment allows the user to get such a
dump and thus a better view at the system's state. Right now the only way to
do this in BPF is to signal userspace to trigger kexec/panic. This is
suboptimal as going through userspace requires context changes and adds
significant delays taking system further away from "the right moment". On a
single-cpu system the situation is even worse because BPF program won't even be
able to block the thread of interest.
This patchset tries to solve this problem by allowing properly marked tracing
bpf programs to call crash_kexec() kernel function.
This is a continuation of bpf_panic patchset with initial feedback taken into
account.
Changes from RFC:
- sysctl knob dropped
- using crash_kexec() instead of panic()
- using kfuncs instead of adding a new helper
Artem Savkov (4):
bpf: add BPF_F_DESTRUCTIVE flag for BPF_PROG_LOAD
bpf: add destructive kfunc set
selftests/bpf: add destructive kfunc tests
bpf: export crash_kexec() as destructive kfunc
include/linux/bpf.h | 1 +
include/linux/btf.h | 2 +
include/uapi/linux/bpf.h | 6 +++
kernel/bpf/syscall.c | 4 +-
kernel/bpf/verifier.c | 12 ++++++
kernel/kexec_core.c | 22 ++++++++++
net/bpf/test_run.c | 12 +++++-
tools/include/uapi/linux/bpf.h | 6 +++
.../selftests/bpf/prog_tests/kfunc_call.c | 41 +++++++++++++++++++
.../bpf/progs/kfunc_call_destructive.c | 14 +++++++
10 files changed, 118 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
--
2.35.3
Add a BPF_F_DESTRUCTIVE flag which will be required to be supplied
during BPF_PROG_LOAD for programs to be able to call destructive kfuncs.
Signed-off-by: Artem Savkov <[email protected]>
---
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 6 ++++++
kernel/bpf/syscall.c | 4 +++-
tools/include/uapi/linux/bpf.h | 6 ++++++
4 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a5bf00649995e..7b404d0b80aef 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1044,6 +1044,7 @@ struct bpf_prog_aux {
bool sleepable;
bool tail_call_reachable;
bool xdp_has_frags;
+ bool destructive;
/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
const struct btf_type *attach_func_proto;
/* function name for valid attach_btf_id */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 379e68fb866fc..ae81ad2e658dd 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1122,6 +1122,12 @@ enum bpf_link_type {
*/
#define BPF_F_XDP_HAS_FRAGS (1U << 5)
+/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
+ * will be able to perform destructive operations such as calling bpf_panic()
+ * helper.
+ */
+#define BPF_F_DESTRUCTIVE (1U << 6)
+
/* link_create.kprobe_multi.flags used in LINK_CREATE command for
* BPF_TRACE_KPROBE_MULTI attach type to create return probe.
*/
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 83c7136c5788d..86927521d0ea2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2467,7 +2467,8 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
BPF_F_TEST_STATE_FREQ |
BPF_F_SLEEPABLE |
BPF_F_TEST_RND_HI32 |
- BPF_F_XDP_HAS_FRAGS))
+ BPF_F_XDP_HAS_FRAGS |
+ BPF_F_DESTRUCTIVE))
return -EINVAL;
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
@@ -2554,6 +2555,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
prog->aux->offload_requested = !!attr->prog_ifindex;
prog->aux->sleepable = attr->prog_flags & BPF_F_SLEEPABLE;
prog->aux->xdp_has_frags = attr->prog_flags & BPF_F_XDP_HAS_FRAGS;
+ prog->aux->destructive = attr->prog_flags & BPF_F_DESTRUCTIVE;
err = security_bpf_prog_alloc(prog->aux);
if (err)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 379e68fb866fc..ae81ad2e658dd 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1122,6 +1122,12 @@ enum bpf_link_type {
*/
#define BPF_F_XDP_HAS_FRAGS (1U << 5)
+/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
+ * will be able to perform destructive operations such as calling bpf_panic()
+ * helper.
+ */
+#define BPF_F_DESTRUCTIVE (1U << 6)
+
/* link_create.kprobe_multi.flags used in LINK_CREATE command for
* BPF_TRACE_KPROBE_MULTI attach type to create return probe.
*/
--
2.35.3
Add BTF_KFUNC_TYPE_DESTRUCTIVE and a new destructive_set in struct
btf_kfunc_id_set. Functions in this set will require CAP_SYS_BOOT
capabilities and BPF_F_DESTRUCTIVE flag.
Signed-off-by: Artem Savkov <[email protected]>
---
include/linux/btf.h | 2 ++
kernel/bpf/verifier.c | 12 ++++++++++++
2 files changed, 14 insertions(+)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 1bfed7fa04287..6c58aa70e8125 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -18,6 +18,7 @@ enum btf_kfunc_type {
BTF_KFUNC_TYPE_RELEASE,
BTF_KFUNC_TYPE_RET_NULL,
BTF_KFUNC_TYPE_KPTR_ACQUIRE,
+ BTF_KFUNC_TYPE_DESTRUCTIVE,
BTF_KFUNC_TYPE_MAX,
};
@@ -37,6 +38,7 @@ struct btf_kfunc_id_set {
struct btf_id_set *release_set;
struct btf_id_set *ret_null_set;
struct btf_id_set *kptr_acquire_set;
+ struct btf_id_set *destructive_set;
};
struct btf_id_set *sets[BTF_KFUNC_TYPE_MAX];
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c59c3df0fea61..064035e70deac 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7582,6 +7582,18 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
return -EACCES;
}
+ if (btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
+ BTF_KFUNC_TYPE_DESTRUCTIVE, func_id)) {
+ if (!env->prog->aux->destructive) {
+ verbose(env, "destructive kfunc calls require BPF_F_DESTRUCTIVE flag\n");
+ return -EACCES;
+ }
+ if (!capable(CAP_SYS_BOOT)) {
+ verbose(env, "destructive kfunc calls require CAP_SYS_BOOT capabilities\n");
+ return -EACCES;
+ }
+ }
+
acq = btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
BTF_KFUNC_TYPE_ACQUIRE, func_id);
--
2.35.3
Add tests checking that programs calling destructive kfuncs can only do
so if they have BPF_F_DESTRUCTIVE flag set and CAP_SYS_BOOT
capabilities.
Signed-off-by: Artem Savkov <[email protected]>
---
net/bpf/test_run.c | 12 +++++-
.../selftests/bpf/prog_tests/kfunc_call.c | 41 +++++++++++++++++++
.../bpf/progs/kfunc_call_destructive.c | 14 +++++++
3 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 2ca96acbc50a0..dc267fa308f92 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -691,6 +691,10 @@ noinline void bpf_kfunc_call_test_mem_len_fail2(u64 *mem, int len)
{
}
+noinline void bpf_kfunc_call_test_destructive(void)
+{
+}
+
__diag_pop();
ALLOW_ERROR_INJECTION(bpf_modify_return_test, ERRNO);
@@ -714,6 +718,7 @@ BTF_ID(func, bpf_kfunc_call_test_fail3)
BTF_ID(func, bpf_kfunc_call_test_mem_len_pass1)
BTF_ID(func, bpf_kfunc_call_test_mem_len_fail1)
BTF_ID(func, bpf_kfunc_call_test_mem_len_fail2)
+BTF_ID(func, bpf_kfunc_call_test_destructive)
BTF_SET_END(test_sk_check_kfunc_ids)
BTF_SET_START(test_sk_acquire_kfunc_ids)
@@ -738,6 +743,10 @@ BTF_SET_START(test_sk_kptr_acquire_kfunc_ids)
BTF_ID(func, bpf_kfunc_call_test_kptr_get)
BTF_SET_END(test_sk_kptr_acquire_kfunc_ids)
+BTF_SET_START(test_sk_destructive_kfunc_ids)
+BTF_ID(func, bpf_kfunc_call_test_destructive)
+BTF_SET_END(test_sk_destructive_kfunc_ids)
+
static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
u32 size, u32 headroom, u32 tailroom)
{
@@ -1622,7 +1631,8 @@ static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
.acquire_set = &test_sk_acquire_kfunc_ids,
.release_set = &test_sk_release_kfunc_ids,
.ret_null_set = &test_sk_ret_null_kfunc_ids,
- .kptr_acquire_set = &test_sk_kptr_acquire_kfunc_ids
+ .kptr_acquire_set = &test_sk_kptr_acquire_kfunc_ids,
+ .destructive_set = &test_sk_destructive_kfunc_ids,
};
BTF_ID_LIST(bpf_prog_test_dtor_kfunc_ids)
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index c00eb974eb85e..1c2c4fdfba88b 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -5,6 +5,9 @@
#include "kfunc_call_test.lskel.h"
#include "kfunc_call_test_subprog.skel.h"
#include "kfunc_call_test_subprog.lskel.h"
+#include "kfunc_call_destructive.skel.h"
+
+#include "cap_helpers.h"
static void test_main(void)
{
@@ -86,6 +89,41 @@ static void test_subprog_lskel(void)
kfunc_call_test_subprog_lskel__destroy(skel);
}
+static int test_destructive_open_and_load(int set_flag)
+{
+ struct kfunc_call_destructive *skel;
+ int err;
+
+ skel = kfunc_call_destructive__open();
+ if (!ASSERT_OK_PTR(skel, "prog_open"))
+ return -1;
+
+ if (set_flag)
+ bpf_program__set_flags(skel->progs.kfunc_destructive_test,
+ bpf_program__flags(skel->progs.kfunc_destructive_test) | BPF_F_DESTRUCTIVE);
+
+ err = kfunc_call_destructive__load(skel);
+
+ kfunc_call_destructive__destroy(skel);
+
+ return err;
+}
+
+static void test_destructive(void)
+{
+ __u64 save_caps = 0;
+
+ ASSERT_EQ(test_destructive_open_and_load(0), -13, "no_flag_failure");
+ ASSERT_OK(test_destructive_open_and_load(1), "succesful_load");
+
+ if (!ASSERT_OK(cap_disable_effective(1ULL << CAP_SYS_BOOT, &save_caps), "drop_caps"))
+ return;
+
+ ASSERT_EQ(test_destructive_open_and_load(1), -13, "no_caps_failure");
+
+ cap_enable_effective(save_caps, NULL);
+}
+
void test_kfunc_call(void)
{
if (test__start_subtest("main"))
@@ -96,4 +134,7 @@ void test_kfunc_call(void)
if (test__start_subtest("subprog_lskel"))
test_subprog_lskel();
+
+ if (test__start_subtest("destructive"))
+ test_destructive();
}
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
new file mode 100644
index 0000000000000..767472bc5a97b
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+extern void bpf_kfunc_call_test_destructive(void) __ksym;
+
+SEC("tc")
+int kfunc_destructive_test(void)
+{
+ bpf_kfunc_call_test_destructive();
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.35.3
Allow properly marked bpf programs to call crash_kexec().
Signed-off-by: Artem Savkov <[email protected]>
---
kernel/kexec_core.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 4d34c78334ce4..a21fe8d326a8e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -39,6 +39,8 @@
#include <linux/hugetlb.h>
#include <linux/objtool.h>
#include <linux/kmsg_dump.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
#include <asm/page.h>
#include <asm/sections.h>
@@ -1238,3 +1240,23 @@ void __weak arch_kexec_protect_crashkres(void)
void __weak arch_kexec_unprotect_crashkres(void)
{}
+
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+BTF_SET_START(kexec_btf_ids)
+BTF_ID(func, crash_kexec)
+BTF_SET_END(kexec_btf_ids)
+
+static const struct btf_kfunc_id_set kexec_kfunc_set = {
+ .owner = THIS_MODULE,
+ .check_set = &kexec_btf_ids,
+ .destructive_set = &kexec_btf_ids,
+};
+
+static int __init crash_kfunc_init(void)
+{
+ register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &kexec_kfunc_set);
+ return 0;
+}
+
+subsys_initcall(crash_kfunc_init);
+#endif
--
2.35.3
On 7/20/22 1:46 PM, Artem Savkov wrote:
> eBPF is often used for kernel debugging, and one of the widely used and
> powerful debugging techniques is post-mortem debugging with a full memory dump.
> Triggering a panic at exactly the right moment allows the user to get such a
> dump and thus a better view at the system's state. Right now the only way to
> do this in BPF is to signal userspace to trigger kexec/panic. This is
> suboptimal as going through userspace requires context changes and adds
> significant delays taking system further away from "the right moment". On a
> single-cpu system the situation is even worse because BPF program won't even be
> able to block the thread of interest.
>
> This patchset tries to solve this problem by allowing properly marked tracing
> bpf programs to call crash_kexec() kernel function.
>
> This is a continuation of bpf_panic patchset with initial feedback taken into
> account.
>
> Changes from RFC:
> - sysctl knob dropped
> - using crash_kexec() instead of panic()
> - using kfuncs instead of adding a new helper
>
> Artem Savkov (4):
> bpf: add BPF_F_DESTRUCTIVE flag for BPF_PROG_LOAD
> bpf: add destructive kfunc set
> selftests/bpf: add destructive kfunc tests
> bpf: export crash_kexec() as destructive kfunc
First and second patch ccould be folded together into one. The selftest
should be last in series so that if people bisect the test won't fail due
to missing functionality. First one also has a stale comment wrt bpf_panic()
helper.
> include/linux/bpf.h | 1 +
> include/linux/btf.h | 2 +
> include/uapi/linux/bpf.h | 6 +++
> kernel/bpf/syscall.c | 4 +-
> kernel/bpf/verifier.c | 12 ++++++
> kernel/kexec_core.c | 22 ++++++++++
> net/bpf/test_run.c | 12 +++++-
> tools/include/uapi/linux/bpf.h | 6 +++
> .../selftests/bpf/prog_tests/kfunc_call.c | 41 +++++++++++++++++++
> .../bpf/progs/kfunc_call_destructive.c | 14 +++++++
> 10 files changed, 118 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
>
On Wed, Jul 20, 2022 at 4:47 AM Artem Savkov <[email protected]> wrote:
>
> +/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
> + * will be able to perform destructive operations such as calling bpf_panic()
> + * helper.
> + */
> +#define BPF_F_DESTRUCTIVE (1U << 6)
I don't understand what value this flag provides.
bpf prog won't be using kexec accidentally.
Requiring user space to also pass this flag seems pointless.
On Thu, Jul 21, 2022 at 07:02:07AM -0700, Alexei Starovoitov wrote:
> On Wed, Jul 20, 2022 at 4:47 AM Artem Savkov <[email protected]> wrote:
> >
> > +/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
> > + * will be able to perform destructive operations such as calling bpf_panic()
> > + * helper.
> > + */
> > +#define BPF_F_DESTRUCTIVE (1U << 6)
>
> I don't understand what value this flag provides.
>
> bpf prog won't be using kexec accidentally.
> Requiring user space to also pass this flag seems pointless.
bpf program likely won't. But I think it is not uncommon for people to
run bpftrace scripts they fetched off the internet to run them without
fully reading the code. So the idea was to provide intermediate tools
like that with a common way to confirm user's intent without
implementing their own guards around dangerous calls.
If that is not a good enough of a reason to add the flag I can drop it.
--
Artem
On Thu, Jul 21, 2022 at 9:18 PM Artem Savkov <[email protected]> wrote:
>
> On Thu, Jul 21, 2022 at 07:02:07AM -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 20, 2022 at 4:47 AM Artem Savkov <[email protected]> wrote:
> > >
> > > +/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
> > > + * will be able to perform destructive operations such as calling bpf_panic()
> > > + * helper.
> > > + */
> > > +#define BPF_F_DESTRUCTIVE (1U << 6)
> >
> > I don't understand what value this flag provides.
> >
> > bpf prog won't be using kexec accidentally.
> > Requiring user space to also pass this flag seems pointless.
>
> bpf program likely won't. But I think it is not uncommon for people to
> run bpftrace scripts they fetched off the internet to run them without
> fully reading the code. So the idea was to provide intermediate tools
> like that with a common way to confirm user's intent without
> implementing their own guards around dangerous calls.
> If that is not a good enough of a reason to add the flag I can drop it.
The intent makes sense, but bpftrace will set the flag silently.
Since bpftrace compiles the prog it knows what helpers are being
called, so it will have to pass that extra flag automatically anyway.
You can argue that bpftrace needs to require a mandatory cmdline flag
from users to run such scripts, but even if you convince the bpftrace
community to do that everybody else might just ignore that request.
Any tool (even libbpf) can scan the insns and provide flags.
Long ago we added the 'kern_version' field to the prog_load command.
The intent was to tie bpf prog with kernel version.
Soon enough people started querying the kernel and put that
version in there ignoring SEC("version") that bpf prog had.
It took years to clean that up.
BPF_F_DESTRUCTIVE flag looks similar to me.
Good intent, but unlikely to achieve the goal.
Do you have other ideas to achieve the goal:
'cannot run destructive prog by accident' ?
If we had an UI it would be a question 'are you sure? please type: yes'.
I hate to propose the following, since it will delay your patch
for a long time, but maybe we should only allow signed bpf programs
to be destructive?
On Thu, Jul 21, 2022 at 09:32:51PM -0700, Alexei Starovoitov wrote:
> On Thu, Jul 21, 2022 at 9:18 PM Artem Savkov <[email protected]> wrote:
> >
> > On Thu, Jul 21, 2022 at 07:02:07AM -0700, Alexei Starovoitov wrote:
> > > On Wed, Jul 20, 2022 at 4:47 AM Artem Savkov <[email protected]> wrote:
> > > >
> > > > +/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
> > > > + * will be able to perform destructive operations such as calling bpf_panic()
> > > > + * helper.
> > > > + */
> > > > +#define BPF_F_DESTRUCTIVE (1U << 6)
> > >
> > > I don't understand what value this flag provides.
> > >
> > > bpf prog won't be using kexec accidentally.
> > > Requiring user space to also pass this flag seems pointless.
> >
> > bpf program likely won't. But I think it is not uncommon for people to
> > run bpftrace scripts they fetched off the internet to run them without
> > fully reading the code. So the idea was to provide intermediate tools
> > like that with a common way to confirm user's intent without
> > implementing their own guards around dangerous calls.
> > If that is not a good enough of a reason to add the flag I can drop it.
>
> The intent makes sense, but bpftrace will set the flag silently.
> Since bpftrace compiles the prog it knows what helpers are being
> called, so it will have to pass that extra flag automatically anyway.
> You can argue that bpftrace needs to require a mandatory cmdline flag
> from users to run such scripts, but even if you convince the bpftrace
> community to do that everybody else might just ignore that request.
> Any tool (even libbpf) can scan the insns and provide flags.
>
> Long ago we added the 'kern_version' field to the prog_load command.
> The intent was to tie bpf prog with kernel version.
> Soon enough people started querying the kernel and put that
> version in there ignoring SEC("version") that bpf prog had.
> It took years to clean that up.
> BPF_F_DESTRUCTIVE flag looks similar to me.
> Good intent, but unlikely to achieve the goal.
Good point, I only thought of those who would like to use this, not the
ones who would try to work around it.
> Do you have other ideas to achieve the goal:
> 'cannot run destructive prog by accident' ?
>
> If we had an UI it would be a question 'are you sure? please type: yes'.
>
> I hate to propose the following, since it will delay your patch
> for a long time, but maybe we should only allow signed bpf programs
> to be destructive?
Anything I can think of is likely to be as easily defeated as the flag,
requirement for destructive programs to be signed is not. So I like the
idea. However I think that if bpf program signature checking is disabled
on the system then destructive programs should be able to run with just
CAP_SYS_BOOT. So maybe we can treat everything as this case until we
have the ability to sign bpf programs?
--
Artem
On Thu, Jul 21, 2022 at 09:32:51PM -0700, Alexei Starovoitov wrote:
> On Thu, Jul 21, 2022 at 9:18 PM Artem Savkov <[email protected]> wrote:
> >
> > On Thu, Jul 21, 2022 at 07:02:07AM -0700, Alexei Starovoitov wrote:
> > > On Wed, Jul 20, 2022 at 4:47 AM Artem Savkov <[email protected]> wrote:
> > > >
> > > > +/* If BPF_F_DESTRUCTIVE is used in BPF_PROG_LOAD command, the loaded program
> > > > + * will be able to perform destructive operations such as calling bpf_panic()
> > > > + * helper.
> > > > + */
> > > > +#define BPF_F_DESTRUCTIVE (1U << 6)
> > >
> > > I don't understand what value this flag provides.
> > >
> > > bpf prog won't be using kexec accidentally.
> > > Requiring user space to also pass this flag seems pointless.
> >
> > bpf program likely won't. But I think it is not uncommon for people to
> > run bpftrace scripts they fetched off the internet to run them without
> > fully reading the code. So the idea was to provide intermediate tools
> > like that with a common way to confirm user's intent without
> > implementing their own guards around dangerous calls.
> > If that is not a good enough of a reason to add the flag I can drop it.
>
> The intent makes sense, but bpftrace will set the flag silently.
> Since bpftrace compiles the prog it knows what helpers are being
> called, so it will have to pass that extra flag automatically anyway.
> You can argue that bpftrace needs to require a mandatory cmdline flag
> from users to run such scripts, but even if you convince the bpftrace
> community to do that everybody else might just ignore that request.
> Any tool (even libbpf) can scan the insns and provide flags.
FWIW I added --unsafe flag to bpftrace a while ago for
situations/helpers such as these. So this load flag would work OK for
bpftrace.
[...]
> Do you have other ideas to achieve the goal:
> 'cannot run destructive prog by accident' ?
>
> If we had an UI it would be a question 'are you sure? please type: yes'.
>
> I hate to propose the following, since it will delay your patch
> for a long time, but maybe we should only allow signed bpf programs
> to be destructive?
I don't have any opinion on the signing part but I do think it'd be nice
if there was some sort of opt-in mechanism. It wouldn't be very nice if
some arbitrary tracing tool panicked my machine. But I suppose tracing
programs could already do some significant damage by bpf_send_signal()ing
random processes.
Thanks,
Daniel