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 23:44:38 +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 06:03 PM, Daniel Borkmann wrote:
> On 03/13/2015 05:58 PM, Alexei Starovoitov wrote:
...
>>> +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()

Hmm, just got back to this thinking about it a bit more. I don't
like that we would return with a negative error here. An eBPF
application would then need to check for errors as well to make
sure, and the signature returns max u64. So _theoretically_,
-EINVAL could also come as a result from prandom() albeit unlikely.
That would make the interface a bit ugly, imho.

I think the interface should be as simple as possible for
obtaining a random number. I think that if we would want to
support something like get_random_bytes_arch(), it would be
better to do so in a new interface, where the name also more
clearly indicates where the random data comes from.

That said, I'd rather prefer to leave the code as is, but go
with the rename, maybe even BPF_FUNC_get_prandom_val so it
indicates it's pseudo-random as opposed to, say, something like
BPF_FUNC_get_srandom_val if we need that one day.

Cheers,
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