[PATCH bpf v4 0/3] Fix kprobe_multi interface issues for 5.18

[PATCH bpf v4 0/3] Fix kprobe_multi interface issues for 5.18

[Eugene Syromiatnikov]

Hello.

While [1] seems to require additional work[2] due to changes
in the interface (and it has already been re-targeted for bpf-next),
I would like to ask to consider the following three patches, that fix
possible out-of-bounds write, properly disable the interface
for 32-bit compat user space, and prepare the libbpf interface change,
for the 5.18 release.  Thank you.

[1] https://lore.kernel.org/lkml/cover.1652772731.git.esyr@redhat.com/
[2] https://lore.kernel.org/lkml/YoTXiAk1EpZ0rLKE@krava/i

v4:
 - Added additional size checks for INT_MAX, as suggested by Yonghong
   Song
 - Added the third patch for the user space kprobe_multi.addrs type
   change, split from the 4th bpf-next patch, as suggested by Yonghong
   Song and Andrii Nakryiko

v3: https://lore.kernel.org/lkml/cover.1652876187.git.esyr@redhat.com/
 - Split out patches for 5.18
 - Removed superfluous size assignments after overflow_mul_check,
   as suggested by Yonghong Song

v2: https://lore.kernel.org/lkml/20220516230441.GA22091@asgard.redhat.com/
 - Fixed the isses reported by CI

v1: https://lore.kernel.org/lkml/20220516182657.GA28596@asgard.redhat.com/

Eugene Syromiatnikov (3):
  bpf_trace: check size for overflow in bpf_kprobe_multi_link_attach
  bpf_trace: bail out from bpf_kprobe_multi_link_attach when in compat
  libbpf, selftests/bpf: pass array of u64 values in kprobe_multi.addrs

 kernel/trace/bpf_trace.c                                  | 15 +++++++++------
 tools/lib/bpf/bpf.h                                       |  2 +-
 tools/lib/bpf/libbpf.c                                    |  8 ++++----
 tools/lib/bpf/libbpf.h                                    |  2 +-
 tools/testing/selftests/bpf/prog_tests/bpf_cookie.c       |  2 +-
 .../testing/selftests/bpf/prog_tests/kprobe_multi_test.c  |  8 ++++----
 6 files changed, 20 insertions(+), 17 deletions(-)

-- 
2.1.4

[PATCH bpf v4 1/3] bpf_trace: check size for overflow in bpf_kprobe_multi_link_attach

[Eugene Syromiatnikov]

Check that size would not overflow before calculation (and return
-EOVERFLOW if it will), to prevent potential out-of-bounds write
with the following copy_from_user.  Add the same check
to kprobe_multi_resolve_syms in case it will be called from elsewhere
in the future.  The INT_MAX checks are performed in order to avoid
triggering kvmalloc_node warning [1].

