Re: [PATCH 3/3] firmware: arm_scmi: Use preferred strscpy to handle strings

[Peter Hilber <peter.hilber@opensynergy.com>]

On 08.06.22 11:55, Cristian Marussi wrote:
> Remove strlcpy calls used to collect short domain names of SCMI resources
> in favour of the preferred strscpy; while doing that change also the used
> count argument of strscpy to make always use of SCMI_SHORT_NAME_MAX_SIZE
> while dealing with short domain names, so as to limit the possibility that
> an ill-formed non-NULL terminated short reply from the SCMI platform
> firmware can leak stale content laying in the underlying transport shared
> memory area.
> 
> Cc: Peter Hilber <peter.hilber@opensynergy.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Fixes: ca64b719a1e66 ("firmware: arm_scmi: use strlcpy to ensure NULL-terminated strings")
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>

Reviewed-by: Peter Hilber <peter.hilber@opensynergy.com>

> ---
>  drivers/firmware/arm_scmi/clock.c   | 2 +-
>  drivers/firmware/arm_scmi/perf.c    | 2 +-
>  drivers/firmware/arm_scmi/power.c   | 2 +-
>  drivers/firmware/arm_scmi/reset.c   | 2 +-
>  drivers/firmware/arm_scmi/sensors.c | 4 ++--
>  drivers/firmware/arm_scmi/voltage.c | 2 +-
>  6 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> index 1a718faa4192..c7a83f6e38e5 100644
> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -153,7 +153,7 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph,
>  	if (!ret) {
>  		u32 latency = 0;
>  		attributes = le32_to_cpu(attr->attributes);
> -		strlcpy(clk->name, attr->name, SCMI_MAX_STR_SIZE);
> +		strscpy(clk->name, attr->name, SCMI_SHORT_NAME_MAX_SIZE);
>  		/* clock_enable_latency field is present only since SCMI v3.1 */
>  		if (PROTOCOL_REV_MAJOR(version) >= 0x2)
>  			latency = le32_to_cpu(attr->clock_enable_latency);
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index c1f701623058..bbb0331801ff 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -252,7 +252,7 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph,
>  			dom_info->mult_factor =
>  					(dom_info->sustained_freq_khz * 1000) /
>  					dom_info->sustained_perf_level;
> -		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
> +		strscpy(dom_info->name, attr->name, SCMI_SHORT_NAME_MAX_SIZE);
>  	}
>  
>  	ph->xops->xfer_put(ph, t);
> diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
> index 964882cc8747..356e83631664 100644
> --- a/drivers/firmware/arm_scmi/power.c
> +++ b/drivers/firmware/arm_scmi/power.c
> @@ -122,7 +122,7 @@ scmi_power_domain_attributes_get(const struct scmi_protocol_handle *ph,
>  		dom_info->state_set_notify = SUPPORTS_STATE_SET_NOTIFY(flags);
>  		dom_info->state_set_async = SUPPORTS_STATE_SET_ASYNC(flags);
>  		dom_info->state_set_sync = SUPPORTS_STATE_SET_SYNC(flags);
> -		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
> +		strscpy(dom_info->name, attr->name, SCMI_SHORT_NAME_MAX_SIZE);
>  	}
>  	ph->xops->xfer_put(ph, t);
>  
> diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
> index a420a9102094..673f3eb498f4 100644
> --- a/drivers/firmware/arm_scmi/reset.c
> +++ b/drivers/firmware/arm_scmi/reset.c
> @@ -116,7 +116,7 @@ scmi_reset_domain_attributes_get(const struct scmi_protocol_handle *ph,
>  		dom_info->latency_us = le32_to_cpu(attr->latency);
>  		if (dom_info->latency_us == U32_MAX)
>  			dom_info->latency_us = 0;
> -		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
> +		strscpy(dom_info->name, attr->name, SCMI_SHORT_NAME_MAX_SIZE);
>  	}
>  
>  	ph->xops->xfer_put(ph, t);
> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
> index 58fe4f0175be..42efaee27b7c 100644
> --- a/drivers/firmware/arm_scmi/sensors.c
> +++ b/drivers/firmware/arm_scmi/sensors.c
> @@ -412,7 +412,7 @@ iter_axes_desc_process_response(const struct scmi_protocol_handle *ph,
>  	attrh = le32_to_cpu(adesc->attributes_high);
>  	a->scale = S32_EXT(SENSOR_SCALE(attrh));
>  	a->type = SENSOR_TYPE(attrh);
> -	strscpy(a->name, adesc->name, SCMI_MAX_STR_SIZE);
> +	strscpy(a->name, adesc->name, SCMI_SHORT_NAME_MAX_SIZE);
>  
>  	if (a->extended_attrs) {
>  		unsigned int ares = le32_to_cpu(adesc->resolution);
> @@ -633,7 +633,7 @@ iter_sens_descr_process_response(const struct scmi_protocol_handle *ph,
>  			    SUPPORTS_AXIS(attrh) ?
>  			    SENSOR_AXIS_NUMBER(attrh) : 0,
>  			    SCMI_MAX_NUM_SENSOR_AXIS);
> -	strscpy(s->name, sdesc->name, SCMI_MAX_STR_SIZE);
> +	strscpy(s->name, sdesc->name, SCMI_SHORT_NAME_MAX_SIZE);
>  
>  	/*
>  	 * If supported overwrite short name with the extended
> diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c
> index 97df6d3dd131..5de93f637bd4 100644
> --- a/drivers/firmware/arm_scmi/voltage.c
> +++ b/drivers/firmware/arm_scmi/voltage.c
> @@ -233,7 +233,7 @@ static int scmi_voltage_descriptors_get(const struct scmi_protocol_handle *ph,
>  		v = vinfo->domains + dom;
>  		v->id = dom;
>  		attributes = le32_to_cpu(resp_dom->attr);
> -		strlcpy(v->name, resp_dom->name, SCMI_MAX_STR_SIZE);
> +		strscpy(v->name, resp_dom->name, SCMI_SHORT_NAME_MAX_SIZE);
>  
>  		/*
>  		 * If supported overwrite short name with the extended one;