netdev - Re: [PATCH net-next 6/6] bpf: show bpf programs

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: <fed27cb3-4509-8cd1-c0a3-6b7ea8cb5831@stressinduktion.org>
Date:   Thu, 27 Apr 2017 15:28:50 +0200
From:   Hannes Frederic Sowa <hannes@...essinduktion.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     netdev@...r.kernel.org, ast@...nel.org, daniel@...earbox.com,
        jbenc@...hat.com, aconole@...heb.org,
        Martin KaFai Lau <kafai@...com>
Subject: Re: [PATCH net-next 6/6] bpf: show bpf programs

On 26.04.2017 23:25, Alexei Starovoitov wrote:
> On Wed, Apr 26, 2017 at 08:24:19PM +0200, Hannes Frederic Sowa wrote:
>>  
>> +static const char *bpf_type_string(enum bpf_prog_type type)
>> +{
>> +	static const char *bpf_type_names[] = {
>> +#define X(type) #type
>> +		BPF_PROG_TYPES
>> +#undef X
>> +	};
>> +
>> +	if (type >= ARRAY_SIZE(bpf_type_names))
>> +		return "<unknown>";
>> +
>> +	return bpf_type_names[type];
>> +}
>> +
>>  static int ebpf_proc_show(struct seq_file *s, void *v)
>>  {
>> +	struct bpf_prog *prog;
>> +	struct bpf_prog_aux *aux;
>> +	char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
>> +
>>  	if (v == SEQ_START_TOKEN) {
>> -		seq_printf(s, "# tag\n");
>> +		seq_printf(s, "# tag\t\t\ttype\t\t\truntime\tcap\tmemlock\n");
>>  		return 0;
>>  	}
>>  
>> +	aux = v;
>> +	prog = aux->prog;
>> +
>> +	bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
>> +	seq_printf(s, "%s\t%s\t%s\t%s\t%llu\n", prog_tag,
>> +		   bpf_type_string(prog->type),
>> +		   prog->jited ? "jit" : "int",
>> +		   prog->priv_cap_sys_admin ? "priv" : "unpriv",
>> +		   prog->pages * 1ULL << PAGE_SHIFT);
> 
> As I said several times already I'm strongly against procfs
> style of exposing information about the programs.

I would not expand procfs interface to dump ebpf bytecode or jitcode,
albeit I would prefer a file system abstraction for that.

> I don't want this to become debugfs for bpf.

Right now it just prints a list of ebpf programs. You reject of where
things are going or do you already reject this particular patch?

> Maintaining the list of all loaded programs is fine
> and we need a way to iterate through them, but procfs
> is obviously not the interface to do that.
> Programs/maps are binary whereas any fs interface is text.

It should give admins only a high level overview of what is going on in
there system. I don't want o print any kind of opcodes there, just
giving people the possibility to see what is going on.

> It also doesn't scale with large number of programs/maps.
> I prefer Daniel's suggestion on adding 'get_next' like API.
> Also would be good if you can wait for Martin to finish his
> prog->handle/id patches. Then user space will be able
> to iterate through all the progs/maps and fetch all info about
> them through syscall in extensible way.
> And you wouldn't need to abuse kallsyms list for different purpose.

I don't buy the scalability argument:

What is the scalability issue with this O(1) approach on loading and
O(n) (but also without any locks held) on dumping? You can't do any
better. You can even dump several programs with one syscall instead of
slowly iterating over them.

Certainly I can wait with those patches until Martin presented his patches.

And regarding abusing the list, I just renamed it. If you think I abuse
something I can also add another list like bpf_prog_id patches do?

Bye bye,
Hannes