[1] https://lore.kernel.org/lkml/cfe6abea-8d00-8f8c-f84c-e6f27753b5d1@fb.com/

Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 kernel/trace/bpf_trace.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d8553f4..26cf99c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2352,13 +2352,15 @@ static int
 kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt,
 			  unsigned long *addrs)
 {
-	unsigned long addr, size;
+	unsigned long addr, sym_size;
+	u32 size;
 	const char __user **syms;
 	int err = -ENOMEM;
 	unsigned int i;
 	char *func;
 
-	size = cnt * sizeof(*syms);
+	if (check_mul_overflow(cnt, (u32)sizeof(*syms), &size) || size > INT_MAX)
+		return -EOVERFLOW;
 	syms = kvzalloc(size, GFP_KERNEL);
 	if (!syms)
 		return -ENOMEM;
@@ -2382,9 +2384,9 @@ kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt,
 		addr = kallsyms_lookup_name(func);
 		if (!addr)
 			goto error;
-		if (!kallsyms_lookup_size_offset(addr, &size, NULL))
+		if (!kallsyms_lookup_size_offset(addr, &sym_size, NULL))
 			goto error;
-		addr = ftrace_location_range(addr, addr + size - 1);
+		addr = ftrace_location_range(addr, addr + sym_size - 1);
 		if (!addr)
 			goto error;
 		addrs[i] = addr;
@@ -2429,7 +2431,8 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	if (!cnt)
 		return -EINVAL;
 
-	size = cnt * sizeof(*addrs);
+	if (check_mul_overflow(cnt, (u32)sizeof(*addrs), &size) || size > INT_MAX)
+		return -EOVERFLOW;
 	addrs = kvmalloc(size, GFP_KERNEL);
 	if (!addrs)
 		return -ENOMEM;
-- 
2.1.4

Re: [PATCH bpf v4 1/3] bpf_trace: check size for overflow in bpf_kprobe_multi_link_attach

[Yonghong Song]

On 5/19/22 11:14 AM, Eugene Syromiatnikov wrote:
> Check that size would not overflow before calculation (and return
> -EOVERFLOW if it will), to prevent potential out-of-bounds write
> with the following copy_from_user.  Add the same check
> to kprobe_multi_resolve_syms in case it will be called from elsewhere
> in the future.  The INT_MAX checks are performed in order to avoid
> triggering kvmalloc_node warning [1].
> 
> [1] https://lore.kernel.org/lkml/cfe6abea-8d00-8f8c-f84c-e6f27753b5d1@fb.com/
> 
> Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
> Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>

Acked-by: Yonghong Song <yhs@fb.com>

[PATCH bpf v4 2/3] bpf_trace: bail out from bpf_kprobe_multi_link_attach when in compat

[Eugene Syromiatnikov]

Since bpf_kprobe_multi_link_attach doesn't support 32-bit kernels
for whatever reason, having it enabled for compat processes on 64-bit
kernels makes even less sense due to discrepances in the type sizes
that it does not handle.

Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 kernel/trace/bpf_trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 26cf99c..d6db124 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2412,7 +2412,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	int err;
 
 	/* no support for 32bit archs yet */
-	if (sizeof(u64) != sizeof(void *))
+	if (sizeof(u64) != sizeof(void *) || in_compat_syscall())
 		return -EOPNOTSUPP;
 
 	if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI)
-- 
2.1.4

[PATCH bpf v4 3/3] libbpf, selftests/bpf: pass array of u64 values in kprobe_multi.addrs

[Eugene Syromiatnikov]

With the interface as defined, it is impossible to pass 64-bit kernel
addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI,
which severly limits the useability of the interface, change the API
to accept an array of u64 values instead of (kernel? user?) longs.
This patch implements the user space part of the change (without
the relevant kernel changes, since, as of now, an attempt to add
kprobe_multi link will fail with -EOPNOTSUPP), to avoid changing
the interface after a release.

Fixes: 5117c26e877352bc ("libbpf: Add bpf_link_create support for multi kprobes")
Fixes: ddc6b04989eb0993 ("libbpf: Add bpf_program__attach_kprobe_multi_opts function")
Fixes: f7a11eeccb111854 ("selftests/bpf: Add kprobe_multi attach test")
Fixes: 9271a0c7ae7a9147 ("selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts")
Fixes: 2c6401c966ae1fbe ("selftests/bpf: Add kprobe_multi bpf_cookie test")
Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/lib/bpf/bpf.h                                        | 2 +-
 tools/lib/bpf/libbpf.c                                     | 8 ++++----
 tools/lib/bpf/libbpf.h                                     | 2 +-
 tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        | 2 +-
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 8 ++++----
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index f4b4afb..f677602 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -417,7 +417,7 @@ struct bpf_link_create_opts {
 			__u32 flags;
 			__u32 cnt;
 			const char **syms;
-			const unsigned long *addrs;
+			const __u64 *addrs;
 			const __u64 *cookies;
 		} kprobe_multi;
 	};
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 809fe20..03a14a6 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10279,7 +10279,7 @@ static bool glob_match(const char *str, const char *pat)
 
 struct kprobe_multi_resolve {
 	const char *pattern;
-	unsigned long *addrs;
+	__u64 *addrs;
 	size_t cap;
 	size_t cnt;
 };
@@ -10294,12 +10294,12 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
 	if (!glob_match(sym_name, res->pattern))
 		return 0;
 
