Hi,
On Thu, Sep 06, 2018 at 04:10:39PM +0100, Sudeep Holla wrote:
> Firmware can provide zero as values for sustained performance level and
> corresponding sustained frequency in kHz in order to hide the actual
> frequencies and provide only abstract values. It may endup with divide
> by zero scenario resulting in kernel panic.
>
> Let's set the multiplication factor to one if either one or both of them
> (sustained_perf_level and sustained_freq) are set to zero.
>
> Fixes: a9e3fbfaa0ff ("firmware: arm_scmi: add initial support for performance
> protocol")
> Reported-by: Ionela Voinescu <ionela.voine...@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.ho...@arm.com>
> ---
> drivers/firmware/arm_scmi/perf.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> Hi ARM SoC team,
>
> Can you pick this patch directly ?
Applied, however:
> diff --git a/drivers/firmware/arm_scmi/perf.c
> b/drivers/firmware/arm_scmi/perf.c
> index 721e6c57beae..64342944d917 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -166,7 +166,13 @@ scmi_perf_domain_attributes_get(const struct scmi_handle
> *handle, u32 domain,
> le32_to_cpu(attr->sustained_freq_khz);
> dom_info->sustained_perf_level =
> le32_to_cpu(attr->sustained_perf_level);
> - dom_info->mult_factor = (dom_info->sustained_freq_khz * 1000) /
> + if (!dom_info->sustained_freq_khz ||
> + !dom_info->sustained_perf_level)
> + /* CPUFreq converts to kHz, hence default 1000 */
> + dom_info->mult_factor = 1000;
> + else
> + 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);
I noticed you do memcpy of these name strings in a few places, and use
it as a string. Any firmware that would return a non-terminated string
would cause problems later on. strlcpy() might be a better approach.
-Olof
