linux-kernel - Re: [RFC PATCH bpf-next 3/4] bpf: add bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAADnVQ+ju04JAqyEbA_7oVj9uBAuL-fUP1FBr_OTygGf915RfQ@mail.gmail.com>
Date:   Tue, 12 Jul 2022 11:08:54 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Song Liu <song@...nel.org>
Cc:     Artem Savkov <asavkov@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Andrea Arcangeli <aarcange@...hat.com>
Subject: Re: [RFC PATCH bpf-next 3/4] bpf: add bpf_panic() helper

On Tue, Jul 12, 2022 at 10:53 AM Song Liu <song@...nel.org> wrote:
>
> >
> > +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.

+1. New syscal isn't warranted here.
Just CAP_SYS_BOOT would be enough here.

Also full blown panic() seems unnecessary.
If the motivation is to get a memory dump then crash_kexec() helper
would be more suitable.
If the goal is to reboot the system then the wrapper of sys_reboot()
is better.
Unfortunately the cover letter lacks these details.
Why this destructive action cannot be delegated to user space?

btw, we should avoid adding new uapi helpers in most cases.
Ideally all of them should be added as new kfunc-s, because they're
unstable and we can rip them out later if our judgement call
turns out to be problematic for whatever reason.