-	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
+	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(__u64),
 				res->cnt + 1);
 	if (err)
 		return err;
 
-	res->addrs[res->cnt++] = (unsigned long) sym_addr;
+	res->addrs[res->cnt++] = sym_addr;
 	return 0;
 }
 
@@ -10314,7 +10314,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 	};
 	struct bpf_link *link = NULL;
 	char errmsg[STRERR_BUFSIZE];
-	const unsigned long *addrs;
+	const __u64 *addrs;
 	int err, link_fd, prog_fd;
 	const __u64 *cookies;
 	const char **syms;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 05dde85..ec1cb61 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -431,7 +431,7 @@ struct bpf_kprobe_multi_opts {
 	/* array of function symbols to attach */
 	const char **syms;
 	/* array of function addresses to attach */
-	const unsigned long *addrs;
+	const __u64 *addrs;
 	/* array of user-provided values fetchable through bpf_get_attach_cookie */
 	const __u64 *cookies;
 	/* number of elements in syms/addrs/cookies arrays */
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index 923a613..5aa482a 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -137,7 +137,7 @@ static void kprobe_multi_link_api_subtest(void)
 	cookies[6] = 7;
 	cookies[7] = 8;
 
-	opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
+	opts.kprobe_multi.addrs = (const __u64 *) &addrs;
 	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
 	opts.kprobe_multi.cookies = (const __u64 *) &cookies;
 	prog_fd = bpf_program__fd(skel->progs.test_kprobe);
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index b9876b5..fbf4cf2 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -105,7 +105,7 @@ static void test_link_api_addrs(void)
 	GET_ADDR("bpf_fentry_test7", addrs[6]);
 	GET_ADDR("bpf_fentry_test8", addrs[7]);
 
-	opts.kprobe_multi.addrs = (const unsigned long*) addrs;
+	opts.kprobe_multi.addrs = (const __u64 *) addrs;
 	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
 	test_link_api(&opts);
 }
@@ -183,7 +183,7 @@ static void test_attach_api_addrs(void)
 	GET_ADDR("bpf_fentry_test7", addrs[6]);
 	GET_ADDR("bpf_fentry_test8", addrs[7]);
 
-	opts.addrs = (const unsigned long *) addrs;
+	opts.addrs = (const __u64 *) addrs;
 	opts.cnt = ARRAY_SIZE(addrs);
 	test_attach_api(NULL, &opts);
 }
@@ -241,7 +241,7 @@ static void test_attach_api_fails(void)
 		goto cleanup;
 
 	/* fail_2 - both addrs and syms set */
-	opts.addrs = (const unsigned long *) addrs;
+	opts.addrs = (const __u64 *) addrs;
 	opts.syms = syms;
 	opts.cnt = ARRAY_SIZE(syms);
 	opts.cookies = NULL;
@@ -255,7 +255,7 @@ static void test_attach_api_fails(void)
 		goto cleanup;
 
 	/* fail_3 - pattern and addrs set */
-	opts.addrs = (const unsigned long *) addrs;
+	opts.addrs = (const __u64 *) addrs;
 	opts.syms = NULL;
 	opts.cnt = ARRAY_SIZE(syms);
 	opts.cookies = NULL;
-- 
2.1.4

Re: [PATCH bpf v4 3/3] libbpf, selftests/bpf: pass array of u64 values in kprobe_multi.addrs

[Yonghong Song]

