Commit 72fe61d7 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'Fix attaching fentry/fexit/fmod_ret/lsm to modules' 



Viktor Malik says:

====================

I noticed that the verifier behaves incorrectly when attaching to fentry
of multiple functions of the same name located in different modules (or
in vmlinux). The reason for this is that if the target program is not
specified, the verifier will search kallsyms for the trampoline address
to attach to. The entire kallsyms is always searched, not respecting the
module in which the function to attach to is located.

As Yonghong correctly pointed out, there is yet another issue - the
trampoline acquires the module reference in register_fentry which means
that if the module is unloaded between the place where the address is
found in the verifier and register_fentry, it is possible that another
module is loaded to the same address in the meantime, which may lead to
errors.

This patch fixes the above issues by extracting the module name from the
BTF of the attachment target (which must be specified) and by doing the
search in kallsyms of the correct module. At the same time, the module
reference is acquired right after the address is found and only released
right before the program itself is unloaded.
---
Changes in v10:
- added the new test to DENYLIST.aarch64 (suggested by Andrii)
- renamed the test source file to match the test name

Changes in v9:
- two small changes suggested by Jiri Olsa and Jiri's ack

Changes in v8:
- added module_put to error paths in bpf_check_attach_target after the
  module reference is acquired

Changes in v7:
- refactored the module reference manipulation (comments by Jiri Olsa)
- cleaned up the test (comments by Andrii Nakryiko)

Changes in v6:
- storing the module reference inside bpf_prog_aux instead of
  bpf_trampoline and releasing it when the program is unloaded
  (suggested by Jiri Olsa)

Changes in v5:
- fixed acquiring and releasing of module references by trampolines to
  prevent modules being unloaded between address lookup and trampoline
  allocation

Changes in v4:
- reworked module kallsyms lookup approach using existing functions,
  verifier now calls btf_try_get_module to retrieve the module and
  find_kallsyms_symbol_value to get the symbol address (suggested by
  Alexei)
- included Jiri Olsa's comments
- improved description of the new test and added it as a comment into
  the test source

Changes in v3:
- added trivial implementation for kallsyms_lookup_name_in_module() for
  !CONFIG_MODULES (noticed by test robot, fix suggested by Hao Luo)

Changes in v2:
- introduced and used more space-efficient kallsyms lookup function,
  suggested by Jiri Olsa
- included Hao Luo's comments
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents b8a2e3f9 aa3d65de

include/linux/bpf.h

+2 −0
Original line number Diff line number Diff line
@@ -1103,6 +1103,7 @@ struct bpf_trampoline { 
struct bpf_attach_target_info {

	struct btf_func_model fmodel;

	long tgt_addr;

	struct module *tgt_mod;

	const char *tgt_name;

	const struct btf_type *tgt_type;

};
@@ -1406,6 +1407,7 @@ struct bpf_prog_aux { 
	 * main prog always has linfo_idx == 0

	 */

	u32 linfo_idx;

	struct module *mod;

	u32 num_exentries;

	struct exception_table_entry *extable;

	union {

kernel/bpf/syscall.c

+6 −0
Original line number Diff line number Diff line
@@ -2067,6 +2067,7 @@ static void __bpf_prog_put_noref(struct bpf_prog *prog, bool deferred) 
{

	bpf_prog_kallsyms_del_all(prog);

	btf_put(prog->aux->btf);

	module_put(prog->aux->mod);

	kvfree(prog->aux->jited_linfo);

	kvfree(prog->aux->linfo);

	kfree(prog->aux->kfunc_tab);
@@ -3113,6 +3114,11 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, 
		if (err)

			goto out_unlock;



		if (tgt_info.tgt_mod) {

			module_put(prog->aux->mod);

			prog->aux->mod = tgt_info.tgt_mod;

		}



		tr = bpf_trampoline_get(key, &tgt_info);

		if (!tr) {

			err = -ENOMEM;

kernel/bpf/trampoline.c

+0 −28
Original line number Diff line number Diff line
@@ -9,7 +9,6 @@ 
#include <linux/btf.h>

#include <linux/rcupdate_trace.h>

#include <linux/rcupdate_wait.h>

#include <linux/module.h>

#include <linux/static_call.h>

#include <linux/bpf_verifier.h>

#include <linux/bpf_lsm.h>
@@ -172,26 +171,6 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key) 
	return tr;

}



static int bpf_trampoline_module_get(struct bpf_trampoline *tr)

{

	struct module *mod;

	int err = 0;



	preempt_disable();

	mod = __module_text_address((unsigned long) tr->func.addr);

	if (mod && !try_module_get(mod))

		err = -ENOENT;

	preempt_enable();

	tr->mod = mod;

	return err;

}



static void bpf_trampoline_module_put(struct bpf_trampoline *tr)

{

	module_put(tr->mod);

	tr->mod = NULL;

}



static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)

{

	void *ip = tr->func.addr;
@@ -202,8 +181,6 @@ static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr) 
	else

		ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, NULL);



	if (!ret)

		bpf_trampoline_module_put(tr);

	return ret;

}
@@ -238,9 +215,6 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr) 
		tr->func.ftrace_managed = true;

	}



	if (bpf_trampoline_module_get(tr))

		return -ENOENT;



	if (tr->func.ftrace_managed) {

		ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);

		ret = register_ftrace_direct_multi(tr->fops, (long)new_addr);
