diff mbox series

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

Message ID 20220608095530.497879-3-cristian.marussi@arm.com (mailing list archive)
State New, archived
Headers show
Series [1/3] firmware: arm_scmi: Review BASE protocol string-buffers sizes | expand

Commit Message

Cristian Marussi June 8, 2022, 9:55 a.m. UTC
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>
---
 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(-)

Comments

Peter Hilber June 8, 2022, 3:19 p.m. UTC | #1
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;
diff mbox series

Patch

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;