On 5/19/22 11:14 AM, Eugene Syromiatnikov wrote:
> With the interface as defined, it is impossible to pass 64-bit kernel
> addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI,
> which severly limits the useability of the interface, change the API
> to accept an array of u64 values instead of (kernel? user?) longs.
> This patch implements the user space part of the change (without
> the relevant kernel changes, since, as of now, an attempt to add
> kprobe_multi link will fail with -EOPNOTSUPP), to avoid changing
> the interface after a release.
> 
> Fixes: 5117c26e877352bc ("libbpf: Add bpf_link_create support for multi kprobes")
> Fixes: ddc6b04989eb0993 ("libbpf: Add bpf_program__attach_kprobe_multi_opts function")
> Fixes: f7a11eeccb111854 ("selftests/bpf: Add kprobe_multi attach test")
> Fixes: 9271a0c7ae7a9147 ("selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts")
> Fixes: 2c6401c966ae1fbe ("selftests/bpf: Add kprobe_multi bpf_cookie test")
> Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> ---
>   tools/lib/bpf/bpf.h                                        | 2 +-
>   tools/lib/bpf/libbpf.c                                     | 8 ++++----
>   tools/lib/bpf/libbpf.h                                     | 2 +-
>   tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        | 2 +-
>   tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 8 ++++----
>   5 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index f4b4afb..f677602 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -417,7 +417,7 @@ struct bpf_link_create_opts {
>   			__u32 flags;
>   			__u32 cnt;
>   			const char **syms;
> -			const unsigned long *addrs;
> +			const __u64 *addrs;

Patch 2 and 3 will prevent supporting 64-bit kernel, 32-bit userspace 
for kprobe_multi. So effectively, kprobe_multi only supports
64-bit kernel and 64-bit user space.
This is definitely an option, but it would be great
if other people can chime in as well for whether this choice
is best or not.

>   			const __u64 *cookies;
>   		} kprobe_multi;
>   	};
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 809fe20..03a14a6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10279,7 +10279,7 @@ static bool glob_match(const char *str, const char *pat)
>   
>   struct kprobe_multi_resolve {
>   	const char *pattern;
> -	unsigned long *addrs;
> +	__u64 *addrs;
>   	size_t cap;
>   	size_t cnt;
>   };
> @@ -10294,12 +10294,12 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>   	if (!glob_match(sym_name, res->pattern))
>   		return 0;
>   
> -	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> +	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(__u64),
>   				res->cnt + 1);
>   	if (err)
>   		return err;
>   
> -	res->addrs[res->cnt++] = (unsigned long) sym_addr;
> +	res->addrs[res->cnt++] = sym_addr;
>   	return 0;
>   }
>   
> @@ -10314,7 +10314,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>   	};
>   	struct bpf_link *link = NULL;
>   	char errmsg[STRERR_BUFSIZE];
> -	const unsigned long *addrs;
> +	const __u64 *addrs;
>   	int err, link_fd, prog_fd;
>   	const __u64 *cookies;
>   	const char **syms;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 05dde85..ec1cb61 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -431,7 +431,7 @@ struct bpf_kprobe_multi_opts {
>   	/* array of function symbols to attach */
>   	const char **syms;
>   	/* array of function addresses to attach */
> -	const unsigned long *addrs;
> +	const __u64 *addrs;
>   	/* array of user-provided values fetchable through bpf_get_attach_cookie */
>   	const __u64 *cookies;
>   	/* number of elements in syms/addrs/cookies arrays */
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> index 923a613..5aa482a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> @@ -137,7 +137,7 @@ static void kprobe_multi_link_api_subtest(void)
>   	cookies[6] = 7;
>   	cookies[7] = 8;
>   
> -	opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
> +	opts.kprobe_multi.addrs = (const __u64 *) &addrs;
>   	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
>   	opts.kprobe_multi.cookies = (const __u64 *) &cookies;
>   	prog_fd = bpf_program__fd(skel->progs.test_kprobe);
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> index b9876b5..fbf4cf2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -105,7 +105,7 @@ static void test_link_api_addrs(void)
>   	GET_ADDR("bpf_fentry_test7", addrs[6]);
>   	GET_ADDR("bpf_fentry_test8", addrs[7]);
>   
> -	opts.kprobe_multi.addrs = (const unsigned long*) addrs;
> +	opts.kprobe_multi.addrs = (const __u64 *) addrs;
>   	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
>   	test_link_api(&opts);
>   }
> @@ -183,7 +183,7 @@ static void test_attach_api_addrs(void)
>   	GET_ADDR("bpf_fentry_test7", addrs[6]);
>   	GET_ADDR("bpf_fentry_test8", addrs[7]);
>   
> -	opts.addrs = (const unsigned long *) addrs;
> +	opts.addrs = (const __u64 *) addrs;
>   	opts.cnt = ARRAY_SIZE(addrs);
>   	test_attach_api(NULL, &opts);
>   }
> @@ -241,7 +241,7 @@ static void test_attach_api_fails(void)
>   		goto cleanup;
>   
>   	/* fail_2 - both addrs and syms set */
> -	opts.addrs = (const unsigned long *) addrs;
> +	opts.addrs = (const __u64 *) addrs;
>   	opts.syms = syms;
>   	opts.cnt = ARRAY_SIZE(syms);
>   	opts.cookies = NULL;
> @@ -255,7 +255,7 @@ static void test_attach_api_fails(void)
>   		goto cleanup;
>   
>   	/* fail_3 - pattern and addrs set */
> -	opts.addrs = (const unsigned long *) addrs;
> +	opts.addrs = (const __u64 *) addrs;
>   	opts.syms = NULL;
>   	opts.cnt = ARRAY_SIZE(syms);
>   	opts.cookies = NULL;

