netdev - Re: [PATCH 1/1] bpf: Allow bpf_map_lookup

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:   Wed, 05 Jun 2019 11:36:43 -0700
From:   "Jonathan Lemon" <jonathan.lemon@...il.com>
To:     "Björn Töpel" <bjorn.topel@...il.com>
Cc:     "Björn Töpel" <bjorn.topel@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        "Toke Høiland-Jørgensen" <toke@...hat.com>,
        "Jesper Dangaard Brouer" <brouer@...hat.com>,
        "Kernel Team" <kernel-team@...com>,
        Netdev <netdev@...r.kernel.org>,
        "Daniel Borkmann" <daniel@...earbox.net>,
        "Alexei Starovoitov" <ast@...nel.org>
Subject: Re: [PATCH 1/1] bpf: Allow bpf_map_lookup_elem() on an xskmap

On 5 Jun 2019, at 10:16, Björn Töpel wrote:

> On Wed, 5 Jun 2019 at 17:58, Jonathan Lemon <jonathan.lemon@...il.com> 
> wrote:
>>
>> Currently, the AF_XDP code uses a separate map in order to
>> determine if an xsk is bound to a queue.  Instead of doing this,
>> have bpf_map_lookup_elem() return a xdp_sock.
>>
>> Rearrange some xdp_sock members to eliminate structure holes.
>>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@...il.com>
>> ---
>>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 55bfc941d17a..f2d9d77b4b57 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -5680,6 +5680,46 @@ BPF_CALL_1(bpf_skb_ecn_set_ce, struct sk_buff 
>> *, skb)
>>         return INET_ECN_set_ce(skb);
>>  }
>>
>> +bool bpf_xdp_sock_is_valid_access(int off, int size, enum 
>> bpf_access_type type,
>> +                                 struct bpf_insn_access_aux *info)
>> +{
>> +       if (off < 0 || off >= offsetofend(struct bpf_xdp_sock, 
>> queue_id))
>> +               return false;
>> +
>> +       if (off % size != 0)
>> +               return false;
>> +
>> +       switch (off) {
>> +       default:
>> +               return size == sizeof(__u32);
>> +       }
>
> Hmm? Missing case or remove?

It looks odd because the default is for 32 bit accesses, and
separate case statements are added for overrides (64 bit, etc).
The style matches the rest of the file.

It compiles down to a single comparison, so no additional overhead.
-- 
Jonathan