linux-kernel - Re: [PATCH A 01/10] OMAP2/3: Add non-CORE DPLL rate set code and M, N programming

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090128221056.GJ23301@n2100.arm.linux.org.uk>
Date:	Wed, 28 Jan 2009 22:10:56 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Paul Walmsley <paul@...an.com>
Cc:	linux-arm-kernel@...ts.arm.linux.org.uk,
	linux-kernel@...r.kernel.org, Tony Lindgren <tony@...mide.com>,
	linux-omap@...r.kernel.org
Subject: Re: [PATCH A 01/10] OMAP2/3: Add non-CORE DPLL rate set code and M, N programming

Since it's been posted to lists, comments are going to be made...

On Tue, Jan 27, 2009 at 07:12:47PM -0700, Paul Walmsley wrote:
> +	/*
> +	 * According to the 12-5 CDP code from TI, "Limitation 2.5"
> +	 * on 3430ES1 prevents us from changing DPLL multipliers or dividers
> +	 * on DPLL4.
> +	 */
> +	if (omap_rev() == OMAP3430_REV_ES1_0 &&
> +	    !strcmp("dpll4_ck", clk->name)) {
> +		printk(KERN_ERR "clock: DPLL4 cannot change rate due to "
> +		       "silicon 'Limitation 2.5' on 3430ES1.\n");
> +		return -EINVAL;
> +	}

Yuck.  That's revolting and extremely fragile.  Don't play these games.
You've got plenty of free flag bits in clk->flags which could be used
to prevent the DPLL from being changed.  You've also got other ways to
prevent it - eg, setting dpll_data to NULL.

To do exception handling based on strcmp(), or even worse register offset
and bit position (as is done elsewhere in the OMAP code) is utterly insane.

However, what's worse is that, below...

> +static int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned long rate)
> +{
> +	u16 freqsel;
> +	struct dpll_data *dd;
> +
> +	if (!clk || !rate)
> +		return -EINVAL;
> +
> +	dd = clk->dpll_data;
> +	if (!dd)
> +		return -EINVAL;
> +
> +	if (rate == omap2_get_dpll_rate(clk))
> +		return 0;
> +
> +	if (dd->last_rounded_rate != rate)
> +		omap2_dpll_round_rate(clk, rate);
> +
> +	if (dd->last_rounded_rate == 0)
> +		return -EINVAL;
> +
> +	freqsel = _omap3_dpll_compute_freqsel(clk, dd->last_rounded_n);
> +	if (!freqsel)
> +		WARN_ON(1);
> +
> +	omap3_noncore_dpll_program(clk, dd->last_rounded_m, dd->last_rounded_n,
> +				   freqsel);

The return value from the above test isn't checked, so this function
will succeed even for dpll4_ck.
--
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/