Re: [PATCH v2 31/58] perf gecko: Port gecko to use python module

sashiko-bot
2026-04-23T04:20:27Z

Sashiko AI review found 3 potential issue(s):
- [High] The local HTTP server exposes the entire current working directory to any local user.
- [High] Unsafe file writing to a predictable filename can lead to arbitrary file overwrite (Symlink Attack).
- [Medium] The `--event` filtering logic is broken and will silently filter out all events.
--

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

perf gecko: Port gecko to use python module

This patch adds a port of the gecko script that uses the perf python
module directly. This approach is intended to be significantly faster
than using perf script callbacks by avoiding the creation of
intermediate dictionaries for all event fields.

> diff --git a/tools/perf/python/gecko.py b/tools/perf/python/gecko.py
> new file mode 100755
> index 0000000000000..15b0198e4e1c7
> --- /dev/null
> +++ b/tools/perf/python/gecko.py
[ ... ]
> +class CORSRequestHandler(SimpleHTTPRequestHandler):
> + """Enable CORS for requests from profiler.firefox.com."""
> + def end_headers(self):
> + self.send_header('Access-Control-Allow-Origin', 'https://profiler.firefox.com')
> + super().end_headers()

While this behavior might be inherited from the original script being ported,
SimpleHTTPRequestHandler serves the entire current working directory by
default.

Since the server binds to 127.0.0.1 and stays open until interrupted, could
this cause a regression where any unprivileged local user can connect to the
port and read arbitrary files from the directory where the script was run?

[ ... ]
> + def process_event(self, sample) -> None:
> + """Process a single perf sample event."""
> + if self.args.event_name and str(sample.evsel) != self.args.event_name:
> + return

Does str(sample.evsel) return the event name wrapped in "evsel(...)"?

Looking at the pyrf_evsel string representation in tools/perf/util/python.c,
it seems to use PyUnicode_FromFormat("evsel(%s)", evsel__name(evsel)).

If a user runs the script with "-e cycles", event_name would be "cycles".
Would the comparison "evsel(cycles)" != "cycles" always be true, causing a
regression where all events are silently filtered out?

[ ... ]
> + output_file = self.args.save_only
> + if output_file is None:
> + output_file = 'gecko_profile.json'
> + self._write_and_launch(gecko_profile, output_file)
[ ... ]
> + def _write_and_launch(self, profile: Dict, filename: str) -> None:
> + """Write the profile to a file and launch the Firefox profiler."""
> + print("Starting Firefox Profiler on your default browser...")
> + with open(filename, 'w', encoding='utf-8') as f:
> + json.dump(profile, f, indent=2)

While this issue might not be new to the port, writing to a predictable,
hardcoded filename like gecko_profile.json could be vulnerable to symlink
attacks if the script is run in a shared or world-writable directory.

Would this regression allow an attacker to pre-create a symbolic link with
this name pointing to a system file, causing it to be overwritten?

Would it be safer to use the tempfile module or os.open with O_CREAT and
O_EXCL flags to securely create the file?

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