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

Commit ef3cf980303e2 ("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: ef3cf980303e2 ("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>

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index d0ac96da1ddfff6b9e579bc2043430217a4e0fc0..a52f084a6a87bbb5349b79fddd8de2fb2ee7dc58 100644 (file)
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -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);
 

diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index 1a718faa4192498457c93739d51c388722aa3eea..c7a83f6e38e5a76bdb0fb1d20be585b9ffdd24a4 100644 (file)
--- 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 c1f7016230582713b6d3729b9b42eb1271188c94..bbb0331801ff49274d23f2ea61d582f4f0dbdf7b 100644 (file)
--- 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 964882cc87471210080202e416d2ef9a22c11369..356e83631664d965141511f5481cd5ed18fdc7a1 100644 (file)
--- 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/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index 73304af5ec4ac49118c4946de0f9d069424fd3a0..c679f3fb8718bb845fbe43068a7398c740d7f5e9 100644 (file)
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -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))))

diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
index a420a91020940e5deb69886f33746277219709b7..673f3eb498f43483c1ece184c98d8c4b3ce5aef0 100644 (file)
--- 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 8a93dd944c49c5de75e67c3193a0d0f1dca38249..7288c61178380813cb63fb83e588180c3308ab93 100644 (file)
--- 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);
@@ -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

diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c
index 97df6d3dd1318d4e4b33c33e072df4a840f16df7..5de93f637bd47d50c114a5c3815cd5c323c1d136 100644 (file)
--- 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/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 1c58646ba381c368f4458d9a1a46f4f06015de4f..704111f63993739f48d0dda2efd8728ab527122c 100644 (file)
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -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 {