netdev - Re: [PATCH bpf-next v2 2/7] bpf: add program side {rd,wr}only support

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, 1 Mar 2019 10:01:21 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     ast@...com, bpf@...r.kernel.org, netdev@...r.kernel.org,
        joe@...d.net.nz, john.fastabend@...il.com, tgraf@...g.ch,
        yhs@...com, andriin@...com, lmb@...udflare.com
Subject: Re: [PATCH bpf-next v2 2/7] bpf: add program side {rd,wr}only support

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@...earbox.net>
>> ---
>>  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