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 |
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 --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;
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(-)