Re: [PATCH v7 11/23] firmware: arm_scmi: Add Telemetry notification support

Hi Cristian,

A couple of notification payload sizing issues inline.

> 	/* At least sized as an empty notification */
> 	if (payld_sz < sizeof(*p))
> 		return NULL;
>
> 	r->timestamp = timestamp;
> 	r->agent_id = le32_to_cpu(p->agent_id);
> 	r->status = le32_to_cpu(p->status);
> 	r->num_dwords = le32_to_cpu(p->num_dwords);
>
> 	if (r->num_dwords * sizeof(r->dwords[0]) > payld_sz)
> 		return NULL;

payld_sz includes the fixed wire header containing agent_id, status and
num_dwords. The array length is compared against that full size instead of
the bytes remaining after sizeof(*p).

This can accept num_dwords values whose array extends beyond the received
payload. The multiplication can also wrap before the comparison.

Please validate num_dwords using division after accounting for the fixed
header, for example against:

	(payld_sz - sizeof(*p)) / sizeof(p->array[0])

after first checking payld_sz >= sizeof(*p).

> static const struct scmi_event tlm_events[] = {
> 	{
> 		.id = SCMI_EVENT_TELEMETRY_UPDATE,
> 		.max_payld_sz = 0,
> 		.max_report_sz = 0,
> 	},
> };

With max_report_sz set to zero, the generic notification code allocates
max_msg_sz bytes for the decoded report.

The decoded scmi_telemetry_update_report has a larger fixed header than
the wire payload because it also includes ktime_t. Consequently, a
maximum-sized valid wire payload requires more than max_msg_sz bytes after
conversion and can overflow the report buffer.

Please provide or calculate enough space for the decoded report header
plus the maximum dword array, or constrain the number of copied dwords to
the actual report-buffer capacity.

Thanks,
Fayçal