Re: [PATCH v7 30/59] perf flamegraph: Port flamegraph to use python module

sashiko-bot
2026-04-25T23:09:21Z

Sashiko AI review found 4 potential issue(s):
- [High] Fallback symbol resolution incorrectly assigns `None` to the node name instead of `"[unknown]"`, breaking the generated JSON.
- [Medium] Sequential template string replacements allow untrusted input to corrupt the generated HTML file.
- [Medium] Path evaluation mismatch causes unintended network access instead of reading local files.
- [Medium] Interactive `input()` prompt is written to standard output, corrupting the generated flame graph when stdout is redirected.
--

commit f4e070e566d630118a5c68aeacf90a2bc934cc23
Author: Ian Rogers <irogers@google.com>

perf flamegraph: Port flamegraph to use python module

Add a port of the flamegraph script that uses the perf python module
directly. This approach improves performance by avoiding intermediate
dictionaries for event fields.

> diff --git a/tools/perf/python/flamegraph.py b/tools/perf/python/flamegraph.py
> new file mode 100755
> index 000000000000..b0eb5844b772
> --- /dev/null
> +++ b/tools/perf/python/flamegraph.py

[ ... ]

> + else:
> + # Fallback if no callchain
> + name = getattr(sample, "symbol", "[unknown]")

If sample.symbol exists but evaluates to None (for example, if the symbol is
unresolved), Python's getattr() will return None rather than falling back to
"[unknown]".

Will this cause the node name to be serialized as {"n": null} in the output
JSON and break the d3-flame-graph rendering?

> + libtype = self.get_libtype_from_dso(getattr(sample, "dso", "[unknown]"))
> + node = self.find_or_create_node(node, name, libtype)

[ ... ]

> + use_minimal = False
> + try:
> + if not template:
> + use_minimal = True
> + elif template.startswith("http"):
> + with urllib.request.urlopen(template) as url_template:

If a local file happens to be named in the format of a URL (e.g.,
"http://example.com"), os.path.isfile() checked earlier will return true, but
this block will still pass it to urllib.request.urlopen().

Could this cause unintended external network connections instead of reading the
intended local file?

> + output_str = "".join([l.decode("utf-8") for l in url_template.readlines()])
> + else:
> + with open(template, "r", encoding="utf-8") as f:
> + output_str = f.read()

[ ... ]

> + elif template_md5sum:
> + assert output_str is not None
> + download_md5sum = hashlib.md5(output_str.encode("utf-8")).hexdigest()
> + if download_md5sum != template_md5sum:
> + s = None
> + while s not in ["y", "n"]:
> + s = input(f"""Unexpected template md5sum.
> +{download_md5sum} != {template_md5sum}, for:
> +{template}
> +continue?[yn] """).lower()

If the script is invoked with standard output redirected (for example,
"flamegraph.py -o - > output.html"), will this input() prompt be written
directly into the HTML payload and corrupt the output?

Additionally, would the script appear to hang since the user won't see the
prompt on standard error?

> + if s == "n":
> + sys.exit(1)
> +
> + assert output_str is not None
> + output_str = output_str.replace("/** @options_json **/", options_json)
> + output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)

If the untrusted perf.data file header contains the string
"/** @flamegraph_json **/", it will be injected into options_json during the
first replacement.

The second replacement will then substitute stacks_json inside the newly
injected placeholder within the options string literal.

Could this result in malformed JavaScript and cause a SyntaxError in the
generated HTML?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260425224951.174663-1-irogers@google.com?part=30