* [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference
@ 2017-04-20 14:38 Marek Vasut
2017-04-21 0:14 ` Kuninori Morimoto
0 siblings, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2017-04-20 14:38 UTC (permalink / raw)
To: alsa-devel
Cc: Marek Vasut, Geert Uytterhoeven, Hiroyuki Yokoyama,
Kuninori Morimoto, Mark Brown, linux-renesas-soc
In case the "clock-frequency" DT property is not present, the
of_find_property(np, "clock-frequency", NULL) will return NULL
and the subsequent req_size = prop->length / sizeof(u32); will
trigger a NULL pointer dereference.
This patch adds check for the NULL return value and propagates
the error into the caller of the function.
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-renesas-soc@vger.kernel.org
---
sound/soc/sh/rcar/adg.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c
index 96fef91b480c..a7cbe7e89dfc 100644
--- a/sound/soc/sh/rcar/adg.c
+++ b/sound/soc/sh/rcar/adg.c
@@ -425,8 +425,7 @@ static void rsnd_adg_get_clkin(struct rsnd_priv *priv,
dev_dbg(dev, "clk %d : %p : %ld\n", i, clk, clk_get_rate(clk));
}
-static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
- struct rsnd_adg *adg)
+static int rsnd_adg_get_clkout(struct rsnd_priv *priv, struct rsnd_adg *adg)
{
struct clk *clk;
struct device *dev = rsnd_priv_to_dev(priv);
@@ -459,7 +458,10 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
* ADG supports BRRA/BRRB output only
* this means all clkout0/1/2/3 will be same rate
*/
- prop = of_find_property(np, "clock-frequency", NULL);;
+ prop = of_find_property(np, "clock-frequency", NULL);
+ if (!prop)
+ return -EINVAL;
+
req_size = prop->length / sizeof(u32);
of_property_read_u32_array(np, "clock-frequency", req_rate, req_size);
@@ -568,6 +570,7 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
dev_dbg(dev, "clkout %d : %p : %ld\n", i, clk, clk_get_rate(clk));
dev_dbg(dev, "BRGCKR = 0x%08x, BRRA/BRRB = 0x%x/0x%x\n",
ckr, rbga, rbgb);
+ return 0;
}
int rsnd_adg_probe(struct rsnd_priv *priv)
@@ -589,7 +592,9 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
return ret;
rsnd_adg_get_clkin(priv, adg);
- rsnd_adg_get_clkout(priv, adg);
+ ret = rsnd_adg_get_clkout(priv, adg);
+ if (ret)
+ return ret;
if (of_get_property(np, "clkout-lr-asynchronous", NULL))
adg->flags = LRCLK_ASYNC;
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference
2017-04-20 14:38 [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference Marek Vasut
@ 2017-04-21 0:14 ` Kuninori Morimoto
0 siblings, 0 replies; 4+ messages in thread
From: Kuninori Morimoto @ 2017-04-21 0:14 UTC (permalink / raw)
To: Marek Vasut
Cc: alsa-devel, Marek Vasut, Geert Uytterhoeven, Hiroyuki Yokoyama,
Mark Brown, linux-renesas-soc
Hi Marek
> In case the "clock-frequency" DT property is not present, the
> of_find_property(np, "clock-frequency", NULL) will return NULL
> and the subsequent req_size = prop->length / sizeof(u32); will
> trigger a NULL pointer dereference.
>
> This patch adds check for the NULL return value and propagates
> the error into the caller of the function.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
Thank you for your patch, it was my fail.
But your patch will breaks Lager board sound
(= it doesn't need clkout, and have no "clock-frequency")
I will try to fix this issue
> sound/soc/sh/rcar/adg.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c
> index 96fef91b480c..a7cbe7e89dfc 100644
> --- a/sound/soc/sh/rcar/adg.c
> +++ b/sound/soc/sh/rcar/adg.c
> @@ -425,8 +425,7 @@ static void rsnd_adg_get_clkin(struct rsnd_priv *priv,
> dev_dbg(dev, "clk %d : %p : %ld\n", i, clk, clk_get_rate(clk));
> }
>
> -static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
> - struct rsnd_adg *adg)
> +static int rsnd_adg_get_clkout(struct rsnd_priv *priv, struct rsnd_adg *adg)
> {
> struct clk *clk;
> struct device *dev = rsnd_priv_to_dev(priv);
> @@ -459,7 +458,10 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
> * ADG supports BRRA/BRRB output only
> * this means all clkout0/1/2/3 will be same rate
> */
> - prop = of_find_property(np, "clock-frequency", NULL);;
> + prop = of_find_property(np, "clock-frequency", NULL);
> + if (!prop)
> + return -EINVAL;
> +
> req_size = prop->length / sizeof(u32);
>
> of_property_read_u32_array(np, "clock-frequency", req_rate, req_size);
> @@ -568,6 +570,7 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
> dev_dbg(dev, "clkout %d : %p : %ld\n", i, clk, clk_get_rate(clk));
> dev_dbg(dev, "BRGCKR = 0x%08x, BRRA/BRRB = 0x%x/0x%x\n",
> ckr, rbga, rbgb);
> + return 0;
> }
>
> int rsnd_adg_probe(struct rsnd_priv *priv)
> @@ -589,7 +592,9 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
> return ret;
>
> rsnd_adg_get_clkin(priv, adg);
> - rsnd_adg_get_clkout(priv, adg);
> + ret = rsnd_adg_get_clkout(priv, adg);
> + if (ret)
> + return ret;
>
> if (of_get_property(np, "clkout-lr-asynchronous", NULL))
> adg->flags = LRCLK_ASYNC;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference
@ 2017-04-21 0:14 ` Kuninori Morimoto
0 siblings, 0 replies; 4+ messages in thread
From: Kuninori Morimoto @ 2017-04-21 0:14 UTC (permalink / raw)
To: Marek Vasut
Cc: alsa-devel, Marek Vasut, Geert Uytterhoeven, Hiroyuki Yokoyama,
Mark Brown, linux-renesas-soc
Hi Marek
> In case the "clock-frequency" DT property is not present, the
> of_find_property(np, "clock-frequency", NULL) will return NULL
> and the subsequent req_size = prop->length / sizeof(u32); will
> trigger a NULL pointer dereference.
>
> This patch adds check for the NULL return value and propagates
> the error into the caller of the function.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
Thank you for your patch, it was my fail.
But your patch will breaks Lager board sound
(= it doesn't need clkout, and have no "clock-frequency")
I will try to fix this issue
> sound/soc/sh/rcar/adg.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c
> index 96fef91b480c..a7cbe7e89dfc 100644
> --- a/sound/soc/sh/rcar/adg.c
> +++ b/sound/soc/sh/rcar/adg.c
> @@ -425,8 +425,7 @@ static void rsnd_adg_get_clkin(struct rsnd_priv *priv,
> dev_dbg(dev, "clk %d : %p : %ld\n", i, clk, clk_get_rate(clk));
> }
>
> -static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
> - struct rsnd_adg *adg)
> +static int rsnd_adg_get_clkout(struct rsnd_priv *priv, struct rsnd_adg *adg)
> {
> struct clk *clk;
> struct device *dev = rsnd_priv_to_dev(priv);
> @@ -459,7 +458,10 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
> * ADG supports BRRA/BRRB output only
> * this means all clkout0/1/2/3 will be same rate
> */
> - prop = of_find_property(np, "clock-frequency", NULL);;
> + prop = of_find_property(np, "clock-frequency", NULL);
> + if (!prop)
> + return -EINVAL;
> +
> req_size = prop->length / sizeof(u32);
>
> of_property_read_u32_array(np, "clock-frequency", req_rate, req_size);
> @@ -568,6 +570,7 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
> dev_dbg(dev, "clkout %d : %p : %ld\n", i, clk, clk_get_rate(clk));
> dev_dbg(dev, "BRGCKR = 0x%08x, BRRA/BRRB = 0x%x/0x%x\n",
> ckr, rbga, rbgb);
> + return 0;
> }
>
> int rsnd_adg_probe(struct rsnd_priv *priv)
> @@ -589,7 +592,9 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
> return ret;
>
> rsnd_adg_get_clkin(priv, adg);
> - rsnd_adg_get_clkout(priv, adg);
> + ret = rsnd_adg_get_clkout(priv, adg);
> + if (ret)
> + return ret;
>
> if (of_get_property(np, "clkout-lr-asynchronous", NULL))
> adg->flags = LRCLK_ASYNC;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference
2017-04-21 0:14 ` Kuninori Morimoto
(?)
@ 2017-04-21 0:40 ` Marek Vasut
-1 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2017-04-21 0:40 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: alsa-devel, Marek Vasut, Geert Uytterhoeven, Hiroyuki Yokoyama,
Mark Brown, linux-renesas-soc
On 04/21/2017 02:14 AM, Kuninori Morimoto wrote:
>
> Hi Marek
Hi!
>> In case the "clock-frequency" DT property is not present, the
>> of_find_property(np, "clock-frequency", NULL) will return NULL
>> and the subsequent req_size = prop->length / sizeof(u32); will
>> trigger a NULL pointer dereference.
>>
>> This patch adds check for the NULL return value and propagates
>> the error into the caller of the function.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
>> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>
> Thank you for your patch, it was my fail.
> But your patch will breaks Lager board sound
> (= it doesn't need clkout, and have no "clock-frequency")
>
> I will try to fix this issue
Cool, thanks!
>> sound/soc/sh/rcar/adg.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c
>> index 96fef91b480c..a7cbe7e89dfc 100644
>> --- a/sound/soc/sh/rcar/adg.c
>> +++ b/sound/soc/sh/rcar/adg.c
>> @@ -425,8 +425,7 @@ static void rsnd_adg_get_clkin(struct rsnd_priv *priv,
>> dev_dbg(dev, "clk %d : %p : %ld\n", i, clk, clk_get_rate(clk));
>> }
>>
>> -static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
>> - struct rsnd_adg *adg)
>> +static int rsnd_adg_get_clkout(struct rsnd_priv *priv, struct rsnd_adg *adg)
>> {
>> struct clk *clk;
>> struct device *dev = rsnd_priv_to_dev(priv);
>> @@ -459,7 +458,10 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
>> * ADG supports BRRA/BRRB output only
>> * this means all clkout0/1/2/3 will be same rate
>> */
>> - prop = of_find_property(np, "clock-frequency", NULL);;
>> + prop = of_find_property(np, "clock-frequency", NULL);
>> + if (!prop)
>> + return -EINVAL;
>> +
>> req_size = prop->length / sizeof(u32);
>>
>> of_property_read_u32_array(np, "clock-frequency", req_rate, req_size);
>> @@ -568,6 +570,7 @@ static void rsnd_adg_get_clkout(struct rsnd_priv *priv,
>> dev_dbg(dev, "clkout %d : %p : %ld\n", i, clk, clk_get_rate(clk));
>> dev_dbg(dev, "BRGCKR = 0x%08x, BRRA/BRRB = 0x%x/0x%x\n",
>> ckr, rbga, rbgb);
>> + return 0;
>> }
>>
>> int rsnd_adg_probe(struct rsnd_priv *priv)
>> @@ -589,7 +592,9 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
>> return ret;
>>
>> rsnd_adg_get_clkin(priv, adg);
>> - rsnd_adg_get_clkout(priv, adg);
>> + ret = rsnd_adg_get_clkout(priv, adg);
>> + if (ret)
>> + return ret;
>>
>> if (of_get_property(np, "clkout-lr-asynchronous", NULL))
>> adg->flags = LRCLK_ASYNC;
>> --
>> 2.11.0
>>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-21 0:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20 14:38 [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference Marek Vasut
2017-04-21 0:14 ` Kuninori Morimoto
2017-04-21 0:14 ` Kuninori Morimoto
2017-04-21 0:40 ` Marek Vasut
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.