From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Kan Liang <kan.liang@linux.intel.com>,
Nick Terrell <terrelln@fb.com>,
Yanteng Si <siyanteng@loongson.cn>,
Yicong Yang <yangyicong@hisilicon.com>,
James Clark <james.clark@linaro.org>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v1 4/8] perf header: Fail read if header sections overlap
Date: Thu, 29 Aug 2024 08:01:50 -0700 [thread overview]
Message-ID: <20240829150154.37929-5-irogers@google.com> (raw)
In-Reply-To: <20240829150154.37929-1-irogers@google.com>
Buggy perf.data files can have the attributes and data
overlapping. For example, when processing pipe data the attributes
aren't known and so file offset header calculations can consider them
not present. Later this can cause the attributes to overwrite the
data. This can be seen in:
```
$ perf record -o - true > a.data
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.059 MB - ]
$ perf inject -i a.data -o b.data
$ perf report --stats -i b.data
0x68 [0]: failed to process type: 510379 [Invalid argument]
Error:
failed to process sample
```
This change makes reading the corrupt file fail:
```
$ perf report --stats -i b.data
Perf file header corrupt: Attributes and data overlap
incompatible file format (rerun with -v to learn more)
```
Which is more informative.
Signed-off-by: Ian Rogers <irogers@google.com>
---
| 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 3309fe7f1d12..65c9086610cb 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3986,6 +3986,24 @@ int perf_file_header__read(struct perf_file_header *header,
adds_features));
}
+ if (header->size > header->attrs.offset) {
+ pr_err("Perf file header corrupt: header overlaps attrs\n");
+ return -1;
+ }
+
+ if (header->size > header->data.offset) {
+ pr_err("Perf file header corrupt: header overlaps data\n");
+ return -1;
+ }
+
+ if ((header->attrs.offset <= header->data.offset &&
+ header->attrs.offset + header->attrs.size > header->data.offset) ||
+ (header->attrs.offset > header->data.offset &&
+ header->data.offset + header->data.size > header->attrs.offset)) {
+ pr_err("Perf file header corrupt: Attributes and data overlap\n");
+ return -1;
+ }
+
if (header->size != sizeof(*header)) {
/* Support the previous format */
if (header->size == offsetof(typeof(*header), adds_features))
--
2.46.0.295.g3b9ea8a38a-goog
next prev parent reply other threads:[~2024-08-29 15:02 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-29 15:01 [PATCH v1 0/8] Correct inject's handling of pipe files on disk Ian Rogers
2024-08-29 15:01 ` [PATCH v1 1/8] perf report: Name events in stats for pipe mode Ian Rogers
2024-08-29 15:01 ` [PATCH v1 2/8] perf session: Document struct and constify auxtrace Ian Rogers
2024-08-29 15:01 ` [PATCH v1 3/8] perf header: Add kerneldoc to perf_file_header Ian Rogers
2024-08-29 15:01 ` Ian Rogers [this message]
2024-08-29 15:01 ` [PATCH v1 5/8] perf header: Allow attributes to be written after data Ian Rogers
2024-08-29 19:31 ` Arnaldo Carvalho de Melo
2024-08-29 20:12 ` Ian Rogers
2024-08-29 20:58 ` Arnaldo Carvalho de Melo
2024-08-29 21:42 ` Ian Rogers
2024-08-30 4:39 ` Namhyung Kim
2024-08-30 5:03 ` Ian Rogers
2024-08-30 16:04 ` Namhyung Kim
2024-08-30 12:19 ` Arnaldo Carvalho de Melo
2024-08-29 15:01 ` [PATCH v1 6/8] perf inject: Overhaul handling of pipe files Ian Rogers
2024-08-29 15:01 ` [PATCH v1 7/8] perf header: Remove repipe option Ian Rogers
2024-08-29 15:01 ` [PATCH v1 8/8] perf test: Additional pipe tests with pipe output written to a file Ian Rogers
2024-08-29 15:22 ` [PATCH v1 0/8] Correct inject's handling of pipe files on disk Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240829150154.37929-5-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=siyanteng@loongson.cn \
--cc=terrelln@fb.com \
--cc=yangyicong@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox