Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220711083220.2175036-1-asavkov@redhat.com> <20220711083220.2175036-4-asavkov@redhat.com>
 <CAPhsuW7xTRpLf1kyj5ejH0fV_aHCMQjUwn-uhWeNytXedh4+TQ@mail.gmail.com>
 <CAADnVQ+ju04JAqyEbA_7oVj9uBAuL-fUP1FBr_OTygGf915RfQ@mail.gmail.com>
 <Ys7JL9Ih3546Eynf@wtfbox.lan> <CAADnVQ+6aN5nMwaTjoa9ddnT6rakgwb9oPhtdWSsgyaHP8kZ6Q@mail.gmail.com>
 <YtFjCSR8YiK8E13J@samus.usersys.redhat.com>
In-Reply-To: <YtFjCSR8YiK8E13J@samus.usersys.redhat.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Mon, 18 Jul 2022 14:01:02 -0700
Message-ID: <CAADnVQLjJK+9Jf+14WNp4O9q+s88eB-FF8pA_5TRziYVKoJxUQ@mail.gmail.com>
Subject: Re: [RFC PATCH bpf-next 3/4] bpf: add bpf_panic() helper
To:     Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Song Liu <song@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
        Networking <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        Andrea Arcangeli <aarcange@redhat.com>, dvacek@redhat.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jul 15, 2022 at 5:52 AM Artem Savkov <asavkov@redhat.com> wrote:
>
> On Wed, Jul 13, 2022 at 03:20:22PM -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 13, 2022 at 6:31 AM Artem Savkov <asavkov@redhat.com> wrote:
> > >
> > > On Tue, Jul 12, 2022 at 11:08:54AM -0700, Alexei Starovoitov wrote:
> > > > On Tue, Jul 12, 2022 at 10:53 AM Song Liu <song@kernel.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.
> > >
> > > Point taken, I'll remove sysctl knob in any further versions.
> > >
> > > > 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.
> > >
> > > The main goal is to get the memory dump, so crash_kexec() should be enough.
> > > However panic() is a bit more versatile and it's consequences are configurable
> > > to some extent. Are there any downsides to using it?
> >
> > versatile? In what sense? That it does a lot more than kexec?
> > That's a disadvantage.
> > We should provide bpf with minimal building blocks and let
> > bpf program decide what to do.
> > If dmesg (that is part of panic) is useful it should be its
> > own kfunc.
> > If halt is necessary -> separate kfunc as well.
> > reboot -> another kfunc.
> >
> > Also panic() is not guaranteed to do kexec and just
> > panic is not what you stated is the goal of the helper.
>
> Alright, if the aim is to provide the smallest building blocks then
> crash_kexec() is a better choice.
>
> > >
> > > > Why this destructive action cannot be delegated to user space?
> > >
> > > Going through userspace adds delays and makes it impossible to hit "exactly
> > > the right moment" thus making it unusable in most cases.
> >
> > What would be an example of that?
> > kexec is not instant either.
>
> With kexec at least the thread it got called in is in a proper state. I
> guess it is possible to achieve this by signalling userspace to do
> kexec/panic and then block the thread somehow but that won't work in a
> single-cpu case. Or am I missing something?

Something like this.
We can extend bpf_send_signal to send a signal to pid 1
or another user agent.
It's still not clear to me why you want that memory dump.