Re: [PATCH v4 16/23] vmalloc: Add flag for free of special permsissions

From: Peter Zijlstra
Date: Thu Apr 25 2019 - 16:39:01 EST


On Mon, Apr 22, 2019 at 11:57:58AM -0700, Rick Edgecombe wrote:
> Add a new flag VM_FLUSH_RESET_PERMS, for enabling vfree operations to
> immediately clear executable TLB entries before freeing pages, and handle
> resetting permissions on the directmap. This flag is useful for any kind
> of memory with elevated permissions, or where there can be related
> permissions changes on the directmap. Today this is RO+X and RO memory.
>
> Although this enables directly vfreeing non-writeable memory now,
> non-writable memory cannot be freed in an interrupt because the allocation
> itself is used as a node on deferred free list. So when RO memory needs to
> be freed in an interrupt the code doing the vfree needs to have its own
> work queue, as was the case before the deferred vfree list was added to
> vmalloc.
>
> For architectures with set_direct_map_ implementations this whole operation
> can be done with one TLB flush when centralized like this. For others with
> directmap permissions, currently only arm64, a backup method using
> set_memory functions is used to reset the directmap. When arm64 adds
> set_direct_map_ functions, this backup can be removed.
>
> When the TLB is flushed to both remove TLB entries for the vmalloc range
> mapping and the direct map permissions, the lazy purge operation could be
> done to try to save a TLB flush later. However today vm_unmap_aliases
> could flush a TLB range that does not include the directmap. So a helper
> is added with extra parameters that can allow both the vmalloc address and
> the direct mapping to be flushed during this operation. The behavior of the
> normal vm_unmap_aliases function is unchanged.

> +static inline void set_vm_flush_reset_perms(void *addr)
> +{
> + struct vm_struct *vm = find_vm_area(addr);
> +
> + if (vm)
> + vm->flags |= VM_FLUSH_RESET_PERMS;
> +}

So, previously in the series we added NX to module_alloc() and fixed up
all the usage site. And now we're going through those very same sites to
add set_vm_flush_reset_perms().

Why isn't module_alloc() calling the above function and avoid sprinkling
it all over the place again?