@@ -248,8 +222,6 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr) 
		ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);

	}



	if (ret)

		bpf_trampoline_module_put(tr);

	return ret;

}

kernel/bpf/verifier.c

+17 −1
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 
#include <linux/bpf_lsm.h>

#include <linux/btf_ids.h>

#include <linux/poison.h>

#include "../module/internal.h"









#include "disasm.h"
















@@ -18307,6 +18308,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







	const char *tname;









	struct btf *btf;









	long addr = 0;









	struct module *mod = NULL;

















	if (!btf_id) {









		bpf_log(log, "Tracing programs must provide btf_id\n");








@@ -18479,9 +18481,18 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







				addr = (long) tgt_prog->bpf_func;









			else









				addr = (long) tgt_prog->aux->func[subprog]->bpf_func;









		} else {









			if (btf_is_module(btf)) {









				mod = btf_try_get_module(btf);









				if (mod)









					addr = find_kallsyms_symbol_value(mod, tname);









				else









					addr = 0;









			} else {









				addr = kallsyms_lookup_name(tname);









			}









			if (!addr) {









				module_put(mod);









				bpf_log(log,









					"The address of function %s cannot be found\n",









					tname);








@@ -18521,11 +18532,13 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







				break;









			}









			if (ret) {









				module_put(mod);









				bpf_log(log, "%s is not sleepable\n", tname);









				return ret;









			}









		} else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {









			if (tgt_prog) {









				module_put(mod);









				bpf_log(log, "can't modify return codes of BPF programs\n");









				return -EINVAL;









			}








@@ -18534,6 +18547,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







			    !check_attach_modify_return(addr, tname))









				ret = 0;









			if (ret) {









				module_put(mod);









				bpf_log(log, "%s() is not modifiable\n", tname);









				return ret;









			}








@@ -18544,6 +18558,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







	tgt_info->tgt_addr = addr;









	tgt_info->tgt_name = tname;









	tgt_info->tgt_type = t;









	tgt_info->tgt_mod = mod;









	return 0;









}
















@@ -18623,6 +18638,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)







	/* store info about the attachment target that will be used later */









	prog->aux->attach_func_proto = tgt_info.tgt_type;









	prog->aux->attach_func_name = tgt_info.tgt_name;









	prog->aux->mod = tgt_info.tgt_mod;

















	if (tgt_prog) {









		prog->aux->saved_dst_prog_type = tgt_prog->type;

























kernel/module/internal.h



+5
−0






























Original line number
Diff line number
Diff line







@@ -256,6 +256,11 @@ static inline bool sect_empty(const Elf_Shdr *sect)







static inline void init_build_id(struct module *mod, const struct load_info *info) { }









static inline void layout_symtab(struct module *mod, struct load_info *info) { }









static inline void add_kallsyms(struct module *mod, const struct load_info *info) { }









static inline unsigned long find_kallsyms_symbol_value(struct module *mod,









						       const char *name)









{









	return 0;









}









#endif /* CONFIG_KALLSYMS */



















#ifdef CONFIG_SYSFS