Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] selftests/bpf: integer overflow of AREA_MAX on 32-bit in hold_mmap_lock() --
commit 27a041a03328fba58bd21d5e62e3d149445f0523 Author: Alexei Starovoitov <[email protected]> selftests/bpf: Test uprobes and fork() with busy mmap_lock This commit adds a BPF selftest that exercises uprobe attach and detach concurrently with fork() while a process holds its mmap_lock via MADV_POPULATE_READ. > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_busy_mm.c > b/tools/testing/selftests/bpf/prog_tests/uprobe_busy_mm.c > new file mode 100644 > index 0000000000000..30614e8b92cd5 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_busy_mm.c > @@ -0,0 +1,447 @@ [ ... ] > +#define AREA_MAX (64UL << 30) > +#define AREA_MIN (64UL << 20) [ ... ] > +/* > + * MADV_POPULATE_READ holds mmap_lock for reading while it maps zero page > + * all over the area. It costs page tables only. > + */ > +static void *hold_mmap_lock(void *arg) > +{ > + size_t size = AREA_MIN; > + void *area; > + __u64 t; > + > + area = mmap(NULL, AREA_MAX, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | > MAP_NORESERVE, -1, 0); > + if (area == MAP_FAILED) > + goto err; [Severity: Medium] In tools/testing/selftests/bpf/prog_tests/uprobe_busy_mm.c, does AREA_MAX overflow on 32-bit architectures when passed to mmap() in hold_mmap_lock()? When serial_test_uprobe_busy_mm() runs on a 32-bit system via: serial_test_uprobe_busy_mm() -> start_busy_process() -> busy_process() -> hold_mmap_lock() unsigned long and size_t are 32 bits wide, so (64UL << 30) overflows to 0 (and a 64 GiB area exceeds the 32-bit virtual address space). Passing 0 as the length to mmap() causes it to return MAP_FAILED with EINVAL, jumping to err and failing the test. Should AREA_MAX be sized appropriately for 32-bit architectures as well? > + /* EINVAL without CONFIG_TRANSPARENT_HUGEPAGE */ > + madvise(area, AREA_MAX, MADV_NOHUGEPAGE); > + > + t = now_ns(); > + if (madvise(area, size, MADV_POPULATE_READ)) > + goto err; > + t = now_ns() - t; > + size *= sh->hold_ms * 1000000ULL / t + 1; > + if (size > AREA_MAX) > + size = AREA_MAX; > + > + while (!LOAD(sh->stop)) { > + STORE(sh->in_hold, 1); > + t = now_ns(); > + madvise(area, size, MADV_POPULATE_READ); > + /* before anything that can fault and wait for fork() */ > + INC(sh->hold_seq); > + STORE(sh->hold_ns, now_ns() - t); > + STORE(sh->in_hold, 0); > + madvise(area, size, MADV_DONTNEED); > + STORE(sh->ready, 1); > + } > + return NULL; > +err: > + INC(sh->errors); > + STORE(sh->ready, 1); > + return NULL; > +} [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
