Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF

From: Alexei Starovoitov <ast@fb.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Andy Lutomirski <luto@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	"LSM List" <linux-security-module@vger.kernel.org>,
	James Morris <jmorris@namei.org>, Jann Horn <jannh@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Network Development <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>, Kernel Team <Kernel-team@fb.com>,
	Linux API <linux-api@vger.kernel.org>
Subject: Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
Date: Tue, 1 Oct 2019 22:18:18 +0000	[thread overview]
Message-ID: <6e8b910c-a739-857d-4867-395bd369bc6a@fb.com> (raw)
In-Reply-To: <20191001181052.43c9fabb@gandalf.local.home>

On 10/1/19 3:10 PM, Steven Rostedt wrote:
> On Mon, 30 Sep 2019 18:22:28 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> tracefs is a file system, so clearly file based acls are much better fit
>> for all tracefs operations.
>> But that is not the case for ftrace overall.
>> bpf_trace_printk() calls trace_printk() that dumps into trace pipe.
>> Technically it's ftrace operation, but it cannot be controlled by tracefs
>> and by file permissions. That's the motivation to guard bpf_trace_printk()
>> usage from bpf program with CAP_TRACING.
> 
> BTW, I'd rather have bpf use an event that records a string than using
> trace printk itself.
> 
> Perhaps something like "bpf_print" event? That could be defined like:
> 
> TRACE_EVENT(bpf_print,
> 	TP_PROTO(const char *msg),
> 	TP_ARGS(msg),
> 	TP_STRUCT__entry(
> 		__string(msg, msg)
> 	),
> 	TP_fast_assign(
> 		__assign_str(msg, msg)
> 	),
> 	TP_printk("msg=%s", __get_str(msg))
> );
> 
> And then you can just format the string from the bpf_trace_printk()
> into msg, and then have:
> 
> 	trace_bpf_print(msg);

It's an interesting idea, but I don't think it can work.
Please see bpf_trace_printk implementation in kernel/trace/bpf_trace.c
It's a lot more than string printing.

> The user could then just enable the trace event from the file system. I
> could also work on making instances work like /tmp does (with the
> sticky bit) in creation. That way people with write access to the
> instances directory, can make their own buffers that they can use (and
> others can't access).

We tried instances in bcc in the past and eventually removed all the 
support. The overhead of instances is too high to be usable.

> 
> 
>>
>> Both 'trace' and 'trace_pipe' have quirky side effects.
>> Like opening 'trace' file will make all parallel trace_printk() to be ignored.
>> While reading 'trace_pipe' file will clear it.
>> The point that traditional 'read' and 'write' ACLs don't map as-is
>> to tracefs, so I would be careful categorizing things into
>> confidentiality vs integrity only based on access type.
> 
> What exactly is the bpf_trace_printk() used for? I may have other ideas
> that can help.

It's debugging of bpf programs. Same is what printk() is used for
by kernel developers.