netdev - Re: [RFC PATCH net] bpf: introduce BPF_F_ALLOW

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] [day] [month] [year] [list]

Message-ID: <589E781C.3040909@fb.com>
Date:   Fri, 10 Feb 2017 18:34:04 -0800
From:   Alexei Starovoitov <ast@...com>
To:     Andy Lutomirski <luto@...capital.net>
CC:     "David S . Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Ahern <dsa@...ulusnetworks.com>,
        Daniel Mack <daniel@...que.org>, Tejun Heo <tj@...nel.org>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH net] bpf: introduce BPF_F_ALLOW_OVERRIDE flag

On 2/10/17 1:38 PM, Andy Lutomirski wrote:
> On Thu, Feb 9, 2017 at 10:59 AM, Alexei Starovoitov <ast@...com> wrote:
>> If BPF_F_ALLOW_OVERRIDE flag is used in BPF_PROG_ATTACH command
>> to the given cgroup the descendent cgroup will be able to override
>> effective bpf program that was inherited from this cgroup.
>> By default it's not passed, therefore override is disallowed.
>>
>> Examples:
>> 1.
>> prog X attached to /A with default
>> prog Y fails to attach to /A/B and /A/B/C
>> Everything under /A runs prog X
>>
>> 2.
>> prog X attached to /A with ALLOW_OVERRIDE
>> prog Y attached to /A/B with default. Everything under /A/B runs prog Y
>
> I think that, for ease of future extension, Y should also need
> ALLOW_OVERRIDE.  Otherwise, when non-overridable hooks can stack,
> there could be confusion as to whether Y should override something or
> should stack.

I see. Fair enough. It's indeed easier for future extensions.

>> 2.
>> we can add another flag to reverse this call order too.
>> Instead of calling the progs from child to parent, do parent to child.
>
> I think the order should depend on the hook.  Hooks for
> process-initiated actions (egress, socket creation) should run
> innermost first and hooks for outside actions (ingress) should be
> outermost first.

There are use cases where both ingress and egress
would want both ordering. Like the monitoring would want to
see the bytes that app wants to send and it would want
to see the bytes that it's actually sending. So if something
in the middle wants to drop due to whatever conditions,
the monitoring needs to be the first and the last in the prog chain.
That's one of the use cases for 'attach_priority'.
Some high priority can be reserved for debugging and so on.

>> Andy,
>> does it all make sense?
>
> Yes with the caveat above.

great!

>> Do you still insist on submitting this patch officially?
>
> I'm not sure what you mean.

it's an RFC. In netdev we never apply rfc patches.

>> or you're ok keeping it overridable for now.
>
> I really think the default should change for 4.10.  People are going

fine. will respin with requested change.