linux-kernel - Re: [PATCH 2/3] perf: collect BPF metadata from existing BPF programs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aD9sxuFwwxwHGzNi@google.com>
Date: Tue, 3 Jun 2025 14:44:38 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Blake Jones <blakejones@...gle.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Andrii Nakryiko <andrii@...nel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>,
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
	Yonghong Song <yonghong.song@...ux.dev>,
	John Fastabend <john.fastabend@...il.com>,
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
	Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Chun-Tse Shao <ctshao@...gle.com>,
	Zhongqiu Han <quic_zhonhan@...cinc.com>,
	James Clark <james.clark@...aro.org>,
	Charlie Jenkins <charlie@...osinc.com>,
	Andi Kleen <ak@...ux.intel.com>, Dmitry Vyukov <dvyukov@...gle.com>,
	Leo Yan <leo.yan@....com>, Yujie Liu <yujie.liu@...el.com>,
	Graham Woodward <graham.woodward@....com>,
	Yicong Yang <yangyicong@...ilicon.com>,
	Ben Gainey <ben.gainey@....com>, linux-kernel@...r.kernel.org,
	bpf@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 2/3] perf: collect BPF metadata from existing BPF programs

Hi Blake,

On Tue, Jun 03, 2025 at 02:27:53PM -0700, Blake Jones wrote:
> Hi Namhyung,
> 
> On Tue, Jun 3, 2025 at 1:15 PM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > On Wed, May 21, 2025 at 03:27:24PM -0700, Blake Jones wrote:
> > > Look for .rodata maps, find ones with 'bpf_metadata_' variables, extract
> > > their values as strings, and create a new PERF_RECORD_BPF_METADATA
> > > synthetic event using that data. The code gets invoked from the existing
> > > routine perf_event__synthesize_one_bpf_prog().
> >
> > It would be great if you can show an example how those metadata is
> > constructed and shared between BPF programs.
> 
> I've added the following to my commit message:
> 
> | For example, a BPF program with the following variables:
> |
> |     const char bpf_metadata_version[] SEC(".rodata") = "3.14159";
> |     int bpf_metadata_value[] SEC(".rodata") = 42;
> |
> | would generate a PERF_RECORD_BPF_METADATA record with:
> |
> |     .prog_name        = <BPF program name, e.g. "bpf_prog_a1b2c3_foo">
> |     .nr_entries       = 2
> |     .entries[0].key   = "version"
> |     .entries[0].value = "3.14159"
> |     .entries[1].key   = "value"
> |     .entries[1].value = "42"
> |
> | Each of the BPF programs and subprograms that share those variables would
> | get a distinct PERF_RECORD_BPF_METADATA record, with the ".prog_name" showing
> | the name of each program or subprogram. The prog_name is deliberately the
> | same as the ".name" field in the corresponding PERF_RECORD_KSYMBOL record.

Thanks!

> 
> > IIUC the metadata is collected for each BPF program which may have
> > multiple subprograms.  Then this patch creates multiple PERF_RECORD_
> > BPF_METADATA for each subprogram, right?
> >
> > Can it be shared using the BPF program ID?
> 
> In theory, yes, it could be shared. But I want to be able to correlate them
> with the corresponding PERF_RECORD_KSYMBOL events, and KSYMBOL events for
> subprograms don't have the full-program ID, so I wouldn't be able to do that.

It's unfortunate that KSYMBOL doesn't have the program ID, but IIRC the
following BPF_EVENT should have it.  I think it's safe to think KSYMBOLs
belong to the BPF_EVENT when they are from the same thread.

> 
> > > +     rodata = calloc(1, map_info.value_size);
> >
> > You can use 'zalloc()' instead, in other places too.
> 
> Fixed, thanks.
> 
> > > +void bpf_metadata_free(struct bpf_metadata *metadata)
> > > +{
> > > +     if (metadata == NULL)
> > > +             return;
> > > +     for (__u32 index = 0; index < metadata->nr_prog_names; index++)
> > > +             free(metadata->prog_names[index]);
> > > +     if (metadata->prog_names != NULL)
> > > +             free(metadata->prog_names);
> > > +     if (metadata->event != NULL)
> > > +             free(metadata->event);
> >
> > No need to NULL change for free().
> 
> I've removed the NULL checks.
> 
> > > +static int synthesize_perf_record_bpf_metadata(
> > > [...]
> > > +     for (__u32 index = 0; index < metadata->nr_prog_names; index++) {
> > > +             memcpy(event->bpf_metadata.prog_name,
> > > +                    metadata->prog_names[index], BPF_PROG_NAME_LEN);
> >
> > Is it possible to call synthesize_bpf_prog_name() directly to the
> > event->bpf_metadata.prog_name instead of saving it metadata->prog_names?
> 
> Not with the way the code is currently structured - we need the BTF data
> to call synthesize_bpf_prog_name(), and that's allocated and freed inside
> of bpf_metadata_create().

I see.  You already freed the map data and BTF.  Ok, it's not a big deal
and probably not needed if we can switch to BPF ID.

Thanks,
Namhyung