Re: [PATCH v1] perf synthetic-events: Fix stale build ID in module MMAP2 records

[Chuck Lever <cel@kernel.org>]

On 3/11/26 12:14 PM, Ian Rogers wrote:
> On Tue, Mar 10, 2026 at 12:32 PM Ian Rogers <irogers@google.com> wrote:
>>
>> On Tue, Mar 10, 2026 at 10:59 AM Chuck Lever <cel@kernel.org> wrote:
>>>
>>> From: Chuck Lever <chuck.lever@oracle.com>
>>>
>>> perf_event__synthesize_modules() allocates a single union
>>> perf_event and reuses it across every kernel module callback.
>>> After the first module is processed,
>>> perf_record_mmap2__read_build_id() sets
>>> PERF_RECORD_MISC_MMAP_BUILD_ID in header.misc and writes that
>>> module's build ID into the event. On subsequent iterations the
>>> callback overwrites start, len, pid, and filename for the next
>>> module but never clears the stale build ID fields or the
>>> MMAP_BUILD_ID flag. When perf_record_mmap2__read_build_id()
>>> runs for the second module it sees the flag, reads the stale
>>> build ID into a dso_id, and __dso__improve_id() permanently
>>> poisons the DSO with the wrong build ID. Every module after
>>> the first therefore receives the first module's build ID in
>>> its MMAP2 record. On a system with the sunrpc and nfsd modules
>>> loaded, this causes perf script and perf report to show
>>> [unknown] for all module symbols.
>>>
>>> The latent bug has existed since commit d9f2ecbc5e47 ("perf
>>> dso: Move build_id to dso_id") introduced the
>>> PERF_RECORD_MISC_MMAP_BUILD_ID check in
>>> perf_record_mmap2__read_build_id(). Commit 53b00ff358dc ("perf
>>> record: Make --buildid-mmap the default") then exposed it to
>>> all users by making the MMAP2-with-build-ID path the default.
>>> Both commits were merged in the same series.
>>>
>>> Clear the MMAP_BUILD_ID flag and zero the build_id union
>>> before each call to perf_record_mmap2__read_build_id() so
>>> that every module starts with a clean slate.
>>>
>>> Fixes: d9f2ecbc5e47 ("perf dso: Move build_id to dso_id")
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>>  tools/perf/util/synthetic-events.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
>>> index ef79433ebc3a..ddf1cbda1902 100644
>>> --- a/tools/perf/util/synthetic-events.c
>>> +++ b/tools/perf/util/synthetic-events.c
>>> @@ -703,6 +703,11 @@ static int perf_event__synthesize_modules_maps_cb(struct map *map, void *data)
>>>
>>>                 memcpy(event->mmap2.filename, dso__long_name(dso), dso__long_name_len(dso) + 1);
> 
> AI points out that leaving stale data isn't just a problem for misc
> bits but also after the filename, due to the event's alignment to 8
> bytes. I think this is benign, you're only leaking bits of the
> filename from the last event, not uninitialized data from the perf
> execution. The AI made me think about it so I thought I'd  repeat it.
> We could add a memset here to clear the filename up to the alignment,
> as is done here:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/synthetic-events.c?h=perf-tools-next#n529
> ```
> memset(event->mmap2.filename + size, 0, machine->id_hdr_size +
>         (aligned_size - size));
> ```

Hi Ian,

I'm not attached to the exact way this issue is addressed, so if
you know of improvements, please feel free to incorporate them.


-- 
Chuck Lever