Re: [PATCH v7 06/23] firmware: arm_scmi: Add basic Telemetry support

Hi Cristian,

I found a few error-path consistency issues inline.

> 	tde->de.tstamp_support = !!tde->ts_type;
> 	/* Count timestamped DEs */
> 	ti->num_des_tstamp += !!tde->de.tstamp_support;
> 	tde->de.fc_support = IS_FC_SUPPORTED(desc);
> 	tde->de.name_support = IS_NAME_SUPPORTED(desc);
> [...]
> 	if (rx_len < payld_sz)
> 		return -ENOSPC;

num_des_tstamp is incremented before all variable parts of the descriptor
and the optional fast-channel mapping have been validated.

If a later validation or ioremap operation fails, the descriptor is
rejected but the timestamp count is not rolled back. GET_ALL state checks
can then compare against an inflated timestamp count.

Please update this counter only after the complete descriptor has been
validated and successfully registered, or undo it on every later failure.

> err:
> 	/* DE not enumerated at this point were created in this call */
> 	if (discovered)
> 		scmi_telemetry_free_tde_put(ti, tde);
>
> 	return ret;

At this point descriptor parsing may already have changed fields in tde and
its associated scmi_telemetry_de_info. The object is returned to the free
list without resetting that partial state.

Can the descriptor be fully reset before it is made available for reuse,
or can parsing be done into temporary state that is committed only after
all validation succeeds?

> 	for (int i = 0; i < ti->info.base.num_groups; i++) {
> 		struct scmi_telemetry_group *grp = &rinfo->grps[i];
> [...]
> 		grp->des = no_free_ptr(des);
> 		grp->des_str = no_free_ptr(des_str);
> 		/* Reset group DE counter */
> 		grp->info->num_des = 0;
> 	}
> [...]
> 	rinfo->num_groups = ti->info.base.num_groups;

The per-group allocations are transferred out of automatic cleanup during
the loop, but rinfo->num_groups is only assigned after every group
succeeds.

If allocation for a later group fails, resources_free() calls
scmi_telemetry_groups_free(), which iterates rinfo->num_groups. Since that
is still zero, allocations already installed in earlier groups are leaked.

Please increment a cleanup-visible group count as each group is committed,
or make cleanup iterate base.num_groups and safely free NULL members.

Thanks,
Fayçal