netdev - Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)

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]

Date:	Thu, 18 Sep 2014 07:34:43 -0700
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Daniel Borkmann <dborkman@...hat.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andy Lutomirski <luto@...capital.net>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	Chema Gonzalez <chema@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Pablo Neira Ayuso <pablo@...filter.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kees Cook <keescook@...omium.org>,
	Linux API <linux-api@...r.kernel.org>,
	Network Development <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive
 verification log)

On Wed, Sep 17, 2014 at 11:44 PM, Daniel Borkmann <dborkman@...hat.com> wrote:
> On 09/18/2014 01:45 AM, Alexei Starovoitov wrote:
>>
>> On Wed, Sep 17, 2014 at 12:37 PM, Alexei Starovoitov <ast@...mgrid.com>
>> wrote:
>>>
>>>
>>>> Hm, thinking out loudly ... perhaps this could be made a library
>>>> problem.
>>>> Such that the library which wraps the syscall needs to be aware of a
>>>> marker where the initial version ends, and if the application doesn't
>>>> make use of any of the new features, it would just pass in the length up
>>>> to the marker as size attribute into the syscall. Similarly, if new
>>>> features are always added to the end of a structure and the library
>>>> truncates the overall-length after the last used member we might have
>>>> a chance to load something on older kernels, haven't tried that though.
>>>
>>>
>>> that's a 3rd option. I think it's cleaner than 2nd, since it solves it
>>> completely from user space.
>>> It can even be smarter than that. If this syscall wrapper library
>>> sees that newer features are used and it can workaround them:
>>> it can chop size and pass older fields into the older kernel
>>> and when it returns, do a workaround based on newer fields.
>>
>>
>> the more I think about 'new user space + old kernel' problem,
>> the more certain I am that kernel should not try to help
>> user space, since most of the time it's not going to be enough,
>> but additional code in kernel would need to be maintained.
>>
>> syscall commands and size of bpf_attr is the least of problems.
>> New map_type and prog_type will be added, new helper
>> functions will be available to programs.
>> One would think that md5 of uapi/linux/bpf.h would be
>> enough to say that user app is compatible... In reality,
>> it's not. The 'state pruning' verifier optimization I've talked
>> about will not change a single bit in bpf.h, but it will be
>> able to recognize more programs as safe.
>> A program developed on a new kernel with more
>> advanced verifier will load just fine on new kernel, but
>> this valid program will not load on old kernel, only because
>> verifier is not smart enough. Now we would need a version
>> of verifier exposed all the way to user space?!
>> imo that's too much. I think for eBPF infra kernel
>> should only guarantee backwards compatibility
>> (old user space must work with new kernel) and that's it.
>> That's what this patch is trying to do.
>> Thoughts?
>
>
> Sure, you will never get a full compatibility on that regard
> while backwards compatibility needs to be guaranteed on the
> other hand. I looked at perf_copy_attr() implementation and I
> think that we should mimic it in a very similar way as it
> exactly solves what we need.
>
> For example, it will return with -EINVAL for (size > PAGE_SIZE)
> and (size < PERF_ATTR_SIZE_VER0) where PAGE_SIZE has been chosen
> as an arbitrary hard upper limit where it is believed that it will
> never grow beyond that large limit in future.
>
> So this is a more loose constraint than what we currently do,
> that is, -EINVAL on (size > sizeof(attr)) where attr is the
> currently known size of a specific kernel. That would at least
> be a start, you won't be able to cover everything though, but
> it would allow to address the issue raised when running with
> a basic feature set.

you missed my point. We should not 'do a start', since it
doesn't help user space in the long run and only makes
kernel more complex.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html