Re: [PATCH v2 08/16] perf python: Check counts_values size in set_values

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Deleting the `values` attribute from a `perf.counts_values` object in Python causes a segmentation fault.
- [Medium] The function `tracepoint_field` truncates 64-bit tracepoint fields on 32-bit platforms by casting them to `unsigned long` or `long`.
--

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

    perf python: Check counts_values size in set_values
    
    The set_values function incorrectly assumed the list contained exactly
    5 elements. Add a check to prevent out-of-bounds access.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 550f1e7d14499..c6c729808a0f2 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -1913,13 +1920,23 @@ static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObje
>  	}
>  
>  	size = PyList_Size(list);

[Severity: High]
This isn't a bug introduced by this patch, but does 
pyrf_counts_values_set_values() crash if the list parameter is NULL?

If a user deletes the attribute in Python (e.g., `del counts_values.values`),
the list parameter will be passed as NULL. The PyList_Check(list) macro called
just before this line unconditionally dereferences the pointer, which would
trigger a segmentation fault.

> +	if (size != 5) {
> +		PyErr_SetString(PyExc_ValueError, "List must have exactly 5 entries");
> +		return -1;
> +	}
> +
>  	for (Py_ssize_t i = 0; i < size; i++) {

[ ... ]

[Severity: Medium]
This is also a pre-existing issue, but could tracepoint_field() truncate 64-bit
values on 32-bit platforms?

In tools/perf/util/python.c:tracepoint_field():

		val = tep_read_number(pevent, data + field->offset,
				      field->size);
		if (field->flags & TEP_FIELD_IS_POINTER)
			ret = PyLong_FromUnsignedLong((unsigned long) val);
		else if (field->flags & TEP_FIELD_IS_SIGNED)
			ret = PyLong_FromLong((long) val);
		else
			ret = PyLong_FromUnsignedLong((unsigned long) val);

Since val is returned by tep_read_number(), which provides a 64-bit value,
casting it to unsigned long or long drops the top 32 bits on 32-bit
architectures. Does this lead to silent data corruption for 64-bit tracepoint
fields like timestamps or counters?

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