Skip to content

Commit

Permalink
modules: wait do_free_init correctly
Browse files Browse the repository at this point in the history
[ Upstream commit 8f8cd6c0a43ed637e620bbe45a8d0e0c2f4d5130 ]

The synchronization here is to ensure the ordering of freeing of a module
init so that it happens before W+X checking.  It is worth noting it is not
that the freeing was not happening, it is just that our sanity checkers
raced against the permission checkers which assume init memory is already
gone.

Commit 1a7b7d9 ("modules: Use vmalloc special flag") moved calling
do_free_init() into a global workqueue instead of relying on it being
called through call_rcu(..., do_free_init), which used to allowed us call
do_free_init() asynchronously after the end of a subsequent grace period.
The move to a global workqueue broke the gaurantees for code which needed
to be sure the do_free_init() would complete with rcu_barrier().  To fix
this callers which used to rely on rcu_barrier() must now instead use
flush_work(&init_free_wq).

Without this fix, we still could encounter false positive reports in W+X
checking since the rcu_barrier() here can not ensure the ordering now.

Even worse, the rcu_barrier() can introduce significant delay.  Eric
Chanudet reported that the rcu_barrier introduces ~0.1s delay on a
PREEMPT_RT kernel.

  [    0.291444] Freeing unused kernel memory: 5568K
  [    0.402442] Run /sbin/init as init process

With this fix, the above delay can be eliminated.

Link: https://lkml.kernel.org/r/20240227023546.2490667-1-changbin.du@huawei.com
Fixes: 1a7b7d9 ("modules: Use vmalloc special flag")
Signed-off-by: Changbin Du <changbin.du@huawei.com>
Tested-by: Eric Chanudet <echanude@redhat.com>
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Cc: Xiaoyi Su <suxiaoyi@huawei.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Changbin Du authored and Sasha Levin committed Mar 26, 2024
1 parent b6ca437 commit 8edc140
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
8 changes: 8 additions & 0 deletions include/linux/moduleloader.h
Expand Up @@ -115,6 +115,14 @@ int module_finalize(const Elf_Ehdr *hdr,
const Elf_Shdr *sechdrs,
struct module *mod);

#ifdef CONFIG_MODULES
void flush_module_init_free_work(void);
#else
static inline void flush_module_init_free_work(void)
{
}
#endif

/* Any cleanup needed when module leaves. */
void module_arch_cleanup(struct module *mod);

Expand Down
5 changes: 3 additions & 2 deletions init/main.c
Expand Up @@ -88,6 +88,7 @@
#include <linux/sched/task_stack.h>
#include <linux/context_tracking.h>
#include <linux/random.h>
#include <linux/moduleloader.h>
#include <linux/list.h>
#include <linux/integrity.h>
#include <linux/proc_ns.h>
Expand Down Expand Up @@ -1402,11 +1403,11 @@ static void mark_readonly(void)
if (rodata_enabled) {
/*
* load_module() results in W+X mappings, which are cleaned
* up with call_rcu(). Let's make sure that queued work is
* up with init_free_wq. Let's make sure that queued work is
* flushed so that we don't hit false positives looking for
* insecure pages which are W+X.
*/
rcu_barrier();
flush_module_init_free_work();
mark_rodata_ro();
rodata_test();
} else
Expand Down
9 changes: 7 additions & 2 deletions kernel/module/main.c
Expand Up @@ -2486,6 +2486,11 @@ static void do_free_init(struct work_struct *w)
}
}

void flush_module_init_free_work(void)
{
flush_work(&init_free_wq);
}

#undef MODULE_PARAM_PREFIX
#define MODULE_PARAM_PREFIX "module."
/* Default value for module->async_probe_requested */
Expand Down Expand Up @@ -2590,8 +2595,8 @@ static noinline int do_init_module(struct module *mod)
* Note that module_alloc() on most architectures creates W+X page
* mappings which won't be cleaned up until do_free_init() runs. Any
* code such as mark_rodata_ro() which depends on those mappings to
* be cleaned up needs to sync with the queued work - ie
* rcu_barrier()
* be cleaned up needs to sync with the queued work by invoking
* flush_module_init_free_work().
*/
if (llist_add(&freeinit->node, &init_free_list))
schedule_work(&init_free_wq);
Expand Down

0 comments on commit 8edc140

Please sign in to comment.