firmware: arm_scmi: Avoid using extended string-buffers sizes if not necessary

Commit b260fccaebdc2 ("firmware: arm_scmi: Add SCMI v3.1 protocol extended
names support") moved all the name string buffers to use the extended buffer
size of 64 instead of the required 16 bytes. While that should be fine if
the firmware terminates the string before 16 bytes, there is possibility
of copying random data if the name is not NULL terminated by the firmware.

SCMI base protocol agent_name/vendor_id/sub_vendor_id are defined by the
specification as NULL-terminated ASCII strings up to 16-bytes in length.

The underlying buffers and message descriptors are currently bigger than
needed; resize them to fit only the strictly needed 16 bytes to avoid
any possible leaks when reading data from the firmware.

Change the size argument of strlcpy to use SCMI_SHORT_NAME_MAX_SIZE always
when 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.

While at that, convert all strings handling routines to use the preferred
strscpy.

Link: https://lore.kernel.org/r/20220608095530.497879-1-cristian.marussi@arm.com
Fixes: b260fccaebdc2 ("firmware: arm_scmi: Add SCMI v3.1 protocol extended names support")
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
This commit is contained in:
Cristian Marussi 2022-06-08 10:55:28 +01:00 committed by Sudeep Holla
parent 8e60294c80
commit 4314f9f4f8
9 changed files with 16 additions and 17 deletions

View File

@ -36,7 +36,7 @@ struct scmi_msg_resp_base_attributes {
struct scmi_msg_resp_base_discover_agent {
__le32 agent_id;
u8 name[SCMI_MAX_STR_SIZE];
u8 name[SCMI_SHORT_NAME_MAX_SIZE];
};
@ -119,7 +119,7 @@ scmi_base_vendor_id_get(const struct scmi_protocol_handle *ph, bool sub_vendor)
ret = ph->xops->do_xfer(ph, t);
if (!ret)
memcpy(vendor_id, t->rx.buf, size);
strscpy(vendor_id, t->rx.buf, size);
ph->xops->xfer_put(ph, t);
@ -276,7 +276,7 @@ static int scmi_base_discover_agent_get(const struct scmi_protocol_handle *ph,
ret = ph->xops->do_xfer(ph, t);
if (!ret) {
agent_info = t->rx.buf;
strlcpy(name, agent_info->name, SCMI_MAX_STR_SIZE);
strscpy(name, agent_info->name, SCMI_SHORT_NAME_MAX_SIZE);
}
ph->xops->xfer_put(ph, t);
@ -375,7 +375,7 @@ static int scmi_base_protocol_init(const struct scmi_protocol_handle *ph)
int id, ret;
u8 *prot_imp;
u32 version;
char name[SCMI_MAX_STR_SIZE];
char name[SCMI_SHORT_NAME_MAX_SIZE];
struct device *dev = ph->dev;
struct scmi_revision_info *rev = scmi_revision_area_get(ph);

View File

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

View File

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

View File

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

View File

@ -24,8 +24,6 @@
#include <asm/unaligned.h>
#define SCMI_SHORT_NAME_MAX_SIZE 16
#define PROTOCOL_REV_MINOR_MASK GENMASK(15, 0)
#define PROTOCOL_REV_MAJOR_MASK GENMASK(31, 16)
#define PROTOCOL_REV_MAJOR(x) ((u16)(FIELD_GET(PROTOCOL_REV_MAJOR_MASK, (x))))

View File

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

View File

@ -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);
@ -634,7 +634,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

View File

@ -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;

View File

@ -13,8 +13,9 @@
#include <linux/notifier.h>
#include <linux/types.h>
#define SCMI_MAX_STR_SIZE 64
#define SCMI_MAX_NUM_RATES 16
#define SCMI_MAX_STR_SIZE 64
#define SCMI_SHORT_NAME_MAX_SIZE 16
#define SCMI_MAX_NUM_RATES 16
/**
* struct scmi_revision_info - version information structure
@ -36,8 +37,8 @@ struct scmi_revision_info {
u8 num_protocols;
u8 num_agents;
u32 impl_ver;
char vendor_id[SCMI_MAX_STR_SIZE];
char sub_vendor_id[SCMI_MAX_STR_SIZE];
char vendor_id[SCMI_SHORT_NAME_MAX_SIZE];
char sub_vendor_id[SCMI_SHORT_NAME_MAX_SIZE];
};
struct scmi_clock_info {