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.
Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
Signed-off-by: Eugene Syromiatnikov <esyr(a)redhat.com>
---
kernel/trace/bpf_trace.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d8553f4..e90c4ce7 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2358,6 +2358,8 @@ kprobe_multi_resolve_syms(const void __user *usyms, u32 cnt,
unsigned int i;
char *func;
+ if (check_mul_overflow(cnt, sizeof(*syms), &size))
+ return -EOVERFLOW;
size = cnt * sizeof(*syms);
syms = kvzalloc(size, GFP_KERNEL);
if (!syms)
@@ -2429,6 +2431,8 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
if (!cnt)
return -EINVAL;
+ if (check_mul_overflow(cnt, (u32)sizeof(*addrs), &size))
+ return -EOVERFLOW;
size = cnt * sizeof(*addrs);
addrs = kvmalloc(size, GFP_KERNEL);
if (!addrs)
--
2.1.4
As suggested in [1], the kprobe_multi interface is to be fixed for 32-bit
architectures and compat, rather then disabled. As it turned out,
there are a couple of additional problems that are to be addressed:
- the absence of size overflow checks, leading to possible
out-of-bounds writes (addressed by the first patch);
- the assumption that long has the same size as u64, which would make
cookies arrays size calculation incorrect on 32-bit architectures
(addressed by the second patch);
- the addrs array passing API, that is incompatible with compat and has
to be changed (addressed in the fourth patch): those are kernel
addresses and not user ones (as was incorrectly stated in [2]);
this change is only semantical for 64-bit user/kernelspace,
so it shouldn't impact ABI there, at least.
[1] https://lore.kernel.org/lkml/CAADnVQ+2gwhcMht4PuDnDOFKY68Wsq8QFz4Y69NBX_TLa…
[2] https://lore.kernel.org/lkml/20220510184155.GA8295@asgard.redhat.com/
v2:
- Fixed the isses reported by CI
v1: https://lore.kernel.org/lkml/20220516182657.GA28596@asgard.redhat.com/
Eugene Syromiatnikov (4):
bpf_trace: check size for overflow in bpf_kprobe_multi_link_attach
bpf_trace: support 32-bit kernels in bpf_kprobe_multi_link_attach
bpf_trace: handle compat in kprobe_multi_resolve_syms
bpf_trace: pass array of u64 values in kprobe_multi.addrs
kernel/trace/bpf_trace.c | 62 ++++++++++++++++------
tools/lib/bpf/bpf.h | 2 +-
tools/lib/bpf/libbpf.c | 8 +--
tools/lib/bpf/libbpf.h | 2 +-
.../testing/selftests/bpf/prog_tests/bpf_cookie.c | 2 +-
.../selftests/bpf/prog_tests/kprobe_multi_test.c | 8 +--
6 files changed, 56 insertions(+), 28 deletions(-)
--
2.1.4
The existing logic happens to work fine on UML, but is not correct when
running on other arches.
1. We didn't initialize `int err`, and kunit_filter_suites() doesn't
explicitly set it to 0 on success. So we had false "failures".
Note: it doesn't happen on UML, causing this to get overlooked.
2. If we error out, we do not call kunit_handle_shutdown().
This makes kunit.py timeout when using a non-UML arch, since the QEMU
process doesn't ever exit.
Fixes: a02353f49162 ("kunit: bail out of test filtering logic quicker if OOM")
Signed-off-by: Daniel Latypov <dlatypov(a)google.com>
---
lib/kunit/executor.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 2f73a6a35a7e..96f96e42ce06 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -247,13 +247,13 @@ int kunit_run_all_tests(void)
.start = __kunit_suites_start,
.end = __kunit_suites_end,
};
- int err;
+ int err = 0;
if (filter_glob_param) {
suite_set = kunit_filter_suites(&suite_set, filter_glob_param, &err);
if (err) {
pr_err("kunit executor: error filtering suites: %d\n", err);
- return err;
+ goto out;
}
}
@@ -268,9 +268,10 @@ int kunit_run_all_tests(void)
kunit_free_suite_set(suite_set);
}
- kunit_handle_shutdown();
- return 0;
+out:
+ kunit_handle_shutdown();
+ return err;
}
#if IS_BUILTIN(CONFIG_KUNIT_TEST)
base-commit: 9660209d9418f2295d31fea0d32e313e9b2c1200
--
2.36.0.550.gb090851708-goog
Improve the readability of error message which says module not found.
The new behaviour is consistent with the modprobe command.
Signed-off-by: Gautam Menghani <gautammenghani201(a)gmail.com>
---
tools/testing/selftests/ir/ir_loopback.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ir/ir_loopback.sh b/tools/testing/selftests/ir/ir_loopback.sh
index b90dc9939f45..aff9299c9416 100755
--- a/tools/testing/selftests/ir/ir_loopback.sh
+++ b/tools/testing/selftests/ir/ir_loopback.sh
@@ -10,7 +10,7 @@ if [ $UID != 0 ]; then
fi
if ! /sbin/modprobe -q -n rc-loopback; then
- echo "ir_loopback: module rc-loopback is not found [SKIP]"
+ echo "ir_loopback: module rc-loopback is not found in /lib/modules/`uname -r` [SKIP]"
exit $ksft_skip
fi
--
2.25.1