public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@kernel.org>
To: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>
Cc: bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Christoph Hellwig <hch@lst.de>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Martynas Pumputis <m@lambda.lt>
Subject: [PATCH bpf-next 4/8] bpf: Take module reference on kprobe_multi link
Date: Sun,  9 Oct 2022 23:59:22 +0200	[thread overview]
Message-ID: <20221009215926.970164-5-jolsa@kernel.org> (raw)
In-Reply-To: <20221009215926.970164-1-jolsa@kernel.org>

Currently we allow to create kprobe multi link on function from kernel
module, but we don't take the module reference to ensure it's not
unloaded while we are tracing it.

The multi kprobe link is based on fprobe/ftrace layer which takes
different approach and releases ftrace hooks when module is unloaded
even if there's tracer registered on top of it.

Adding code that gathers all the related modules for the link and takes
their references before it's attached. All kernel module references are
released after link is unregistered.

Note that we do it the same way already for trampoline probes
(but for single address).

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/bpf_trace.c | 100 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 9be1a2b6b53b..f3d7565fee79 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2447,6 +2447,8 @@ struct bpf_kprobe_multi_link {
 	unsigned long *addrs;
 	u64 *cookies;
 	u32 cnt;
+	struct module **mods;
+	u32 mods_cnt;
 };
 
 struct bpf_kprobe_multi_run_ctx {
@@ -2502,6 +2504,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);
@@ -2514,6 +2524,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)
@@ -2523,6 +2534,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);
 }
 
@@ -2658,6 +2670,80 @@ 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_alloc;
+};
+
+static int module_callback(void *data, const char *name,
+			   struct module *mod, unsigned long addr)
+{
+	struct module_addr_args *args = data;
+	bool realloc = !args->mods;
+	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(unsigned long),
+		       bpf_kprobe_multi_addrs_cmp))
+		return 0;
+
+	if (args->mods) {
+		struct module *prev = NULL;
+
+		if (args->mods_cnt > 1)
+			prev = args->mods[args->mods_cnt - 1];
+		if (prev == mod)
+			return 0;
+		if (args->mods_cnt == args->mods_alloc)
+			realloc = true;
+	}
+
+	if (realloc) {
+		args->mods_alloc += 100;
+		mods = krealloc_array(args->mods, args->mods_alloc, 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;
@@ -2768,7 +2854,21 @@ 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) {
-- 
2.37.3


  parent reply	other threads:[~2022-10-09 22:00 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-09 21:59 [PATCH bpf-next 0/8] bpf: Fixes for kprobe multi on kernel modules Jiri Olsa
2022-10-09 21:59 ` [PATCH bpf-next 1/8] kallsyms: Make module_kallsyms_on_each_symbol generally available Jiri Olsa
2022-10-11  6:56   ` Song Liu
2022-10-09 21:59 ` [PATCH bpf-next 2/8] ftrace: Add support to resolve module symbols in ftrace_lookup_symbols Jiri Olsa
2022-10-11  7:05   ` Song Liu
2022-10-11 10:07     ` Jiri Olsa
2022-10-09 21:59 ` [PATCH bpf-next 3/8] bpf: Rename __bpf_kprobe_multi_cookie_cmp to bpf_kprobe_multi_addrs_cmp Jiri Olsa
2022-10-11  7:06   ` Song Liu
2022-10-09 21:59 ` Jiri Olsa [this message]
2022-10-11  7:16   ` [PATCH bpf-next 4/8] bpf: Take module reference on kprobe_multi link Song Liu
2022-10-11 10:09     ` Jiri Olsa
2022-10-13 18:50   ` Andrii Nakryiko
2022-10-14 10:17     ` Jiri Olsa
2022-10-09 21:59 ` [PATCH bpf-next 5/8] selftests/bpf: Add load_kallsyms_refresh function Jiri Olsa
2022-10-11  7:17   ` Song Liu
2022-10-09 21:59 ` [PATCH bpf-next 6/8] selftests/bpf: Add bpf_testmod_fentry_* functions Jiri Olsa
2022-10-11  7:24   ` Song Liu
2022-10-09 21:59 ` [PATCH bpf-next 7/8] selftests/bpf: Add kprobe_multi kmod link api tests Jiri Olsa
2022-10-11  7:27   ` Song Liu
2022-10-11 10:09     ` Jiri Olsa
2022-10-13 19:06   ` Andrii Nakryiko
2022-10-14 10:25     ` Jiri Olsa
2022-10-09 21:59 ` [PATCH bpf-next 8/8] selftests/bpf: Add kprobe_multi check to module attach test Jiri Olsa
2022-10-11  7:27   ` Song Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221009215926.970164-5-jolsa@kernel.org \
    --to=jolsa@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=hch@lst.de \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=m@lambda.lt \
    --cc=mhiramat@kernel.org \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox