All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] firmware: arm_scmi: use strlcpy to ensure NULL-terminated strings
@ 2018-09-07 16:52 ` Sudeep Holla
  0 siblings, 0 replies; 4+ messages in thread
From: Sudeep Holla @ 2018-09-07 16:52 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, Olof Johansson
  Cc: Sudeep Holla, Kevin Hilman, Arnd Bergmann

Replace all the memcpy() for copying name strings from the firmware with
strlcpy() to make sure we are bounded by the source buffer size and we
also always have NULL-terminated strings.

This is needed to avoid out of bounds accesses if the firmware returns
a non-terminated string.

Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/base.c    | 2 +-
 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/sensors.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

Hi Olof,

Let me know if this is rc/fix material or need to wait for v4.20 ?

Regards,
Sudeep

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index 9dff33ea6416..204390297f4b 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -208,7 +208,7 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle,

 	ret = scmi_do_xfer(handle, t);
 	if (!ret)
-		memcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);
+		strlcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);

 	scmi_xfer_put(handle, t);

diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index e4119eb34986..30fc04e28431 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -111,7 +111,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle,

 	ret = scmi_do_xfer(handle, t);
 	if (!ret)
-		memcpy(clk->name, attr->name, SCMI_MAX_STR_SIZE);
+		strlcpy(clk->name, attr->name, SCMI_MAX_STR_SIZE);
 	else
 		clk->name[0] = '\0';

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 721e6c57beae..c3b0041defee 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -168,7 +168,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 					le32_to_cpu(attr->sustained_perf_level);
 		dom_info->mult_factor =	(dom_info->sustained_freq_khz * 1000) /
 					dom_info->sustained_perf_level;
-		memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
+		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
 	}

 	scmi_xfer_put(handle, t);
diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
index cfa033b05aed..62f3401a1f01 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -106,7 +106,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 		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);
-		memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
+		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
 	}

 	scmi_xfer_put(handle, t);
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index 27f2092b9882..b53d5cc9c9f6 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -140,7 +140,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle,
 			s = &si->sensors[desc_index + cnt];
 			s->id = le32_to_cpu(buf->desc[cnt].id);
 			s->type = SENSOR_TYPE(attrh);
-			memcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
+			strlcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
 		}

 		desc_index += num_returned;
--
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] firmware: arm_scmi: use strlcpy to ensure NULL-terminated strings
@ 2018-09-07 16:52 ` Sudeep Holla
  0 siblings, 0 replies; 4+ messages in thread
From: Sudeep Holla @ 2018-09-07 16:52 UTC (permalink / raw)
  To: linux-arm-kernel

Replace all the memcpy() for copying name strings from the firmware with
strlcpy() to make sure we are bounded by the source buffer size and we
also always have NULL-terminated strings.

This is needed to avoid out of bounds accesses if the firmware returns
a non-terminated string.

Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/base.c    | 2 +-
 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/sensors.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

Hi Olof,

Let me know if this is rc/fix material or need to wait for v4.20 ?

Regards,
Sudeep

diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index 9dff33ea6416..204390297f4b 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -208,7 +208,7 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle,

 	ret = scmi_do_xfer(handle, t);
 	if (!ret)
-		memcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);
+		strlcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);

 	scmi_xfer_put(handle, t);

diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index e4119eb34986..30fc04e28431 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -111,7 +111,7 @@ static int scmi_clock_attributes_get(const struct scmi_handle *handle,

 	ret = scmi_do_xfer(handle, t);
 	if (!ret)
-		memcpy(clk->name, attr->name, SCMI_MAX_STR_SIZE);
+		strlcpy(clk->name, attr->name, SCMI_MAX_STR_SIZE);
 	else
 		clk->name[0] = '\0';

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 721e6c57beae..c3b0041defee 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -168,7 +168,7 @@ scmi_perf_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 					le32_to_cpu(attr->sustained_perf_level);
 		dom_info->mult_factor =	(dom_info->sustained_freq_khz * 1000) /
 					dom_info->sustained_perf_level;
-		memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
+		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
 	}

 	scmi_xfer_put(handle, t);
diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
index cfa033b05aed..62f3401a1f01 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -106,7 +106,7 @@ scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain,
 		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);
-		memcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
+		strlcpy(dom_info->name, attr->name, SCMI_MAX_STR_SIZE);
 	}

 	scmi_xfer_put(handle, t);
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index 27f2092b9882..b53d5cc9c9f6 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -140,7 +140,7 @@ static int scmi_sensor_description_get(const struct scmi_handle *handle,
 			s = &si->sensors[desc_index + cnt];
 			s->id = le32_to_cpu(buf->desc[cnt].id);
 			s->type = SENSOR_TYPE(attrh);
-			memcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
+			strlcpy(s->name, buf->desc[cnt].name, SCMI_MAX_STR_SIZE);
 		}

 		desc_index += num_returned;
--
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] firmware: arm_scmi: use strlcpy to ensure NULL-terminated strings
  2018-09-07 16:52 ` Sudeep Holla
@ 2018-09-07 17:14   ` Olof Johansson
  -1 siblings, 0 replies; 4+ messages in thread
From: Olof Johansson @ 2018-09-07 17:14 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Linux Kernel Mailing List, Linux ARM Mailing List, Kevin Hilman,
	Arnd Bergmann

Hi,

On Fri, Sep 7, 2018 at 9:52 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Replace all the memcpy() for copying name strings from the firmware with
> strlcpy() to make sure we are bounded by the source buffer size and we
> also always have NULL-terminated strings.
>
> This is needed to avoid out of bounds accesses if the firmware returns
> a non-terminated string.
>
> Reported-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

In case I don't end up applying it:

Acked-by: Olof Johansson <olof@lixom.net>

> Let me know if this is rc/fix material or need to wait for v4.20 ?

I don't see a reason to merge this as a fix, it's more of a cleanup.


-Olof

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH] firmware: arm_scmi: use strlcpy to ensure NULL-terminated strings
@ 2018-09-07 17:14   ` Olof Johansson
  0 siblings, 0 replies; 4+ messages in thread
From: Olof Johansson @ 2018-09-07 17:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Fri, Sep 7, 2018 at 9:52 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Replace all the memcpy() for copying name strings from the firmware with
> strlcpy() to make sure we are bounded by the source buffer size and we
> also always have NULL-terminated strings.
>
> This is needed to avoid out of bounds accesses if the firmware returns
> a non-terminated string.
>
> Reported-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

In case I don't end up applying it:

Acked-by: Olof Johansson <olof@lixom.net>

> Let me know if this is rc/fix material or need to wait for v4.20 ?

I don't see a reason to merge this as a fix, it's more of a cleanup.


-Olof

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-07 17:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 16:52 [PATCH] firmware: arm_scmi: use strlcpy to ensure NULL-terminated strings Sudeep Holla
2018-09-07 16:52 ` Sudeep Holla
2018-09-07 17:14 ` Olof Johansson
2018-09-07 17:14   ` Olof Johansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.