[alsa-devel] [PATCH] ASoC: rsnd: Fix possible NULL pointer dereference

[Marek Vasut]

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 at gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas at glider.be>
>> Cc: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx at renesas.com>
>> Cc: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>> Cc: Mark Brown <broonie at kernel.org>
>> Cc: linux-renesas-soc at 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