Bpftrace: [RFC] `--safe` mode

Created on 9 May 2019  Â·  8Comments  Â·  Source: iovisor/bpftrace

Context: I'm working on spinning up a new project based on bpftrace at FB. The goal is to provide self-service, on-demand tracing for any machine(s) in the fleet. To that end, bpftrace will be used as the backend mechanism. Users will be able to submit bpftrace scripts to be run.

Given the nature of self service, we must protect against accidental mistakes that can take down hosts. bpftrace, as designed, is generally pretty safe. However, there is currently one (AFAIK) escape hatch: system().

I've thought of two options: safe mode and linting. Obviously, I can lint against using system() in scripts before they're allowed to run. But ideally there'd be a --safe mode so I don't have to worry about keeping the linter up to date if new builtins are added. The safe mode would disable any unsafe builtins.

Any thoughts?

Most helpful comment

The number of probes limit sounds more like a tunable we'll add. I'd keep --unsafe for things that can directly change system state.

All 8 comments

Sure, makes sense since system() could run anything.

Do we want safe mode to be opt-in, or safe by default and opt-out (eg, an --unsafe mode?) If the latter, then we need to check that the following works:

#!/usr/local/bin/bpftrace --unsafe

...

So that my scripts that use system() still work. (We can deal with #!env later.)

Other things to consider: user writes (#28), and whether we get a signal helper. Both can be destructive and should be included by safe/unsafe.

I think if people were sending me bpftrace tools, I'd rather these unsafe things were disabled by default so I didn't run one by accident. So I'd vote for --unsafe instead of --safe.

Can someone think of a better name than unsafe? It might not be unsafe. :-)

@danobi If you want to send a PR for --unsafe for system(), then the sooner the better as it will change the API. Thanks!

This is a great idea. On the surface of it I think it makes sense for the user to have to opt-in to use anything that can have a side effect e.g., system() or register altering. Maybe, --i-accept-the-consequences :-) .

Can someone think of a better name than unsafe? It might not be unsafe. :-)

I don't think --unsafe would be the worst name. The concept exists in rust and we could ride off the semantic coattails, so to speak. I have no strong opinions, though.

ok, we can go with --unsafe.

This sounds cool! 😃

@danobi, some other stuff besides system that I think we could take into account for this use case:

  • Unlimited probes causing system slowdowns due to synchronisation without batching: #502
  • Deadlocks: #534 (Have some code to proactively test for these kind of things. Will push a PR as soon as I can.)

For the deadlocks, I see that as being silly to ever allow, even in unsafe mode. I wonder about unlimited uprobes, though. Should unsafe mode turn off limits (if we added them)?

I totally agree, just mentioned it here so we can proactively test this
better in case new kernel / user space functionality would introduce new
deadlocks :)

On Tue, 14 May 2019 at 19:43, Dan Xu notifications@github.com wrote:

For the deadlocks, I see that as being silly to ever allow, even in unsafe
mode. I wonder about unlimited uprobes, though. Should unsafe mode turn off
limits (if we added them)?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/iovisor/bpftrace/issues/613?email_source=notifications&email_token=AAHKFGBOZRHYZZYLGMO2DNLPVMB5VA5CNFSM4HL4XCHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVMNGRI#issuecomment-492360517,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHKFGESAJQPASLPEOMDYZ3PVMB5VANCNFSM4HL4XCHA
.

>

Fco. Javier Honduvilla Coto

The number of probes limit sounds more like a tunable we'll add. I'd keep --unsafe for things that can directly change system state.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brendangregg picture brendangregg  Â·  7Comments

danobi picture danobi  Â·  8Comments

tty5 picture tty5  Â·  3Comments

brendangregg picture brendangregg  Â·  4Comments

paulofelipefeitosa picture paulofelipefeitosa  Â·  6Comments