From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C9A283446DA for ; Thu, 12 Feb 2026 11:29:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770895750; cv=none; b=kr/Wk1U+ao+8Y3nkEBwfBExtApmMg2y3d4cdGbC6ind7cf3xCyC6inbBvqvodOpmldKMXlcK281Dbagh1JvaDqZ78GvBK0gYknXkEeAasOJmw9s/z4YjXecM8Vqtvn4JQJee7w5JOzYQ5u91vF/JEbSkRIccld2UaXAtFbx515c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770895750; c=relaxed/simple; bh=Gx1vvXbcDJDnpJiOrm7MOVp20kNBtF3rmekpgGStkxw=; h=From:Date:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=q9I/Y5bAQTr/xrCdyudbQzHnKXeIjY1MA4WLwepVpe/uE9A5V+oIpNGOam+9VnXGri6LWoe+l3NbrvT9fQd419tXFMcs06eKsiBxjZ6X1lpV3JKY2WMRueYHYc7tabjOkLntNSXwo7n6GY3lbWniwxLeX/lV1RpbBSazYHi/RPc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=VZbRX4Xx; arc=none smtp.client-ip=209.85.221.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="VZbRX4Xx" Received: by mail-wr1-f43.google.com with SMTP id ffacd0b85a97d-4375d4fb4d4so2248759f8f.0 for ; Thu, 12 Feb 2026 03:29:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1770895747; x=1771500547; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:from:to:cc:subject:date:message-id:reply-to; bh=E8/mcInWOKOyu54erChwZBT6B8PNIkwl9baqwd32+Xc=; b=VZbRX4XxPNDHqoczjsGF8eJGrgKFSSLftM8nJkGKdnZ3KzVnD/JRKONWRdKpRo+llh 5m/+quYgS2TlYjNq8WagAGZjn1uupGXFgYqkgPTw3Ae/5K7kuWOjYxJ1G/NdYK8hOsaK A2hddi0Hjw//QnUtST8URGXrHuXU6Sy34wbhEcaxS5jA0QeK4/evXcxeZc8Zb1TkVL2z 10GrY8Y7IvJpJh6bCMqX74LYEZRbUYhO5fmdi2XAi41/yGJhQkbUmJayUPfTQxqd/NI5 JVxcSaxbcnB86UTIwoQ753Q3Zn8L/2Ds3VwiU920A+mDFCk8V3O/8/CkBoD5o2t7h1TP Pzlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770895747; x=1771500547; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=E8/mcInWOKOyu54erChwZBT6B8PNIkwl9baqwd32+Xc=; b=Xq6ITGvBlOu3nz8/MtTEMLLcgPI3Rv+eTMU6iICJxYWb3v4F+O2ZKIfQGI5ph6MTsW gRhNTzysSqSt2z0Nle0IhTTiup7QMxw7/0cKW1/g+YF98MUlceOeu+uLi4rAN/z3u/+g TbQt5nJ0nMj8bnslodibb++jnlecamSqiF8O7uXsw9WdypMEw97j++DKB+nAjYpx3Gpy yP3aOiE69ktaDP/l6zvdv6uxMNF6kPk+PV41Avpbi7wPOU1WBysyGnA7WVIvy/HQ8B+w x0UDxOMoR9u0nwtaeW7AVbYlJWTnNMAWdhpLIBm6CSt4vJI4yOnLwEI2E5/F9dXx1tGi hklQ== X-Forwarded-Encrypted: i=1; AJvYcCV28SiJa1ny8zmI8YLxKSQ5Lc1dSGldhGHU1KokYFLGpsF/9UI3DD5OEEYS9pl+gZi3CA1TTCm8BfhzIq0=@vger.kernel.org X-Gm-Message-State: AOJu0Yx07Xa1rmxZCjZnfB9aaxfxegX499g11M+VaOep42bkLPAWU+Q+ 8AJsyKyKP6DA0Ox6fbUWoSV1F1yxdaFOuFXgVDM7SU1Tev/azGEqOVUX X-Gm-Gg: AZuq6aL4sPSukydYqEr/SNB9mrZGU1aebfcZjc7s3KAfDHezfh9OWzUejakk2RCpNi1 ncYcgUQIOjctYYGPjHylsbcgXwhnFkPVM15JAowwwnuHsnSniDHrxtA73zNZMNeKt3VXb9BxAs7 gxkg8X35Nsdr1MsRuak9omH/mpPDVW+AtR8YELxsTHQsdb42UI0hxtZD9eSGTywEuSAJpxUmgCK lD1AENVyJs5F+eH4ZmxUgw8C2WDARoQUrw1NgrMaW0sYOC7+RSvJ5sVnOejwpbNQbgAtZr6Mj1E kH/4Xd0F//xSTMzGRHBvsgZWrzbqJtMzfBLjX12MyU2Gd8AIxaZdATzHgLMsDjcxfFaP0i5TFNe TPSv2Xu4g1C7ChScd2EIcX4pI0jog9mNCPICnC9hGaLpPdkGoUlp10ufxGNAmwljWP7fpO04+ X-Received: by 2002:a05:600c:470e:b0:479:3a86:dc1c with SMTP id 5b1f17b1804b1-4836717ec41mr27935195e9.36.1770895746931; Thu, 12 Feb 2026 03:29:06 -0800 (PST) Received: from krava ([2a02:8308:a00c:e200::b44f]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4835dcfafcdsm177464745e9.9.2026.02.12.03.29.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Feb 2026 03:29:06 -0800 (PST) From: Jiri Olsa X-Google-Original-From: Jiri Olsa Date: Thu, 12 Feb 2026 12:29:04 +0100 To: Ihor Solodrai Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Eduard Zingerman , Amery Hung , Mykyta Yatsenko , Alexis =?iso-8859-1?Q?Lothor=E9?= , bpf@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH bpf-next v1 04/14] selftests/bpf: Refactor bpf_get_ksyms() trace helper Message-ID: References: <20260212011356.3266753-1-ihor.solodrai@linux.dev> <20260212011356.3266753-5-ihor.solodrai@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260212011356.3266753-5-ihor.solodrai@linux.dev> On Wed, Feb 11, 2026 at 05:13:46PM -0800, Ihor Solodrai wrote: > ASAN reported a memory leak in bpf_get_ksyms(): it allocates a struct > ksyms internally and never frees it. > > Moe struct ksyms to trace_helpers.h and return it from the > bpf_get_ksyms(), giving ownership to the caller. Add filtered_syms and > filtered_cnt fields to the ksyms to hold the filtered array of > symbols, previously returned by bpf_get_ksyms(). > > Fixup the call sites: kprobe_multi_test and bench_trigger. > > Signed-off-by: Ihor Solodrai > --- > .../selftests/bpf/benchs/bench_trigger.c | 9 ++++---- > .../bpf/prog_tests/kprobe_multi_test.c | 12 ++++------ > tools/testing/selftests/bpf/trace_helpers.c | 23 ++++++++++--------- > tools/testing/selftests/bpf/trace_helpers.h | 11 +++++++-- > 4 files changed, 30 insertions(+), 25 deletions(-) > > diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c > index aeec9edd3851..7231b88cf21a 100644 > --- a/tools/testing/selftests/bpf/benchs/bench_trigger.c > +++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c > @@ -230,8 +230,7 @@ static void trigger_fentry_setup(void) > static void attach_ksyms_all(struct bpf_program *empty, bool kretprobe) > { > LIBBPF_OPTS(bpf_kprobe_multi_opts, opts); > - char **syms = NULL; > - size_t cnt = 0; > + struct ksyms *ksyms = NULL; > > /* Some recursive functions will be skipped in > * bpf_get_ksyms -> skip_entry, as they can introduce sufficient > @@ -241,13 +240,13 @@ static void attach_ksyms_all(struct bpf_program *empty, bool kretprobe) > * So, don't run the kprobe-multi-all and kretprobe-multi-all on > * a debug kernel. > */ > - if (bpf_get_ksyms(&syms, &cnt, true)) { > + if (bpf_get_ksyms(&ksyms, true)) { > fprintf(stderr, "failed to get ksyms\n"); > exit(1); > } > > - opts.syms = (const char **) syms; > - opts.cnt = cnt; > + opts.syms = (const char **)ksyms->filtered_syms; > + opts.cnt = ksyms->filtered_cnt; > opts.retprobe = kretprobe; > /* attach empty to all the kernel functions except bpf_get_numa_node_id. */ > if (!bpf_program__attach_kprobe_multi_opts(empty, NULL, &opts)) { hi, missing free_kallsyms_local call in here ? > 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 9caef222e528..f81dcd609ee9 100644 > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c > @@ -456,25 +456,23 @@ static void test_kprobe_multi_bench_attach(bool kernel) > { > LIBBPF_OPTS(bpf_kprobe_multi_opts, opts); > struct kprobe_multi_empty *skel = NULL; > - char **syms = NULL; > - size_t cnt = 0; > + struct ksyms *ksyms = NULL; > > - if (!ASSERT_OK(bpf_get_ksyms(&syms, &cnt, kernel), "bpf_get_ksyms")) > + if (!ASSERT_OK(bpf_get_ksyms(&ksyms, kernel), "bpf_get_ksyms")) > return; > > skel = kprobe_multi_empty__open_and_load(); > if (!ASSERT_OK_PTR(skel, "kprobe_multi_empty__open_and_load")) > goto cleanup; > > - opts.syms = (const char **) syms; > - opts.cnt = cnt; > + opts.syms = (const char **)ksyms->filtered_syms; > + opts.cnt = ksyms->filtered_cnt; > > do_bench_test(skel, &opts); > > cleanup: > kprobe_multi_empty__destroy(skel); > - if (syms) > - free(syms); > + free_kallsyms_local(ksyms); > } > > static void test_kprobe_multi_bench_attach_addr(bool kernel) > diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c > index eeaab7013ca2..0e63daf83ed5 100644 > --- a/tools/testing/selftests/bpf/trace_helpers.c > +++ b/tools/testing/selftests/bpf/trace_helpers.c > @@ -24,12 +24,6 @@ > #define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe" > #define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe" > > -struct ksyms { > - struct ksym *syms; > - size_t sym_cap; > - size_t sym_cnt; > -}; > - > static struct ksyms *ksyms; > static pthread_mutex_t ksyms_mutex = PTHREAD_MUTEX_INITIALIZER; > > @@ -54,6 +48,8 @@ void free_kallsyms_local(struct ksyms *ksyms) > if (!ksyms) > return; > > + free(ksyms->filtered_syms); > + > if (!ksyms->syms) { > free(ksyms); > return; > @@ -610,7 +606,7 @@ static int search_kallsyms_compare(const void *p1, const struct ksym *p2) > return compare_name(p1, p2->name); > } > > -int bpf_get_ksyms(char ***symsp, size_t *cntp, bool kernel) > +int bpf_get_ksyms(struct ksyms **ksymsp, bool kernel) > { > size_t cap = 0, cnt = 0; > char *name = NULL, *ksym_name, **syms = NULL; > @@ -637,8 +633,10 @@ int bpf_get_ksyms(char ***symsp, size_t *cntp, bool kernel) > else > f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r"); > > - if (!f) > + if (!f) { > + free_kallsyms_local(ksyms); > return -EINVAL; > + } > > map = hashmap__new(symbol_hash, symbol_equal, NULL); > if (IS_ERR(map)) { > @@ -679,15 +677,18 @@ int bpf_get_ksyms(char ***symsp, size_t *cntp, bool kernel) > syms[cnt++] = ksym_name; > } > > - *symsp = syms; > - *cntp = cnt; > + ksyms->filtered_syms = syms; > + ksyms->filtered_cnt = cnt; > + *ksymsp = ksyms; > > error: > free(name); > fclose(f); > hashmap__free(map); > - if (err) > + if (err) { > free(syms); > + free_kallsyms_local(ksyms); > + } I think we could just call free_kallsyms_local unconditionally in here and fix callers to free syms pointer? seems easier than adding filtered* fields to ksyms thanks, jirak > return err; > } > > diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h > index a5576b2dfc26..d5bf1433675d 100644 > --- a/tools/testing/selftests/bpf/trace_helpers.h > +++ b/tools/testing/selftests/bpf/trace_helpers.h > @@ -23,7 +23,14 @@ struct ksym { > long addr; > char *name; > }; > -struct ksyms; > + > +struct ksyms { > + struct ksym *syms; > + size_t sym_cap; > + size_t sym_cnt; > + char **filtered_syms; > + size_t filtered_cnt; > +}; > > typedef int (*ksym_cmp_t)(const void *p1, const void *p2); > typedef int (*ksym_search_cmp_t)(const void *p1, const struct ksym *p2); > @@ -53,7 +60,7 @@ ssize_t get_rel_offset(uintptr_t addr); > > int read_build_id(const char *path, char *build_id, size_t size); > > -int bpf_get_ksyms(char ***symsp, size_t *cntp, bool kernel); > +int bpf_get_ksyms(struct ksyms **ksymsp, bool kernel); > int bpf_get_addrs(unsigned long **addrsp, size_t *cntp, bool kernel); > > #endif > -- > 2.53.0 > >