Re: [RFC PATCH bpf-next 3/4] bpf: add bpf_panic() helper
From: Song Liu
Date: Tue Jul 12 2022 - 13:53:46 EST
On Mon, Jul 11, 2022 at 1:32 AM Artem Savkov <asavkov@xxxxxxxxxx> wrote:
>
> Add a helper that will make the kernel panic immediately with specified
> message. Using this helper requires kernel.destructive_bpf_enabled sysctl
> to be enabled, BPF_F_DESTRUCTIVE flag to be supplied on program load as
> well as CAP_SYS_BOOT capabilities.
>
> Signed-off-by: Artem Savkov <asavkov@xxxxxxxxxx>
> ---
> include/linux/bpf.h | 1 +
> include/uapi/linux/bpf.h | 7 +++++++
> kernel/bpf/core.c | 1 +
> kernel/bpf/helpers.c | 13 +++++++++++++
> kernel/bpf/verifier.c | 7 +++++++
> kernel/trace/bpf_trace.c | 2 ++
> tools/include/uapi/linux/bpf.h | 7 +++++++
> 7 files changed, 38 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 43c008e3587a..77c20ba9ca8e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2339,6 +2339,7 @@ extern const struct bpf_func_proto bpf_strtol_proto;
> extern const struct bpf_func_proto bpf_strtoul_proto;
> extern const struct bpf_func_proto bpf_tcp_sock_proto;
> extern const struct bpf_func_proto bpf_jiffies64_proto;
> +extern const struct bpf_func_proto bpf_panic_proto;
> extern const struct bpf_func_proto bpf_get_ns_current_pid_tgid_proto;
> extern const struct bpf_func_proto bpf_event_output_data_proto;
> extern const struct bpf_func_proto bpf_ringbuf_output_proto;
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4423874b5da4..e2e2c4de44ee 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3927,6 +3927,12 @@ union bpf_attr {
> * Return
> * The 64 bit jiffies
> *
> + * void bpf_panic(const char *msg)
> + * Description
> + * Make the kernel panic immediately
> + * Return
> + * void
> + *
> * long bpf_read_branch_records(struct bpf_perf_event_data *ctx, void *buf, u32 size, u64 flags)
> * Description
> * For an eBPF program attached to a perf event, retrieve the
> @@ -5452,6 +5458,7 @@ union bpf_attr {
> FN(tcp_send_ack), \
> FN(send_signal_thread), \
> FN(jiffies64), \
> + FN(panic), \
> FN(read_branch_records), \
> FN(get_ns_current_pid_tgid), \
> FN(xdp_output), \
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index b5ffebcce6cc..0f333a0e85a5 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2649,6 +2649,7 @@ const struct bpf_func_proto bpf_map_lookup_percpu_elem_proto __weak;
> const struct bpf_func_proto bpf_spin_lock_proto __weak;
> const struct bpf_func_proto bpf_spin_unlock_proto __weak;
> const struct bpf_func_proto bpf_jiffies64_proto __weak;
> +const struct bpf_func_proto bpf_panic_proto __weak;
>
> const struct bpf_func_proto bpf_get_prandom_u32_proto __weak;
> const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak;
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a1c84d256f83..5cb90208a264 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -374,6 +374,19 @@ const struct bpf_func_proto bpf_jiffies64_proto = {
> .ret_type = RET_INTEGER,
> };
>
> +BPF_CALL_1(bpf_panic, const char *, msg)
> +{
> + panic(msg);
I think we should also check
capable(CAP_SYS_BOOT) && destructive_ebpf_enabled()
here. Or at least, destructive_ebpf_enabled(). Otherwise, we
may trigger panic after the sysctl is disabled.
In general, I don't think sysctl is a good API, as it is global, and
the user can easily forget to turn it back off. If possible, I would
rather avoid adding new BPF related sysctls.
Thanks,
Song
> + return 0;
> +}
> +
> +const struct bpf_func_proto bpf_panic_proto = {
> + .func = bpf_panic,
> + .gpl_only = false,
> + .ret_type = RET_VOID,
> + .arg1_type = ARG_PTR_TO_CONST_STR,
> +};
> +
> #ifdef CONFIG_CGROUPS
> BPF_CALL_0(bpf_get_current_cgroup_id)
> {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2859901ffbe3..f49c026917c5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7285,6 +7285,13 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> reg_type_str(env, regs[BPF_REG_1].type));
> return -EACCES;
> }
> + break;
> + case BPF_FUNC_panic:
> + struct bpf_prog_aux *aux = env->prog->aux;
> + if (!aux->destructive) {
> + verbose(env, "bpf_panic() calls require BPF_F_DESTRUCTIVE flag\n");
> + return -EACCES;
> + }
> }
>
> if (err)
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 4be976cf7d63..3ee888507795 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1304,6 +1304,8 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_find_vma_proto;
> case BPF_FUNC_trace_vprintk:
> return bpf_get_trace_vprintk_proto();
> + case BPF_FUNC_panic:
> + return capable(CAP_SYS_BOOT) && destructive_ebpf_enabled() ? &bpf_panic_proto : NULL;
> default:
> return bpf_base_func_proto(func_id);
> }
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 4423874b5da4..e2e2c4de44ee 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -3927,6 +3927,12 @@ union bpf_attr {
> * Return
> * The 64 bit jiffies
> *
> + * void bpf_panic(const char *msg)
> + * Description
> + * Make the kernel panic immediately
> + * Return
> + * void
> + *
> * long bpf_read_branch_records(struct bpf_perf_event_data *ctx, void *buf, u32 size, u64 flags)
> * Description
> * For an eBPF program attached to a perf event, retrieve the
> @@ -5452,6 +5458,7 @@ union bpf_attr {
> FN(tcp_send_ack), \
> FN(send_signal_thread), \
> FN(jiffies64), \
> + FN(panic), \
> FN(read_branch_records), \
> FN(get_ns_current_pid_tgid), \
> FN(xdp_output), \
> --
> 2.35.3
>