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

On Mon, Aug 03, 2026 at 11:55:02PM +0100, Fayssal Benmlih wrote:
> Hi Cristian,
> 

Hi,

> 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).
>

All of this has been revised in v10 accounting for the header and for
any possible 32bit wraparound...

> > 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.

Indeed, I will have to review this sizing...

Thanks,
Cristian