On Thu, May 19, 2022 at 08:15:45PM -0700, Song Liu wrote: > Also, remove set_vm_flush_reset_perms() from alloc_new_pack() and use > set_memory_[nx|rw] in bpf_prog_pack_free(). This is because > VM_FLUSH_RESET_PERMS does not work with huge pages yet. [1] > > [1] https://lore.kernel.org/bpf/aeeeaf0b7ec63fdba55d4834d2f524d8bf05b71b.camel@xxxxxxxxx/ > Suggested-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> > Signed-off-by: Song Liu <song@xxxxxxxxxx> > --- Rick, although VM_FLUSH_RESET_PERMS is rather new my concern here is we're essentially enabling sloppy users to grow without also addressing what if we have to take the leash back to support VM_FLUSH_RESET_PERMS properly? If the hack to support this on other architectures other than x86 is as simple as the one you in vm_remove_mappings() today: if (flush_reset && !IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) { set_memory_nx(addr, area->nr_pages); set_memory_rw(addr, area->nr_pages); } then I suppose this isn't a big deal. I'm just concerned here this being a slippery slope of sloppiness leading to something which we will regret later. My intution tells me this shouldn't be a big issue, but I just want to confirm. Luis > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index cacd8684c3c4..b64d91fcb0ba 100644 > @@ -949,6 +947,8 @@ static void bpf_prog_pack_free(struct bpf_binary_header *hdr) > > mutex_lock(&pack_mutex); > if (hdr->size > bpf_prog_pack_size) { > + set_memory_nx((unsigned long)hdr, hdr->size / PAGE_SIZE); > + set_memory_rw((unsigned long)hdr, hdr->size / PAGE_SIZE); > module_memfree(hdr); > goto out; > } > @@ -975,6 +975,8 @@ static void bpf_prog_pack_free(struct bpf_binary_header *hdr) > if (bitmap_find_next_zero_area(pack->bitmap, bpf_prog_chunk_count(), 0, > bpf_prog_chunk_count(), 0) == 0) { > list_del(&pack->list); > + set_memory_nx((unsigned long)pack->ptr, bpf_prog_pack_size / PAGE_SIZE); > + set_memory_rw((unsigned long)pack->ptr, bpf_prog_pack_size / PAGE_SIZE); > module_memfree(pack->ptr); > kfree(pack); > } > -- > 2.30.2 >
