| Message ID | 20170420143858.32150-1-marek.vasut+renesas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
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 >>
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;
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(-)