> On Nov 28, 2018, at 1:57 AM, Will Deacon <will.deacon@xxxxxxx> wrote: > > On Tue, Nov 27, 2018 at 05:21:08PM -0800, Nadav Amit wrote: >>> On Nov 27, 2018, at 5:06 PM, Nadav Amit <nadav.amit@xxxxxxxxx> wrote: >>> >>>> On Nov 27, 2018, at 4:07 PM, Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx> wrote: >>>> >>>> Sometimes when memory is freed via the module subsystem, an executable >>>> permissioned TLB entry can remain to a freed page. If the page is re-used to >>>> back an address that will receive data from userspace, it can result in user >>>> data being mapped as executable in the kernel. The root of this behavior is >>>> vfree lazily flushing the TLB, but not lazily freeing the underlying pages. >>>> >>>> There are sort of three categories of this which show up across modules, bpf, >>>> kprobes and ftrace: >>>> >>>> 1. When executable memory is touched and then immediatly freed >>>> >>>> This shows up in a couple error conditions in the module loader and BPF JIT >>>> compiler. >>> >>> Interesting! >>> >>> Note that this may cause conflict with "x86: avoid W^X being broken during >>> modules loading”, which I recently submitted. >> >> I actually have not looked on the vmalloc() code too much recent, but it >> seems … strange: >> >> void vm_unmap_aliases(void) >> { >> >> ... >> mutex_lock(&vmap_purge_lock); >> purge_fragmented_blocks_allcpus(); >> if (!__purge_vmap_area_lazy(start, end) && flush) >> flush_tlb_kernel_range(start, end); >> mutex_unlock(&vmap_purge_lock); >> } >> >> Since __purge_vmap_area_lazy() releases the memory, it seems there is a time >> window between the release of the region and the TLB flush, in which the >> area can be allocated for another purpose. This can result in a >> (theoretical) correctness issue. No? > > If __purge_vmap_area_lazy() returns false, then it hasn't freed the memory, > so we only invalidate the TLB if 'flush' is true in that case. If > __purge_vmap_area_lazy() returns true instead, then it takes care of the TLB > invalidation before the freeing. Right. Sorry for my misunderstanding. Thanks, Nadav