netdev - Re: [PATCH RFC bpf-next 0/4] bpf: Add support for XDP programs in DEVMAPs

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: <0bb7b04c-60a9-525a-575d-944385851487@iogearbox.net>
Date:   Wed, 27 May 2020 01:36:11 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     David Ahern <dsahern@...il.com>, David Ahern <dsahern@...nel.org>,
        netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        john.fastabend@...il.com, ast@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, andriin@...com
Subject: Re: [PATCH RFC bpf-next 0/4] bpf: Add support for XDP programs in
 DEVMAPs

On 5/25/20 2:56 PM, Toke Høiland-Jørgensen wrote:
> Jesper Dangaard Brouer <brouer@...hat.com> writes:
>> On Mon, 25 May 2020 14:15:32 +0200
>> Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>> David Ahern <dsahern@...il.com> writes:
>>>> On 5/22/20 9:59 AM, Toke Høiland-Jørgensen wrote:
>>>>> David Ahern <dsahern@...nel.org> writes:
>>>>>    
>>>>>> Implementation of Daniel's proposal for allowing DEVMAP entries to be
>>>>>> a device index, program id pair. Daniel suggested an fd to specify the
>>>>>> program, but that seems odd to me that you insert the value as an fd, but
>>>>>> read it back as an id since the fd can be closed.
>>>>>
>>>>> While I can be sympathetic to the argument that it seems odd, every
>>>>> other API uses FD for insert and returns ID, so why make it different
>>>>> here? Also, the choice has privilege implications, since the CAP_BPF
>>>>> series explicitly makes going from ID->FD a more privileged operation
>>>>> than just querying the ID.

[...]

>> I sympathize with Ahern on this.  It seems very weird to insert/write
>> one value-type, but read another value-type.
> 
> Yeah, I do kinda agree that it's a bit weird. But it's what we do
> everywhere else, so I think consistency wins out here. There might be an
> argument that maps should be different (because they're conceptually a
> read/write data structure not a syscall return value). But again, we
> already have a map type that takes prog IDs, and that already does the
> rewriting, so doing it different here would be even weirder...

Sorry for the late reply. Agree, it would at least be consistent to what is done
in tail call maps, and the XDP netlink API where you have the fd->id in both cases.
Either way, quick glance over the patches, the direction of this RFC looks good to
me, better fit than the prior XDP egress approaches.

Thanks,
Daniel