On Thu, Jul 21, 2022 at 09:32:51PM -0700, Alexei Starovoitov wrote: > On Thu, Jul 21, 2022 at 9:18 PM Artem Savkov <asavkov@xxxxxxxxxx> 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 <asavkov@xxxxxxxxxx> 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
