On 03/01/2019 04:51 AM, Jakub Kicinski wrote:
> On Fri, 1 Mar 2019 00:18:24 +0100, Daniel Borkmann wrote:
>> This work adds two new map creation flags BPF_F_RDONLY_PROG
>> and BPF_F_WRONLY_PROG in order to allow for read-only or
>> write-only BPF maps from a BPF program side.
>>
>> Today we have BPF_F_RDONLY and BPF_F_WRONLY, but this only
>> applies to system call side, meaning the BPF program has full
>> read/write access to the map as usual while bpf(2) calls with
>> map fd can either only read or write into the map depending
>> on the flags. BPF_F_RDONLY_PROG and BPF_F_WRONLY_PROG allows
>> for the exact opposite such that verifier is going to reject
>> program loads if write into a read-only map or a read into a
>> write-only map is detected.
>>
>> We've enabled this generic map extension to various non-special
>> maps holding normal user data: array, hash, lru, lpm, local
>> storage, queue and stack. Further map types could be followed
>> up in future depending on use-case. Main use case here is to
>> forbid writes into .rodata map values from verifier side.
>
> This will also enable optimizing the accesses on system with rich
> memory architecture :)
Nice! :)
>> Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
>> ---
>> include/linux/bpf.h | 18 ++++++++++++++++++
>> include/uapi/linux/bpf.h | 10 +++++++++-
>> kernel/bpf/arraymap.c | 2 +-
>> kernel/bpf/hashtab.c | 2 +-
>> kernel/bpf/local_storage.c | 2 +-
>> kernel/bpf/lpm_trie.c | 2 +-
>> kernel/bpf/queue_stack_maps.c | 3 +--
>> kernel/bpf/verifier.c | 30 +++++++++++++++++++++++++++++-
>> 8 files changed, 61 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index bdcc6e2a9977..3f74194dd4f6 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -427,6 +427,24 @@ struct bpf_array {
>> };
>> };
>>
>> +#define BPF_MAP_CAN_READ BIT(0)
>> +#define BPF_MAP_CAN_WRITE BIT(1)
>> +
>> +static inline u32 bpf_map_flags_to_cap(struct bpf_map *map)
>> +{
>> + u32 access_flags = map->map_flags & (BPF_F_RDONLY_PROG | BPF_F_WRONLY_PROG);
>> +
>> + /* Combination of BPF_F_RDONLY_PROG | BPF_F_WRONLY_PROG is
>> + * not possible.
>> + */
>
> minor nit: we do check that old RDONLY and WRONLY are not set at the
> same time, but here it's not done?
Good point indeed, I'll fix it in a v3 such that an invalid combination of
both, BPF_F_RDONLY_PROG | BPF_F_WRONLY_PROG, is rejected upon map creation.
>> + if (access_flags & BPF_F_RDONLY_PROG)
>> + return BPF_MAP_CAN_READ;
>> + else if (access_flags & BPF_F_WRONLY_PROG)
>> + return BPF_MAP_CAN_WRITE;
>> + else
>> + return BPF_MAP_CAN_READ | BPF_MAP_CAN_WRITE;
>> +}
Thanks,
Daniel
