Le 19/02/2024 à 11:19, Simon Horman a écrit : > On Sat, Feb 17, 2024 at 11:24:07AM +0100, Christophe Leroy wrote: >> arch_protect_bpf_trampoline() and alloc_new_pack() call >> set_memory_rox() which can fail, leading to unprotected memory. >> >> Take into account return from set_memory_XX() functions and add >> __must_check flag to arch_protect_bpf_trampoline(). >> >> Signed-off-by: Christophe Leroy <[email protected]> > > ... > >> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c >> index ea6843be2616..23ce17da3bf7 100644 >> --- a/kernel/bpf/core.c >> +++ b/kernel/bpf/core.c >> @@ -898,23 +898,30 @@ static LIST_HEAD(pack_list); >> static struct bpf_prog_pack *alloc_new_pack(bpf_jit_fill_hole_t >> bpf_fill_ill_insns) >> { >> struct bpf_prog_pack *pack; >> + int err; >> >> pack = kzalloc(struct_size(pack, bitmap, >> BITS_TO_LONGS(BPF_PROG_CHUNK_COUNT)), >> GFP_KERNEL); >> if (!pack) >> return NULL; >> pack->ptr = bpf_jit_alloc_exec(BPF_PROG_PACK_SIZE); >> - if (!pack->ptr) { >> - kfree(pack); >> - return NULL; >> - } >> + if (!pack->ptr) >> + goto out; >> bpf_fill_ill_insns(pack->ptr, BPF_PROG_PACK_SIZE); >> bitmap_zero(pack->bitmap, BPF_PROG_PACK_SIZE / BPF_PROG_CHUNK_SIZE); >> list_add_tail(&pack->list, &pack_list); > > Hi Christophe, > > Here pack is added to pack_list. > >> >> set_vm_flush_reset_perms(pack->ptr); >> - set_memory_rox((unsigned long)pack->ptr, BPF_PROG_PACK_SIZE / >> PAGE_SIZE); >> + err = set_memory_rox((unsigned long)pack->ptr, BPF_PROG_PACK_SIZE / >> PAGE_SIZE); >> + if (err) >> + goto out_free; > > But this unwind path doesn't appear to remove pack form pack_list.
Ah, thanks, Indeed I wondered about it and ignored it as I mis-read pack_list as pack->list, thinking it would implicitely fly away when droping pack. I'll send a v2 in a few days once more people have reviewed it. Christophe
