Re: AT91SAM9/CAP9 watchdog driver

[Alan Cox]

> +static long at91_wdt_ioctl(struct file *file, unsigned int cmd,
> +		unsigned long arg)
> +{
> +	void __user *argp = (void __user *)arg;
> +	int __user *p = argp;

No locking.. so you could get two set timeout calls in parallel. Probably
you need a simple mutex in at91_wdt_settimeout();


> +	res = misc_register(&at91wdt_miscdev);
> +	if (res)
> +		return res;
> +
> +	/* Set watchdog */
> +	if (at91_wdt_settimeout(wdt_timeout) == -EINVAL) {
> +		pr_info("at91sam9_wdt: timeout must be between 1 and %d.\n",
> +			WDT_MAX_TIME);
> +		return 0;

At the moment those two are safe. When the open lock_kernel
goes away it will be possible to get

			misc_register
						open
						ioctl
			wdt_settimout()

So you may want to swap those two around (and disable the timer if the
register fails ?), or lock the open against the register routine.

Otherwise looks good.

Alan
--
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/