linux-kernel - Re: [PATCH 05/89] clk: Return error code when of provider pointer is NULL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <158405483605.149997.16173757299414738003@swboyd.mtv.corp.google.com>
Date:   Thu, 12 Mar 2020 16:13:56 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Eric Anholt <eric@...olt.net>, Maxime Ripard <maxime@...no.tech>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
Cc:     dri-devel@...ts.freedesktop.org,
        linux-rpi-kernel@...ts.infradead.org,
        bcm-kernel-feedback-list@...adcom.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        Tim Gover <tim.gover@...pberrypi.com>,
        Phil Elwell <phil@...pberrypi.com>,
        Maxime Ripard <maxime@...no.tech>,
        Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH 05/89] clk: Return error code when of provider pointer is NULL

Quoting Maxime Ripard (2020-02-24 01:06:07)
> The clock framework DT provider helpers don't check the pointers in the
> array registered by the clock provider before returning it.
> 
> This means that if the array is sparse, we will end up returning a NULL
> pointer while the caller expects an error pointer, resulting in a crash.
> 
> Let's test the pointer returned and properly return an error if the pointer
> is NULL.
> 
> Cc: Michael Turquette <mturquette@...libre.com>
> Cc: Stephen Boyd <sboyd@...nel.org>
> Cc: linux-clk@...r.kernel.org
> Signed-off-by: Maxime Ripard <maxime@...no.tech>
> ---
>  drivers/clk/clk.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index f0f2b599fd7e..8532b5ed1060 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4318,13 +4318,18 @@ struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data)
>  {
>         struct clk_onecell_data *clk_data = data;
>         unsigned int idx = clkspec->args[0];
> +       struct clk *clk;
>  
>         if (idx >= clk_data->clk_num) {
>                 pr_err("%s: invalid clock index %u\n", __func__, idx);
>                 return ERR_PTR(-EINVAL);
>         }
>  
> -       return clk_data->clks[idx];
> +       clk = clk_data->clks[idx];
> +       if (!clk)

NULL is a valid clk. That should keep working and not be overriden with
an error pointer. If you want to return an error pointer either fill it
in with an error pointer or write your own version of this.

> +               return ERR_PTR(-ENODEV);
> +
> +       return clk;
>  }
>  EXPORT_SYMBOL_GPL(of_clk_src_onecell_get);
>  
> @@ -4333,13 +4338,18 @@ of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
>  {
>         struct clk_hw_onecell_data *hw_data = data;
>         unsigned int idx = clkspec->args[0];
> +       struct clk_hw *hw;
>  
>         if (idx >= hw_data->num) {
>                 pr_err("%s: invalid index %u\n", __func__, idx);
>                 return ERR_PTR(-EINVAL);
>         }
>  
> -       return hw_data->hws[idx];
> +       hw = hw_data->hws[idx];
> +       if (!hw)

And this one is the same. We let NULL be returned so that it can be
returned as a NULL pointer to the caller if desired. That indicates a
clk that does nothing when used.

> +               return ERR_PTR(-ENODEV);
> +
> +       return hw;
>  }
>  EXPORT_SYMBOL_GPL(of_clk_hw_onecell_get);