Commit 31af1aa0 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf: Fixes for kprobe multi on kernel modules' 



Jiri Olsa says:

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

hi,
Martynas reported kprobe _multi link does not resolve symbols
from kernel modules, which attach by address works.

In addition while fixing that I realized we do not take module
reference if the module has kprobe_multi link on top of it and
can be removed.

There's mo crash related to this, it will silently disappear from
ftrace tables, while kprobe_multi link stays up with no data.

This patchset has fixes for both issues.

v3 changes:
  - reorder fields in struct bpf_kprobe_multi_link [Andrii]
  - added ack [Andrii]

v2 changes:
  - added acks (Song)
  - added comment to kallsyms_callback (Song)
  - change module_callback realloc logic (Andrii)
  - get rid of macros in tests (Andrii)

thanks,
jirka
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 152e60e3 b2440443

include/linux/module.h

+9 −0
Original line number Diff line number Diff line
@@ -879,8 +879,17 @@ static inline bool module_sig_ok(struct module *module) 
}

#endif	/* CONFIG_MODULE_SIG */



#if defined(CONFIG_MODULES) && defined(CONFIG_KALLSYMS)

int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

					     struct module *, unsigned long),

				   void *data);

#else

static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

						 struct module *, unsigned long),

						 void *data)

{

	return -EOPNOTSUPP;

}

#endif  /* CONFIG_MODULES && CONFIG_KALLSYMS */



#endif /* _LINUX_MODULE_H */

kernel/module/kallsyms.c

+0 −2
Original line number Diff line number Diff line
@@ -494,7 +494,6 @@ unsigned long module_kallsyms_lookup_name(const char *name) 
	return ret;

}



#ifdef CONFIG_LIVEPATCH

int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

					     struct module *, unsigned long),

				   void *data)
@@ -531,4 +530,3 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, 
	mutex_unlock(&module_mutex);

	return ret;

}
 
#endif /* CONFIG_LIVEPATCH */

kernel/trace/bpf_trace.c

+95 −3
Original line number Diff line number Diff line
@@ -2450,6 +2450,8 @@ struct bpf_kprobe_multi_link { 
	unsigned long *addrs;

	u64 *cookies;

	u32 cnt;

	u32 mods_cnt;

	struct module **mods;

};



struct bpf_kprobe_multi_run_ctx {
@@ -2505,6 +2507,14 @@ static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32 
	return err;

}



static void kprobe_multi_put_modules(struct module **mods, u32 cnt)

{

	u32 i;



	for (i = 0; i < cnt; i++)

		module_put(mods[i]);

}



static void free_user_syms(struct user_syms *us)

{

	kvfree(us->syms);
@@ -2517,6 +2527,7 @@ static void bpf_kprobe_multi_link_release(struct bpf_link *link) 


	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);

	unregister_fprobe(&kmulti_link->fp);

	kprobe_multi_put_modules(kmulti_link->mods, kmulti_link->mods_cnt);

}



static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
@@ -2526,6 +2537,7 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link) 
	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);

	kvfree(kmulti_link->addrs);

	kvfree(kmulti_link->cookies);

	kfree(kmulti_link->mods);

	kfree(kmulti_link);

}
@@ -2548,7 +2560,7 @@ static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void 
	swap(*cookie_a, *cookie_b);

}



static int __bpf_kprobe_multi_cookie_cmp(const void *a, const void *b)

static int bpf_kprobe_multi_addrs_cmp(const void *a, const void *b)

{

	const unsigned long *addr_a = a, *addr_b = b;
@@ -2559,7 +2571,7 @@ static int __bpf_kprobe_multi_cookie_cmp(const void *a, const void *b) 


static int bpf_kprobe_multi_cookie_cmp(const void *a, const void *b, const void *priv)

{

	return __bpf_kprobe_multi_cookie_cmp(a, b);

	return bpf_kprobe_multi_addrs_cmp(a, b);

}



static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
@@ -2577,7 +2589,7 @@ static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx) 
		return 0;

	entry_ip = run_ctx->entry_ip;

	addr = bsearch(&entry_ip, link->addrs, link->cnt, sizeof(entry_ip),

		       __bpf_kprobe_multi_cookie_cmp);

		       bpf_kprobe_multi_addrs_cmp);

	if (!addr)

		return 0;

	cookie = link->cookies + (addr - link->addrs);
@@ -2661,6 +2673,71 @@ static void symbols_swap_r(void *a, void *b, int size, const void *priv) 
	}

}



struct module_addr_args {

	unsigned long *addrs;

	u32 addrs_cnt;

	struct module **mods;

	int mods_cnt;

	int mods_cap;

};



static int module_callback(void *data, const char *name,

			   struct module *mod, unsigned long addr)

{

	struct module_addr_args *args = data;

	struct module **mods;



