On Wed, Apr 15, 2020 at 1:04 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > > On Wed, Apr 15, 2020 at 9:07 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > > On Wed, Apr 15, 2020 at 6:59 AM Andrii Nakryiko > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > On Tue, Apr 14, 2020 at 3:50 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > > > > On Tue, Apr 14, 2020 at 9:46 PM Andrii Nakryiko > > > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > On Tue, Apr 14, 2020 at 9:07 AM Jann Horn <jannh@xxxxxxxxxx> wrote: > > > > > > On Fri, Apr 10, 2020 at 10:48 PM Andrii Nakryiko > > > > > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > > > On Fri, Apr 10, 2020 at 1:47 AM Jann Horn <jannh@xxxxxxxxxx> wrote: > > > > > > > > On Fri, Apr 10, 2020 at 1:33 AM Andrii Nakryiko > > > > > > > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > > > > > On Wed, Apr 8, 2020 at 12:42 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > > Hi! > > > > > > > > > > > > > > > > > > > > I saw that BPF allows root to create frozen maps, for which the > > > > > > > > > > verifier then assumes that they contain constant values. However, map > > > > > > > > > > freezing is pretty wobbly: > [...] > > > > > I'd say > > > > > the better way would be to implement immutable BPF maps from the time > > > > > they are created. E.g., at the time of creating map, you specify extra > > > > > flag BPF_F_IMMUTABLE and specify pointer to a blob of memory with > > > > > key/value pairs in it. > > > > > > > > It seems a bit hacky to me to add a new special interface for > > > > populating an immutable map. Wouldn't it make more sense to add a flag > > > > for "you can't use mmap on this map", or "I plan to freeze this map", > > > > or something like that, and keep the freezing API? > > > > > > "you can't use mmap on this map" is default behavior, unless you > > > specify BPF_F_MMAPABLE. > > > > Ah, right. > > > > > "I plan to freeze this map" could be added, > > > but how that would help existing users that freeze and mmap()? > > > Disallowing those now would be a breaking change. > > > Currently, libbpf is using freezing for .rodata variables, but it > > > doesn't mmap() before freezing. > > > > Okay, so it sounds like there are probably no actual users that use > > both BPF_F_MMAPABLE and freezing, and so we can just forbid that > > combination? That sounds great. > > kpsingh pointed out to me that bpf_object__load_skeleton() has code > specifically for mmap()ing BPF_F_RDONLY_PROG maps, so this might not > work... but perhaps we could make `BPF_F_RDONLY_PROG|BPF_F_MMAPABLE` > imply "you can only map with PROT_READ, not with PROT_WRITE"? > bpf_object__load_skeleton() only maps BPF_F_RDONLY_PROG maps with > PROT_READ. I think that's reasonable and given the intent of frozen read-only map, shouldn't break any reasonable use case. I'll send a patch soon.
