linux-kernel - Re: [PATCH 4/5] omap: Avoid crashes in the case of hwmod misconfiguration

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>] [day] [month] [year] [list]

Date:	Thu, 8 Aug 2013 12:29:00 +0300
From:	Pantelis Antoniou <panto@...oniou-consulting.com>
To:	Kevin Hilman <khilman@...aro.org>
Cc:	Tony Lindgren <tony@...mide.com>,
	Russell King <linux@....linux.org.uk>,
	Benoît Coussno <b-cousson@...com>,
	Paul Walmsley <paul@...an.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sourav Poddar <sourav.poddar@...com>,
	Russ Dill <Russ.Dill@...com>, Felipe Balbi <balbi@...com>,
	Koen Kooi <koen@...cuitco.com>, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] omap: Avoid crashes in the case of hwmod misconfiguration

Hi Kevin,

On Aug 8, 2013, at 12:15 AM, Kevin Hilman wrote:

> Pantelis Antoniou <panto@...oniou-consulting.com> writes:
> 
>> omap hwmod is really sensitive to hwmod misconfiguration.
>> Getting a minor clock wrong always ended up in a crash.
>> Attempt to be more resilient by not assigning variables with
>> error codes and then attempting to use them.
>> 
>> Without this patch, missing a clock ends up with something like this:
>> omap_hwmod: ehrpwm0: cannot clk_get opt_clk ehrpwm0_tbclk!
> 
> Definitely agree we should not be crashing when given bad data.
> 
> nit Re: "missing clock".  I don't think there will be any crash if a
> clock is missing.  This looks to me more like the clock name is wrong
> (tbclk instead of dbclk?), not missing.
> 

Yes, I'll rephrase.

> [...]
> 
>> index 7341eff..42cb7d4 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -784,7 +784,9 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>> 		if (IS_ERR(c)) {
>> 			pr_warning("omap_hwmod: %s: cannot clk_get interface_clk %s\n",
>> 				   oh->name, os->clk);
>> -			ret = -EINVAL;
>> +			if (ret == 0)
>> +				ret = -EINVAL;
>> +			continue;
> 
> the 'if (ret == 0)' adds confusion IMO.  If we don't care additional
> failures, errors, then just add a 'break' instead of these 3 lines.
> 
> [...]
> 


I tried to carry on as much as possible even on the presence of errors.
The remaining clocks won't be initialized, but that might be OK.

> Kevin

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/