[PATCH] perf test sample-parsing: Validate PERF_FORMAT_GROUP values without LOST

[PATCH] perf test sample-parsing: Validate PERF_FORMAT_GROUP values without LOST

[PVS Narasimha Rao]

The sample parsing test only validates grouped read values
when PERF_FORMAT_LOST is present.

For PERF_FORMAT_GROUP without PERF_FORMAT_LOST, the contents of
read.group.values[] are not validated, allowing corruption of
parsed value and id fields to go undetected.

Validate value and id for grouped reads and continue to
validate lost when PERF_FORMAT_LOST is present.

Use next_sample_read_value() to walk the variable-sized group
entries.

Signed-off-by: PVS Narasimha Rao <venkatasuryapala@gmail.com>

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 55f0b73ca20e..e44b01e5cd14 100644
--- a/tools/perf/tests/sample-parsing.c
+++ b/tools/perf/tests/sample-parsing.c
@@ -86,10 +86,27 @@ static bool samples_same(struct perf_sample *s1,
 			COMP(read.time_running);
 		/* PERF_FORMAT_ID is forced for PERF_SAMPLE_READ */
 		if (read_format & PERF_FORMAT_GROUP) {
+			struct sample_read_value *v1 = s1->read.group.values;
+			struct sample_read_value *v2 = s2->read.group.values;
+
 			for (i = 0; i < s1->read.group.nr; i++) {
-				/* FIXME: check values without LOST */
-				if (read_format & PERF_FORMAT_LOST)
-					MCOMP(read.group.values[i]);
+				if (v1->value != v2->value) {
+					pr_debug("Samples differ at 'read.group.values[].value'\n");
+					return false;
+				}
+
+				if (v1->id != v2->id) {
+					pr_debug("Samples differ at 'read.group.values[].id'\n");
+					return false;
+				}
+
+				if (read_format & PERF_FORMAT_LOST &&
+				    v1->lost != v2->lost) {
+					pr_debug("Samples differ at 'read.group.values[].lost'\n");
+					return false;
+				}
+				v1 = next_sample_read_value(v1, read_format);
+				v2 = next_sample_read_value(v2, read_format);
 			}
 		} else {
 			COMP(read.one.id);
-- 
2.43.7

Re: [PATCH] perf test sample-parsing: Validate PERF_FORMAT_GROUP values without LOST

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The test setup for grouped reads without PERF_FORMAT_LOST validates garbage data because it provides a standard C array of structs rather than a tightly packed buffer.
--

--- Patch [1]: [PATCH] perf test sample-parsing: Validate PERF_FORMAT_GROUP values without LOST ---
commit 803c0b64d4e8c311df29991bbbdd74726b85a1dd
Author: PVS Narasimha Rao <venkatasuryapala@gmail.com>

perf test sample-parsing: Validate PERF_FORMAT_GROUP values without LOST

This patch updates the sample parsing test to validate the value and id
fields in grouped reads even when PERF_FORMAT_LOST is absent. It uses
next_sample_read_value() to properly navigate the variable-sized group
entries during validation.

> diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
> index 55f0b73ca20e0..e44b01e5cd14c 100644
> --- a/tools/perf/tests/sample-parsing.c
> +++ b/tools/perf/tests/sample-parsing.c
[ ... ]
> @@ -86,10 +86,27 @@ static bool samples_same(struct perf_sample *s1,
>  			COMP(read.time_running);
>  		/* PERF_FORMAT_ID is forced for PERF_SAMPLE_READ */
>  		if (read_format & PERF_FORMAT_GROUP) {
> +			struct sample_read_value *v1 = s1->read.group.values;
> +			struct sample_read_value *v2 = s2->read.group.values;
> +
>  			for (i = 0; i < s1->read.group.nr; i++) {
> -				/* FIXME: check values without LOST */
> -				if (read_format & PERF_FORMAT_LOST)
> -					MCOMP(read.group.values[i]);
> +				if (v1->value != v2->value) {
> +					pr_debug("Samples differ at 'read.group.values[].value'\n");
> +					return false;
> +				}
> +
> +				if (v1->id != v2->id) {
> +					pr_debug("Samples differ at 'read.group.values[].id'\n");
> +					return false;
> +				}
> +
> +				if (read_format & PERF_FORMAT_LOST &&
> +				    v1->lost != v2->lost) {
> +					pr_debug("Samples differ at 'read.group.values[].lost'\n");
> +					return false;
> +				}
> +				v1 = next_sample_read_value(v1, read_format);
> +				v2 = next_sample_read_value(v2, read_format);

[Severity: Medium]
This is a pre-existing issue, but does using next_sample_read_value() here
inadvertently cause the test to assert equality on misaligned garbage data
when PERF_FORMAT_LOST is omitted?

Looking at do_test(), the original sample is initialized with a standard C
array of struct sample_read_value:

tools/perf/tests/sample-parsing.c:do_test() {
    ...
    struct sample_read_value values[] = {{1, 5, 0}, {9, 3, 0}, {2, 7, 0}, {6, 4, 1},};
    ...
    if (read_format & PERF_FORMAT_GROUP) {
        sample.read.group.nr     = 4;
        sample.read.group.values = values;
    }
    ...
}

Since this array always has a 24-byte stride, but next_sample_read_value()
advances by a 16-byte stride when PERF_FORMAT_LOST is absent, both the
synthesizer and this validation loop end up reading overlapping memory
instead of the actual struct fields.

Could the test setup be updated to provide a tightly packed buffer so that
we validate the actual fields rather than falsely passing on identical
misaligned reads?

>  			}
>  		} else {
>  			COMP(read.one.id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725084704.15463-1-venkatasuryapala@gmail.com?part=1