	/* We iterate all modules symbols and for each we:

	 * - search for it in provided addresses array

	 * - if found we check if we already have the module pointer stored

	 *   (we iterate modules sequentially, so we can check just the last

	 *   module pointer)

	 * - take module reference and store it

	 */

	if (!bsearch(&addr, args->addrs, args->addrs_cnt, sizeof(addr),

		       bpf_kprobe_multi_addrs_cmp))

		return 0;



	if (args->mods && args->mods[args->mods_cnt - 1] == mod)

		return 0;



	if (args->mods_cnt == args->mods_cap) {

		args->mods_cap = max(16, args->mods_cap * 3 / 2);

		mods = krealloc_array(args->mods, args->mods_cap, sizeof(*mods), GFP_KERNEL);

		if (!mods)

			return -ENOMEM;

		args->mods = mods;

	}



	if (!try_module_get(mod))

		return -EINVAL;



	args->mods[args->mods_cnt] = mod;

	args->mods_cnt++;

	return 0;

}



static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u32 addrs_cnt)

{

	struct module_addr_args args = {

		.addrs     = addrs,

		.addrs_cnt = addrs_cnt,

	};

	int err;



	/* We return either err < 0 in case of error, ... */

	err = module_kallsyms_on_each_symbol(module_callback, &args);

	if (err) {

		kprobe_multi_put_modules(args.mods, args.mods_cnt);

		kfree(args.mods);

		return err;

	}



	/* or number of modules found if everything is ok. */

	*mods = args.mods;

	return args.mods_cnt;

}



int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)

{

	struct bpf_kprobe_multi_link *link = NULL;
@@ -2771,10 +2848,25 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr 
		       bpf_kprobe_multi_cookie_cmp,

		       bpf_kprobe_multi_cookie_swap,

		       link);

	} else {

		/*

		 * We need to sort addrs array even if there are no cookies

		 * provided, to allow bsearch in get_modules_for_addrs.

		 */

		sort(addrs, cnt, sizeof(*addrs),

		       bpf_kprobe_multi_addrs_cmp, NULL);

	}



	err = get_modules_for_addrs(&link->mods, addrs, cnt);

	if (err < 0) {

		bpf_link_cleanup(&link_primer);

		return err;

	}

	link->mods_cnt = err;



	err = register_fprobe_ips(&link->fp, addrs, cnt);

	if (err) {

		kprobe_multi_put_modules(link->mods, link->mods_cnt);

		bpf_link_cleanup(&link_primer);

		return err;

	}

kernel/trace/ftrace.c

+11 −5
Original line number Diff line number Diff line
@@ -8267,6 +8267,10 @@ struct kallsyms_data { 
	size_t found;

};



/* This function gets called for all kernel and module symbols

 * and returns 1 in case we resolved all the requested symbols,

 * 0 otherwise.

 */

static int kallsyms_callback(void *data, const char *name,

			     struct module *mod, unsigned long addr)

{
@@ -8309,17 +8313,19 @@ static int kallsyms_callback(void *data, const char *name, 
int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)

{

	struct kallsyms_data args;

	int err;

	int found_all;



	memset(addrs, 0, sizeof(*addrs) * cnt);

	args.addrs = addrs;

	args.syms = sorted_syms;

	args.cnt = cnt;

	args.found = 0;

	err = kallsyms_on_each_symbol(kallsyms_callback, &args);

	if (err < 0)

		return err;

	return args.found == args.cnt ? 0 : -ESRCH;



	found_all = kallsyms_on_each_symbol(kallsyms_callback, &args);

	if (found_all)

		return 0;

	found_all = module_kallsyms_on_each_symbol(kallsyms_callback, &args);

	return found_all ? 0 : -ESRCH;

}



#ifdef CONFIG_SYSCTL

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c

+24 −0
Original line number Diff line number Diff line
@@ -128,6 +128,23 @@ __weak noinline struct file *bpf_testmod_return_ptr(int arg) 
	}

}



noinline int bpf_testmod_fentry_test1(int a)

{

	return a + 1;

}



noinline int bpf_testmod_fentry_test2(int a, u64 b)

{

	return a + b;

}



noinline int bpf_testmod_fentry_test3(char a, int b, u64 c)

{

	return a + b + c;

}



int bpf_testmod_fentry_ok;



noinline ssize_t

bpf_testmod_test_read(struct file *file, struct kobject *kobj,

		      struct bin_attribute *bin_attr,
@@ -167,6 +184,13 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj, 
			return snprintf(buf, len, "%d\n", writable.val);

	}



	if (bpf_testmod_fentry_test1(1) != 2 ||

	    bpf_testmod_fentry_test2(2, 3) != 5 ||

	    bpf_testmod_fentry_test3(4, 5, 6) != 15)

		goto out;



	bpf_testmod_fentry_ok = 1;

out:

	return -EIO; /* always fail */

}

EXPORT_SYMBOL(bpf_testmod_test_read);