Re: [PATCH] Enforce valid shift values in clocksource_register()

[Thomas Gleixner]

On Wed, 26 Nov 2008, John Stultz wrote:

> Thomas noted that some of the timekeeping core assumes 
> clocksource shift values will be less then 32. However, 
> we do nothing to enforce such limits.
> 
> This patch prints a warning if a clocksource with an invalid
> shift value is attempted to be registered, and will refuse to
> register the clocksource
> 
> I could not find any clocksources that use 32 or higher for a 
> shift value, so this should be only a preventive warning for
> future clocksource writers.
> 
> Signed-off-by: John Stultz <johnstul@xxxxxxxxxx>
> 
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 9ed2eec..4a3bb13 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -318,7 +318,7 @@ static int clocksource_enqueue(struct clocksource *c)
>   * clocksource_register - Used to install new clocksources
>   * @t:		clocksource to be registered
>   *
> - * Returns -EBUSY if registration fails, zero otherwise.
> + * Returns -EBUSY or -EINVAL if registration fails, zero otherwise.
>   */
>  int clocksource_register(struct clocksource *c)
>  {
> @@ -327,7 +327,14 @@ int clocksource_register(struct clocksource *c)
>  
>  	/* save mult_orig on registration */
>  	c->mult_orig = c->mult;
> -
> +	if (c->shift >= 32) {
> +		printk(KERN_WARNING "===============================\n");
> +		printk(KERN_WARNING "WARNING: Cannot register %s clocksource\n",
> +					c->name);
> +		printk(KERN_WARNING "The shift value must be less then 32\n");
> +		printk(KERN_WARNING "===============================\n");
> +		return -EINVAL;

Just setting the shift value to 31 along with a WARN_ON() should be
enough. We don't need to kill the clocksource in that case, as this
can be nasty when it happens in the early boot code where we dont have
any output.

Thanks,

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