[alsa-devel] [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference
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;
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
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
participants (2)
-
Kuninori Morimoto -
Marek Vasut