On Mon, Mar 11, 2019 at 4:06 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Mon, Mar 11, 2019 at 10:51:18PM +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. For read-only map case also some > > helpers are forbidden for programs that would alter the map > > state such as map deletion, update, etc. > > > > 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. > > I think WRONLY | WRONLY_PROG should be invalid combination? Just curious, what about the cases of special arrays (e.g., at least PROG_ARRAY). Is doing tail call a read from that map? > Since these attributes are set at creation time and cannot be changed, > nothing can ever read from it, so why write into it? > Similarly RDONLY | RDONLY_PROG is invalid too? > > Also looking at the next patch and 'lock' command... > May be it would be cleaner to do add WRONCE (from syscall) flag? > Then for .rodata the attrs will be RDONLY_PROG | WRONCE > and no 'lock' necessary. > WRONCE_PROG probably doesn't make sense. > Storing dangling task_struct pointer in the next patch doesn't look great. > The whole 'lock' concept feels useful, but in the context of implementing > .rodata it feels that WRONCE would be a better fit, > since libbpf won't be able to make a mistake and forget to 'lock'. > 'lock' syscall cmd can be confused with BPF_F_LOCK too. >
