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  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, 27 May 2020 16:57:13 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     David Ahern <dsahern@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        David Ahern <dsahern@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        daniel@...earbox.net, john.fastabend@...il.com, ast@...nel.org,
        kafai@...com, songliubraving@...com, yhs@...com, andriin@...com
Subject: Re: [PATCH bpf-next 1/5] bpf: Handle 8-byte values in DEVMAP and DEVMAP_HASH

David Ahern <dsahern@...il.com> writes:

> On 5/27/20 4:26 AM, Jesper Dangaard Brouer wrote:
>>> @@ -108,9 +118,13 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
>>>  	u64 cost = 0;
>>>  	int err;
>>>  
>>> -	/* check sanity of attributes */
>>> +	/* check sanity of attributes. 2 value sizes supported:
>>> +	 * 4 bytes: ifindex
>>> +	 * 8 bytes: ifindex + prog fd
>>> +	 */
>>>  	if (attr->max_entries == 0 || attr->key_size != 4 ||
>>> -	    attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
>>> +	    (attr->value_size != 4 && attr->value_size != 8) ||
>> 
>> IMHO we really need to leverage BTF here, as I'm sure we need to do more
>> extensions, and this size matching will get more and more unmaintainable.
>> 
>> With BTF in place, dumping the map via bpftool, will also make the
>> fields "self-documenting".
>> 
>> I will try to implement something that uses BTF for this case (and cpumap).
>> 
>
> as mentioned in a past response, BTF does not make any fields special
> and this code should not assume it either.

Either way you're creating a contract where the kernel says "first four
bytes is the ifindex, second four bytes is the fd/id". BTF just makes
that explicit, and allows userspace to declare that it agrees this is
what the fields should mean. And gives us more flexibility when
extending the API later than just adding stuff at the end and looking at
the size...

> You need to know precisely which 4 bytes is the program fd that needs
> to be looked up, and that AFAIK is beyond the scope of BTF.

Exactly - BTF is a way for userspace to explicitly tell the kernel "I am
going to put the fd into these four bytes of the value field", instead
of the kernel implicitly assuming it's always bytes 5-8.

-Toke

Powered by blists - more mailing lists