[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

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

[Namhyung Kim]

Hello,

On Sat, Jul 25, 2026 at 08:56:50AM +0000, sashiko-bot@kernel.org wrote:
> 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?

I think it's a valid issue and it seems there's a bug in the
perf_event__synthesize_sample().  The input values contain the lost
field regardless of the read format.  So it should skip the field
carefully.

Thanks,
Namhyung

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

[PVS Narasimha Rao]

Hi Namhyung,

Thanks for the review.

I reproduced it with gdb and confirmed the mismatch: the values in
do_test() are a plain array of struct sample_read_value with a 24-byte
stride, while sample_read_value_size() is 16 without PERF_FORMAT_LOST.
So the synthesizer reads overlapping bytes, and the test compares the
same garbage on both sides and passes.

read.group.values is expected to be packed per read_format, though
evsel__parse_sample() points it into the event data, and perf inject
feeds such a sample straight back to perf_event__synthesize_sample().
So I plan to fix the test input rather than the synthesizer.

I'm preparing v2 and will send it shortly.

Thanks,
Narasimha

[PATCH v2] 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 the parsed
value and id fields to go undetected.

The values are also handed to the synthesis as a plain array of struct
sample_read_value, which always has a 24-byte stride, while
read.group.values is expected to be packed according to read_format --
evsel__parse_sample() points it into the event data.  Without
PERF_FORMAT_LOST the stride is 16, so both the synthesis and the
comparison walk overlapping bytes and the test passes regardless of the
contents.

Validate value and id for grouped reads and continue to validate lost
when PERF_FORMAT_LOST is present, walking the entries with
next_sample_read_value().  Also build the input packed using
sample_read_value_size() so the compared fields are the real ones.

Verified with a deliberate stride bug in copy_read_group_values(): the
test still passes without this change and fails at read_format 0xc with
it applied.

Signed-off-by: PVS Narasimha Rao <venkatasuryapala@gmail.com>
---
Changes in v2:
- Also build the input values packed according to read_format, using
  sample_read_value_size().  v1 only fixed the comparison, but the input
  was still a plain struct sample_read_value array with a 24-byte stride,
  so without PERF_FORMAT_LOST the fields being compared were overlapping
  bytes and the new checks could never fail.
- Expand the commit message to describe the stride problem and how the
  change was verified.

v1: https://lore.kernel.org/linux-perf-users/20260725084704.15463-1-venkatasuryapala@gmail.com/

 tools/perf/tests/sample-parsing.c | 39 +++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 55f0b73ca20e..08dddab443c9 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);
@@ -283,6 +300,7 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format)
 		},
 	};
 	struct sample_read_value values[] = {{1, 5, 0}, {9, 3, 0}, {2, 7, 0}, {6, 4, 1},};
+	struct sample_read_value packed_values[ARRAY_SIZE(values)];
 	struct perf_sample sample_out, sample_out_endian;
 	size_t i, sz, bufsz;
 	int err, ret = -1;
@@ -302,8 +320,19 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format)
 		*(i + (u8 *)regs) = i & 0xfe;
 
 	if (read_format & PERF_FORMAT_GROUP) {
-		sample.read.group.nr     = 4;
-		sample.read.group.values = values;
+		size_t vsz = sample_read_value_size(read_format);
+
+		/*
+		 * evsel__parse_sample() points read.group.values at the event
+		 * data, where the entries are packed according to read_format,
+		 * so build the input the same way.  Otherwise the fields
+		 * compared afterwards are just overlapping bytes.
+		 */
+		for (i = 0; i < ARRAY_SIZE(values); i++)
+			memcpy((void *)packed_values + i * vsz, &values[i], vsz);
+
+		sample.read.group.nr     = ARRAY_SIZE(values);
+		sample.read.group.values = packed_values;
 	} else {
 		sample.read.one.value = 0x08789faeb786aa87ULL;
 		sample.read.one.id    = 99;
-- 
2.43.0