netdev - Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

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:	Fri, 13 Mar 2015 18:03:28 +0100
From:	Daniel Borkmann <daniel@...earbox.net>
To:	Alexei Starovoitov <ast@...mgrid.com>, davem@...emloft.net
CC:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] ebpf: add prandom helper for packet sampling

On 03/13/2015 05:58 PM, Alexei Starovoitov wrote:
> On 3/13/15 6:22 AM, Daniel Borkmann wrote:
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 3fa1af8..4efc41f 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -165,6 +165,7 @@ enum bpf_func_id {
>>       BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(&map, &key) */
>>       BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
>>       BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
>> +    BPF_FUNC_random,      /* prandom() as u32 or u64 */
>
> can you rename it to BPF_FUNC_get_random_value ?
> I'd like names to be as descriptive as possible.

Ok, sure.

> User space can pick any name it likes anyway:
> static u64 (*my_foo_func)() = (void *) BPF_FUNC_get_random_value;
>
> Also it will make the following less obscure:
>  > +extern const struct bpf_func_proto bpf_random_proto;
>
> 'bpf_get_random_value_proto' looks better then 'bpf_random_proto' ;)
>
>> +static u64 bpf_random(u64 dw, u64 r2, u64 r3, u64 r4, u64 r5)
>> +{
>> +    return prandom_u32() | (dw ? ((u64)prandom_u32() << 32) : 0);
>
> can you make first argument to be a flag instead of bool ?
> 0 to return u32, 1 to return u64 and >=2 to be reserved and
> return -EINVAL?
> This way we can extend it later. For example 2 might return
> 8 truly random bytes from get_random_bytes_arch()

Yes, that's better.

>> +const struct bpf_func_proto bpf_random_proto = {
>> +    .func        = bpf_random,
>> +    .gpl_only    = false,
>
> non-gpl allowed?
> well I guess prandom_u32() is non-gpl either.

It can be used by anyone, yes. If there are strong opinions to make
this GPL-only, I don't mind, but I think it might be exaggerated.

Will respin.

Thanks,
Daniel
--
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