On Thu, Aug 20, 2020 at 06:03:33PM -0700, Markus Mayer wrote:
> We would overrun the error_text array if we hit a TIMEOUT condition,
> because we were using the error code "ETIMEDOUT" (which is 110) as an
> array index.
>
> We fix the problem by correcting the array index and by providing a
> function to retrieve error messages rather than accessing the array
> directly. The function includes a bounds check that prevents the array
> from being overrun.
>
> Signed-off-by: Markus Mayer <mma...@broadcom.com>
> ---
>
> This patch was prepared in response to https://lkml.org/lkml/2020/8/18/505.
>
> drivers/memory/brcmstb_dpfe.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
> index 81abc4a98a27..a986a849f58e 100644
> --- a/drivers/memory/brcmstb_dpfe.c
> +++ b/drivers/memory/brcmstb_dpfe.c
> @@ -190,11 +190,6 @@ struct brcmstb_dpfe_priv {
> struct mutex lock;
> };
>
> -static const char * const error_text[] = {
> - "Success", "Header code incorrect", "Unknown command or argument",
> - "Incorrect checksum", "Malformed command", "Timed out",
> -};
> -
> /*
> * Forward declaration of our sysfs attribute functions, so we can declare
> the
> * attribute data structures early.
> @@ -307,6 +302,20 @@ static const struct dpfe_api dpfe_api_v3 = {
> },
> };
>
> +static const char * const get_error_text(unsigned int i)
The pointer itself is returned by value and you cannot return a const
value. I mean, you can but it does not have an effect. Only pointed
memory should be const (const const char*).
Best regards,
Krzysztof
> +{
> + static const char * const error_text[] = {
> + "Success", "Header code incorrect",
> + "Unknown command or argument", "Incorrect checksum",
> + "Malformed command", "Timed out", "Unknown error",
> + };
> +
> + if (unlikely(i >= ARRAY_SIZE(error_text)))
> + i = ARRAY_SIZE(error_text) - 1;
> +
> + return error_text[i];
> +}
> +
> static bool is_dcpu_enabled(struct brcmstb_dpfe_priv *priv)
> {
> u32 val;
> @@ -446,7 +455,7 @@ static int __send_command(struct brcmstb_dpfe_priv *priv,
> unsigned int cmd,
> }
> if (resp != 0) {
> mutex_unlock(&priv->lock);
> - return -ETIMEDOUT;
> + return -ffs(DCPU_RET_ERR_TIMEDOUT);
> }
>
> /* Compute checksum over the message */
> @@ -695,7 +704,7 @@ static ssize_t generic_show(unsigned int command, u32
> response[],
>
> ret = __send_command(priv, command, response);
> if (ret < 0)
> - return sprintf(buf, "ERROR: %s\n", error_text[-ret]);
> + return sprintf(buf, "ERROR: %s\n", get_error_text(-ret));
>
> return 0;
> }
> --
> 2.17.1
>
