[PATCH V2 3/6] mpcore_wdt: Fix WDIOC_SETOPTIONS handling

Russell King - ARM Linux linux at arm.linux.org.uk
Wed Jul 6 09:48:06 EDT 2011


On Tue, Jul 05, 2011 at 11:00:37PM +0400, Vitaly Kuzmichev wrote:
> According to the include/linux/watchdog.h WDIOC_SETOPTIONS is
> classified as 'read from device' ioctl call:
>   #define WDIOC_SETOPTIONS        _IOR(WATCHDOG_IOCTL_BASE, 4, int)
> 
> However, the driver 'mpcore_wdt' performs 'copy_from_user' only if
> _IOC_WRITE is set, thus the local variable 'uarg' which is used in
> WDIOC_SETOPTIONS handling remains uninitialized.
> 
> The proper way to fix this is to bind WDIOC_SETOPTIONS to _IOW,
> but this will break compatibility.
> So adding additional condition for performing 'copy_from_user'.

That looks like something which needs fixing in watchdog land.  And it's
not too difficult:

1. Change the existing WDIOC_SETOPTIONS to WDIOC_SETOPTIONS_OLD
2. Add a new, correctly defined WDIOC_SETOPTIONS.
3. Update all drivers to use WDIOC_SETOPTIONS.
4. Provide a helper which deals with WDIOC_SETOPTIONS_OLD and translates
   it to the proper WDIOC_SETOPTIONS call (this could be just:

	if (cmd == WDIOC_SETOPTIONS_OLD) {
		static int first = 1;
		if (first)
			pr_warn("%s:%d used old WDIOC_SETOPTIONS call.  Please rebuild\n",
				current->comm, current->pid);
		first = 0;
		cmd = WDIOC_SETOPTIONS;
	}

   before any argument copies.

This means you preserve existing compatibility with userspace, yet gain
a path to a non-broken ioctl interface.

As stuff gets rebuilt and replaced, you'll eventually be able to remove
the above.  That period used to be two years for such simple changes.



More information about the linux-arm-kernel mailing list