Re: [PATCH bpf v4 3/3] libbpf, selftests/bpf: pass array of u64 values in kprobe_multi.addrs

[Jiri Olsa]

On Thu, May 19, 2022 at 12:43:18PM -0700, Yonghong Song wrote:
> 
> 
> On 5/19/22 11:14 AM, Eugene Syromiatnikov wrote:
> > With the interface as defined, it is impossible to pass 64-bit kernel
> > addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI,
> > which severly limits the useability of the interface, change the API
> > to accept an array of u64 values instead of (kernel? user?) longs.
> > This patch implements the user space part of the change (without
> > the relevant kernel changes, since, as of now, an attempt to add
> > kprobe_multi link will fail with -EOPNOTSUPP), to avoid changing
> > the interface after a release.
> > 
> > Fixes: 5117c26e877352bc ("libbpf: Add bpf_link_create support for multi kprobes")
> > Fixes: ddc6b04989eb0993 ("libbpf: Add bpf_program__attach_kprobe_multi_opts function")
> > Fixes: f7a11eeccb111854 ("selftests/bpf: Add kprobe_multi attach test")
> > Fixes: 9271a0c7ae7a9147 ("selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts")
> > Fixes: 2c6401c966ae1fbe ("selftests/bpf: Add kprobe_multi bpf_cookie test")
> > Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> > ---
> >   tools/lib/bpf/bpf.h                                        | 2 +-
> >   tools/lib/bpf/libbpf.c                                     | 8 ++++----
> >   tools/lib/bpf/libbpf.h                                     | 2 +-
> >   tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        | 2 +-
> >   tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 8 ++++----
> >   5 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > index f4b4afb..f677602 100644
> > --- a/tools/lib/bpf/bpf.h
> > +++ b/tools/lib/bpf/bpf.h
> > @@ -417,7 +417,7 @@ struct bpf_link_create_opts {
> >   			__u32 flags;
> >   			__u32 cnt;
> >   			const char **syms;
> > -			const unsigned long *addrs;
> > +			const __u64 *addrs;
> 
> Patch 2 and 3 will prevent supporting 64-bit kernel, 32-bit userspace for
> kprobe_multi. So effectively, kprobe_multi only supports
> 64-bit kernel and 64-bit user space.
> This is definitely an option, but it would be great
> if other people can chime in as well for whether this choice
> is best or not.

IIUC patch 3 is preparation for kernel change that will enable 32 bit
userspace and 64 bit kernel, and Eugene will send it later

sounds good to me

jirka

> 
> >   			const __u64 *cookies;
> >   		} kprobe_multi;
> >   	};
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 809fe20..03a14a6 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -10279,7 +10279,7 @@ static bool glob_match(const char *str, const char *pat)
> >   struct kprobe_multi_resolve {
> >   	const char *pattern;
> > -	unsigned long *addrs;
> > +	__u64 *addrs;
> >   	size_t cap;
> >   	size_t cnt;
> >   };
> > @@ -10294,12 +10294,12 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> >   	if (!glob_match(sym_name, res->pattern))
> >   		return 0;
> > -	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> > +	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(__u64),
> >   				res->cnt + 1);
> >   	if (err)
> >   		return err;
> > -	res->addrs[res->cnt++] = (unsigned long) sym_addr;
> > +	res->addrs[res->cnt++] = sym_addr;
> >   	return 0;
> >   }
> > @@ -10314,7 +10314,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >   	};
> >   	struct bpf_link *link = NULL;
> >   	char errmsg[STRERR_BUFSIZE];
> > -	const unsigned long *addrs;
> > +	const __u64 *addrs;
> >   	int err, link_fd, prog_fd;
> >   	const __u64 *cookies;
> >   	const char **syms;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 05dde85..ec1cb61 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -431,7 +431,7 @@ struct bpf_kprobe_multi_opts {
> >   	/* array of function symbols to attach */
> >   	const char **syms;
> >   	/* array of function addresses to attach */
> > -	const unsigned long *addrs;
> > +	const __u64 *addrs;
> >   	/* array of user-provided values fetchable through bpf_get_attach_cookie */
> >   	const __u64 *cookies;
> >   	/* number of elements in syms/addrs/cookies arrays */
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > index 923a613..5aa482a 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > @@ -137,7 +137,7 @@ static void kprobe_multi_link_api_subtest(void)
> >   	cookies[6] = 7;
> >   	cookies[7] = 8;
> > -	opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
> > +	opts.kprobe_multi.addrs = (const __u64 *) &addrs;
> >   	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> >   	opts.kprobe_multi.cookies = (const __u64 *) &cookies;
> >   	prog_fd = bpf_program__fd(skel->progs.test_kprobe);
> > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > index b9876b5..fbf4cf2 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > @@ -105,7 +105,7 @@ static void test_link_api_addrs(void)
> >   	GET_ADDR("bpf_fentry_test7", addrs[6]);
> >   	GET_ADDR("bpf_fentry_test8", addrs[7]);
> > -	opts.kprobe_multi.addrs = (const unsigned long*) addrs;
> > +	opts.kprobe_multi.addrs = (const __u64 *) addrs;
> >   	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> >   	test_link_api(&opts);
> >   }
> > @@ -183,7 +183,7 @@ static void test_attach_api_addrs(void)
> >   	GET_ADDR("bpf_fentry_test7", addrs[6]);
> >   	GET_ADDR("bpf_fentry_test8", addrs[7]);
> > -	opts.addrs = (const unsigned long *) addrs;
> > +	opts.addrs = (const __u64 *) addrs;
> >   	opts.cnt = ARRAY_SIZE(addrs);
> >   	test_attach_api(NULL, &opts);
> >   }
> > @@ -241,7 +241,7 @@ static void test_attach_api_fails(void)
> >   		goto cleanup;
> >   	/* fail_2 - both addrs and syms set */
> > -	opts.addrs = (const unsigned long *) addrs;
> > +	opts.addrs = (const __u64 *) addrs;
> >   	opts.syms = syms;
> >   	opts.cnt = ARRAY_SIZE(syms);
> >   	opts.cookies = NULL;
> > @@ -255,7 +255,7 @@ static void test_attach_api_fails(void)
> >   		goto cleanup;
> >   	/* fail_3 - pattern and addrs set */
> > -	opts.addrs = (const unsigned long *) addrs;
> > +	opts.addrs = (const __u64 *) addrs;
> >   	opts.syms = NULL;
> >   	opts.cnt = ARRAY_SIZE(syms);
> >   	opts.cookies = NULL;

Re: [PATCH bpf v4 0/3] Fix kprobe_multi interface issues for 5.18

[Alexei Starovoitov]

On Thu, May 19, 2022 at 11:14 AM Eugene Syromiatnikov <esyr@redhat.com> wrote:
>
> Hello.
>
> While [1] seems to require additional work[2] due to changes
> in the interface (and it has already been re-targeted for bpf-next),
> I would like to ask to consider the following three patches, that fix
> possible out-of-bounds write, properly disable the interface
> for 32-bit compat user space, and prepare the libbpf interface change,
> for the 5.18 release.  Thank you.

5.19 is imminent. All fixes should go into bpf-next and backported later.