| 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
| ||
|
Message-ID: <CAP-5=fVqkR7Xd8GXw31MPNSsfvkiPVavQjQSieWUpKasHcLVQQ@mail.gmail.com>
Date: Mon, 19 Aug 2024 12:54:36 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
Masahiro Yamada <masahiroy@...nel.org>, Arnd Bergmann <arnd@...db.de>, Jann Horn <jannh@...gle.com>,
Colin Ian King <colin.i.king@...il.com>, Casey Chen <cachen@...estorage.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
Chaitanya S Prakash <chaitanyas.prakash@....com>, James Clark <james.clark@...aro.org>,
Ze Gao <zegao2021@...il.com>, Yang Jihong <yangjihong1@...wei.com>,
Yunseong Kim <yskelg@...il.com>, Weilin Wang <weilin.wang@...el.com>,
Dominique Martinet <asmadeus@...ewreck.org>, Anne Macedo <retpolanne@...teo.net>,
Sun Haiyong <sunhaiyong@...ngson.cn>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 10/13] perf inject: Fix build ID injection
On Mon, Aug 19, 2024 at 11:01 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> On Fri, Aug 16, 2024 at 11:44:39PM -0700, Ian Rogers wrote:
> > Build ID injection wasn't inserting a sample ID and aligning events to
> > 64 bytes rather than 8. No sample ID means events are unordered and
> > two different build_id events for the same path, as happens when a
> > file is replaced, can't be differentiated.
> >
> > Add in sample ID insertion for the build_id events alongside some
> > refactoring. The refactoring better aligns the function arguments for
> > different use cases, such as synthesizing build_id events without
> > needing to have a dso. The misc bits are explicitly passed as with
> > callchains the maps/dsos may span user and kernel land, so using
> > sample->cpumode isn't good enough.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/builtin-inject.c | 170 ++++++++++++++++++++++-------
> > tools/perf/util/build-id.c | 6 +-
> > tools/perf/util/synthetic-events.c | 44 ++++++--
> > tools/perf/util/synthetic-events.h | 10 +-
> > 4 files changed, 175 insertions(+), 55 deletions(-)
> >
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index a7c859db2e15..84a4bdb5cb0a 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -131,6 +131,7 @@ struct perf_inject {
> > struct perf_file_section secs[HEADER_FEAT_BITS];
> > struct guest_session guest_session;
> > struct strlist *known_build_ids;
> > + const struct evsel *mmap_evsel;
> > };
> >
> > struct event_entry {
> > @@ -139,8 +140,13 @@ struct event_entry {
> > union perf_event event[];
> > };
> >
> > -static int dso__inject_build_id(struct dso *dso, const struct perf_tool *tool,
> > - struct machine *machine, u8 cpumode, u32 flags);
> > +static int dso__inject_build_id(const struct perf_tool *tool,
> > + struct perf_sample *sample,
> > + struct machine *machine,
> > + const struct evsel *evsel,
> > + __u16 misc,
> > + const char *filename,
> > + struct dso *dso, u32 flags);
>
> So in the end the dso was needed, the name of the function remains
> dso__something(), so first arg would be a 'struct dso *'
>
> I processed the patches up to 9/13, so that they can get tested now.
Maybe we should rename the function? We can get the build ID from
mmap2 events now, not just stored away in the dso by build_id events.
Reordering the arguments isn't a problem, I was just aiming for
consistency between the caller, perf_event__synthesize_build_id and
eventually the call to process that all take the arguments in this
order.
Thanks,
Ian
> - Arnaldo
>
> >
> > static int output_bytes(struct perf_inject *inject, void *buf, size_t sz)
> > {
> > @@ -422,6 +428,28 @@ static struct dso *findnew_dso(int pid, int tid, const char *filename,
> > return dso;
> > }
> >
> > +/*
> > + * The evsel used for the sample ID for mmap events. Typically stashed when
> > + * processing mmap events. If not stashed, search the evlist for the first mmap
> > + * gathering event.
> > + */
> > +static const struct evsel *inject__mmap_evsel(struct perf_inject *inject)
> > +{
> > + struct evsel *pos;
> > +
> > + if (inject->mmap_evsel)
> > + return inject->mmap_evsel;
> > +
> > + evlist__for_each_entry(inject->session->evlist, pos) {
> > + if (pos->core.attr.mmap) {
> > + inject->mmap_evsel = pos;
> > + return pos;
> > + }
> > + }
> > + pr_err("No mmap events found\n");
> > + return NULL;
> > +}
> > +
> > static int perf_event__repipe_common_mmap(const struct perf_tool *tool,
> > union perf_event *event,
> > struct perf_sample *sample,
> > @@ -469,12 +497,28 @@ static int perf_event__repipe_common_mmap(const struct perf_tool *tool,
> > }
> >
> > if (dso && !dso__hit(dso)) {
> > - dso__set_hit(dso);
> > - dso__inject_build_id(dso, tool, machine, sample->cpumode, flags);
> > + struct evsel *evsel = evlist__event2evsel(inject->session->evlist, event);
> > +
> > + if (evsel) {
> > + dso__set_hit(dso);
> > + dso__inject_build_id(tool, sample, machine, evsel,
> > + /*misc=*/sample->cpumode,
> > + filename, dso, flags);
> > + }
> > }
> > } else {
> > + int err;
> > +
> > + /*
> > + * Remember the evsel for lazy build id generation. It is used
> > + * for the sample id header type.
> > + */
> > + if (inject->build_id_style == BID_RWS__INJECT_HEADER_LAZY &&
> > + !inject->mmap_evsel)
> > + inject->mmap_evsel = evlist__event2evsel(inject->session->evlist, event);
> > +
> > /* Create the thread, map, etc. Not done for the unordered inject all case. */
> > - int err = perf_event_process(tool, event, sample, machine);
> > + err = perf_event_process(tool, event, sample, machine);
> >
> > if (err) {
> > dso__put(dso);
> > @@ -667,16 +711,20 @@ static bool perf_inject__lookup_known_build_id(struct perf_inject *inject,
> > return false;
> > }
> >
> > -static int dso__inject_build_id(struct dso *dso, const struct perf_tool *tool,
> > - struct machine *machine, u8 cpumode, u32 flags)
> > +static int dso__inject_build_id(const struct perf_tool *tool,
> > + struct perf_sample *sample,
> > + struct machine *machine,
> > + const struct evsel *evsel,
> > + __u16 misc,
> > + const char *filename,
> > + struct dso *dso, u32 flags)
> > {
> > - struct perf_inject *inject = container_of(tool, struct perf_inject,
> > - tool);
> > + struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
> > int err;
> >
> > - if (is_anon_memory(dso__long_name(dso)) || flags & MAP_HUGETLB)
> > + if (is_anon_memory(filename) || flags & MAP_HUGETLB)
> > return 0;
> > - if (is_no_dso_memory(dso__long_name(dso)))
> > + if (is_no_dso_memory(filename))
> > return 0;
> >
> > if (inject->known_build_ids != NULL &&
> > @@ -684,24 +732,65 @@ static int dso__inject_build_id(struct dso *dso, const struct perf_tool *tool,
> > return 1;
> >
> > if (dso__read_build_id(dso) < 0) {
> > - pr_debug("no build_id found for %s\n", dso__long_name(dso));
> > + pr_debug("no build_id found for %s\n", filename);
> > return -1;
> > }
> >
> > - err = perf_event__synthesize_build_id(tool, dso, cpumode,
> > - perf_event__repipe, machine);
> > + err = perf_event__synthesize_build_id(tool, sample, machine,
> > + perf_event__repipe,
> > + evsel, misc, dso__bid(dso),
> > + filename);
> > if (err) {
> > - pr_err("Can't synthesize build_id event for %s\n", dso__long_name(dso));
> > + pr_err("Can't synthesize build_id event for %s\n", filename);
> > return -1;
> > }
> >
> > return 0;
> > }
> >
> > +static int mark_dso_hit(const struct perf_tool *tool,
> > + struct perf_sample *sample,
> > + struct machine *machine,
> > + const struct evsel *mmap_evsel,
> > + struct map *map, bool sample_in_dso)
> > +{
> > + struct dso *dso;
> > + u16 misc = sample->cpumode;
> > +
> > + if (!map)
> > + return 0;
> > +
> > + if (!sample_in_dso) {
> > + u16 guest_mask = PERF_RECORD_MISC_GUEST_KERNEL |
> > + PERF_RECORD_MISC_GUEST_USER;
> > +
> > + if ((misc & guest_mask) != 0) {
> > + misc &= PERF_RECORD_MISC_HYPERVISOR;
> > + misc |= __map__is_kernel(map)
> > + ? PERF_RECORD_MISC_GUEST_KERNEL
> > + : PERF_RECORD_MISC_GUEST_USER;
> > + } else {
> > + misc &= PERF_RECORD_MISC_HYPERVISOR;
> > + misc |= __map__is_kernel(map)
> > + ? PERF_RECORD_MISC_KERNEL
> > + : PERF_RECORD_MISC_USER;
> > + }
> > + }
> > + dso = map__dso(map);
> > + if (dso && !dso__hit(dso)) {
> > + dso__set_hit(dso);
> > + dso__inject_build_id(tool, sample, machine,
> > + mmap_evsel, misc, dso__long_name(dso), dso,
> > + map__flags(map));
> > + }
> > + return 0;
> > +}
> > +
> > struct mark_dso_hit_args {
> > const struct perf_tool *tool;
> > + struct perf_sample *sample;
> > struct machine *machine;
> > - u8 cpumode;
> > + const struct evsel *mmap_evsel;
> > };
> >
> > static int mark_dso_hit_callback(struct callchain_cursor_node *node, void *data)
> > @@ -709,16 +798,8 @@ static int mark_dso_hit_callback(struct callchain_cursor_node *node, void *data)
> > struct mark_dso_hit_args *args = data;
> > struct map *map = node->ms.map;
> >
> > - if (map) {
> > - struct dso *dso = map__dso(map);
> > -
> > - if (dso && !dso__hit(dso)) {
> > - dso__set_hit(dso);
> > - dso__inject_build_id(dso, args->tool, args->machine,
> > - args->cpumode, map__flags(map));
> > - }
> > - }
> > - return 0;
> > + return mark_dso_hit(args->tool, args->sample, args->machine,
> > + args->mmap_evsel, map, /*sample_in_dso=*/false);
> > }
> >
> > int perf_event__inject_buildid(const struct perf_tool *tool, union perf_event *event,
> > @@ -728,10 +809,16 @@ int perf_event__inject_buildid(const struct perf_tool *tool, union perf_event *e
> > {
> > struct addr_location al;
> > struct thread *thread;
> > + struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
> > struct mark_dso_hit_args args = {
> > .tool = tool,
> > + /*
> > + * Use the parsed sample data of the sample event, which will
> > + * have a later timestamp than the mmap event.
> > + */
> > + .sample = sample,
> > .machine = machine,
> > - .cpumode = sample->cpumode,
> > + .mmap_evsel = inject__mmap_evsel(inject),
> > };
> >
> > addr_location__init(&al);
> > @@ -743,13 +830,8 @@ int perf_event__inject_buildid(const struct perf_tool *tool, union perf_event *e
> > }
> >
> > if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
> > - struct dso *dso = map__dso(al.map);
> > -
> > - if (!dso__hit(dso)) {
> > - dso__set_hit(dso);
> > - dso__inject_build_id(dso, tool, machine,
> > - sample->cpumode, map__flags(al.map));
> > - }
> > + mark_dso_hit(tool, sample, machine, args.mmap_evsel, al.map,
> > + /*sample_in_dso=*/true);
> > }
> >
> > sample__for_each_callchain_node(thread, evsel, sample, PERF_MAX_STACK_DEPTH,
> > @@ -1159,17 +1241,27 @@ static int process_build_id(const struct perf_tool *tool,
> > static int synthesize_build_id(struct perf_inject *inject, struct dso *dso, pid_t machine_pid)
> > {
> > struct machine *machine = perf_session__findnew_machine(inject->session, machine_pid);
> > - u8 cpumode = dso__is_in_kernel_space(dso) ?
> > - PERF_RECORD_MISC_GUEST_KERNEL :
> > - PERF_RECORD_MISC_GUEST_USER;
> > + struct perf_sample synth_sample = {
> > + .pid = -1,
> > + .tid = -1,
> > + .time = -1,
> > + .stream_id = -1,
> > + .cpu = -1,
> > + .period = 1,
> > + .cpumode = dso__is_in_kernel_space(dso)
> > + ? PERF_RECORD_MISC_GUEST_KERNEL
> > + : PERF_RECORD_MISC_GUEST_USER,
> > + };
> >
> > if (!machine)
> > return -ENOMEM;
> >
> > dso__set_hit(dso);
> >
> > - return perf_event__synthesize_build_id(&inject->tool, dso, cpumode,
> > - process_build_id, machine);
> > + return perf_event__synthesize_build_id(&inject->tool, &synth_sample, machine,
> > + process_build_id, inject__mmap_evsel(inject),
> > + /*misc=*/synth_sample.cpumode,
> > + dso__bid(dso), dso__long_name(dso));
> > }
> >
> > static int guest_session__add_build_ids_cb(struct dso *dso, void *data)
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index 451d145fa4ed..8982f68e7230 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -277,8 +277,8 @@ static int write_buildid(const char *name, size_t name_len, struct build_id *bid
> > struct perf_record_header_build_id b;
> > size_t len;
> >
> > - len = name_len + 1;
> > - len = PERF_ALIGN(len, NAME_ALIGN);
> > + len = sizeof(b) + name_len + 1;
> > + len = PERF_ALIGN(len, sizeof(u64));
> >
> > memset(&b, 0, sizeof(b));
> > memcpy(&b.data, bid->data, bid->size);
> > @@ -286,7 +286,7 @@ static int write_buildid(const char *name, size_t name_len, struct build_id *bid
> > misc |= PERF_RECORD_MISC_BUILD_ID_SIZE;
> > b.pid = pid;
> > b.header.misc = misc;
> > - b.header.size = sizeof(b) + len;
> > + b.header.size = len;
> >
> > err = do_write(fd, &b, sizeof(b));
> > if (err < 0)
> > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > index 0a7f93ae76fb..6bb62e4e2d5d 100644
> > --- a/tools/perf/util/synthetic-events.c
> > +++ b/tools/perf/util/synthetic-events.c
> > @@ -2225,28 +2225,48 @@ int perf_event__synthesize_tracing_data(const struct perf_tool *tool, int fd, st
> > }
> > #endif
> >
> > -int perf_event__synthesize_build_id(const struct perf_tool *tool, struct dso *pos, u16 misc,
> > - perf_event__handler_t process, struct machine *machine)
> > +int perf_event__synthesize_build_id(const struct perf_tool *tool,
> > + struct perf_sample *sample,
> > + struct machine *machine,
> > + perf_event__handler_t process,
> > + const struct evsel *evsel,
> > + __u16 misc,
> > + const struct build_id *bid,
> > + const char *filename)
> > {
> > union perf_event ev;
> > size_t len;
> >
> > - if (!dso__hit(pos))
> > - return 0;
> > + len = sizeof(ev.build_id) + strlen(filename) + 1;
> > + len = PERF_ALIGN(len, sizeof(u64));
> >
> > - memset(&ev, 0, sizeof(ev));
> > + memset(&ev, 0, len);
> >
> > - len = dso__long_name_len(pos) + 1;
> > - len = PERF_ALIGN(len, NAME_ALIGN);
> > - ev.build_id.size = min(dso__bid(pos)->size, sizeof(dso__bid(pos)->data));
> > - memcpy(&ev.build_id.build_id, dso__bid(pos)->data, ev.build_id.size);
> > + ev.build_id.size = min(bid->size, sizeof(ev.build_id.build_id));
> > + memcpy(ev.build_id.build_id, bid->data, ev.build_id.size);
> > ev.build_id.header.type = PERF_RECORD_HEADER_BUILD_ID;
> > ev.build_id.header.misc = misc | PERF_RECORD_MISC_BUILD_ID_SIZE;
> > ev.build_id.pid = machine->pid;
> > - ev.build_id.header.size = sizeof(ev.build_id) + len;
> > - memcpy(&ev.build_id.filename, dso__long_name(pos), dso__long_name_len(pos));
> > + ev.build_id.header.size = len;
> > + strcpy(ev.build_id.filename, filename);
> > +
> > + if (evsel) {
> > + void *array = &ev;
> > + int ret;
> >
> > - return process(tool, &ev, NULL, machine);
> > + array += ev.header.size;
> > + ret = perf_event__synthesize_id_sample(array, evsel->core.attr.sample_type, sample);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (ret & 7) {
> > + pr_err("Bad id sample size %d\n", ret);
> > + return -EINVAL;
> > + }
> > +
> > + ev.header.size += ret;
> > + }
> > + return process(tool, &ev, sample, machine);
> > }
> >
> > int perf_event__synthesize_stat_events(struct perf_stat_config *config, const struct perf_tool *tool,
> > diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
> > index 31df7653677f..795bf3e18396 100644
> > --- a/tools/perf/util/synthetic-events.h
> > +++ b/tools/perf/util/synthetic-events.h
> > @@ -9,6 +9,7 @@
> > #include <perf/cpumap.h>
> >
> > struct auxtrace_record;
> > +struct build_id;
> > struct dso;
> > struct evlist;
> > struct evsel;
> > @@ -45,7 +46,14 @@ typedef int (*perf_event__handler_t)(const struct perf_tool *tool, union perf_ev
> >
> > int perf_event__synthesize_attrs(const struct perf_tool *tool, struct evlist *evlist, perf_event__handler_t process);
> > int perf_event__synthesize_attr(const struct perf_tool *tool, struct perf_event_attr *attr, u32 ids, u64 *id, perf_event__handler_t process);
> > -int perf_event__synthesize_build_id(const struct perf_tool *tool, struct dso *pos, u16 misc, perf_event__handler_t process, struct machine *machine);
> > +int perf_event__synthesize_build_id(const struct perf_tool *tool,
> > + struct perf_sample *sample,
> > + struct machine *machine,
> > + perf_event__handler_t process,
> > + const struct evsel *evsel,
> > + __u16 misc,
> > + const struct build_id *bid,
> > + const char *filename);
> > int perf_event__synthesize_cpu_map(const struct perf_tool *tool, const struct perf_cpu_map *cpus, perf_event__handler_t process, struct machine *machine);
> > int perf_event__synthesize_event_update_cpus(const struct perf_tool *tool, struct evsel *evsel, perf_event__handler_t process);
> > int perf_event__synthesize_event_update_name(const struct perf_tool *tool, struct evsel *evsel, perf_event__handler_t process);
> > --
> > 2.46.0.184.g6999bdac58-goog
Powered by blists - more mailing lists