[PATCH] RTC: Ensure that time being passed to set_alarm() is valid.

[PATCH] RTC: Ensure that time being passed to set_alarm() is valid.

[Andrew Victor]

RTC: Ensure that the time being passed to set_alarm() is valid.


Signed-off-by: Andrew Victor <andrew@sanpeople.com>
Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>


diff -urN -x CVS linux-2.6.17-rc6/drivers/rtc/interface.c
linux-2.6.17-rc/drivers/rtc/interface.c
--- linux-2.6.17-rc6/drivers/rtc/interface.c	Tue Jun  6 10:28:05 2006
+++ linux-2.6.17-rc/drivers/rtc/interface.c	Wed Jun  7 11:46:28 2006
@@ -129,6 +129,10 @@
 	int err;
 	struct rtc_device *rtc = to_rtc_device(class_dev);
 
+	err = rtc_valid_tm(&alarm->time);
+	if (err != 0)
+		return err;
+
 	err = mutex_lock_interruptible(&rtc->ops_lock);
 	if (err)
 		return -EBUSY;

Re: [PATCH] RTC: Ensure that time being passed to set_alarm() is valid.

[Andrew Morton]

On 07 Jun 2006 20:20:55 +0200
Andrew Victor <andrew@sanpeople.com> wrote:

> RTC: Ensure that the time being passed to set_alarm() is valid.
> 
> 
> Signed-off-by: Andrew Victor <andrew@sanpeople.com>
> Signed-off-by: Alessandro Zummo <a.zummo@towertech.it>
> 
> 
> diff -urN -x CVS linux-2.6.17-rc6/drivers/rtc/interface.c
> linux-2.6.17-rc/drivers/rtc/interface.c
> --- linux-2.6.17-rc6/drivers/rtc/interface.c	Tue Jun  6 10:28:05 2006
> +++ linux-2.6.17-rc/drivers/rtc/interface.c	Wed Jun  7 11:46:28 2006
> @@ -129,6 +129,10 @@
>  	int err;
>  	struct rtc_device *rtc = to_rtc_device(class_dev);
>  
> +	err = rtc_valid_tm(&alarm->time);
> +	if (err != 0)
> +		return err;
> +
>  	err = mutex_lock_interruptible(&rtc->ops_lock);
>  	if (err)
>  		return -EBUSY;
> 

More details, please.  How can this situation come about?  Buggy kernel
code?  Userspace action?

Re: [PATCH] RTC: Ensure that time being passed to set_alarm() is valid.

[Alessandro Zummo]

On Wed, 7 Jun 2006 11:39:11 -0700
Andrew Morton <akpm@osdl.org> wrote:

> > diff -urN -x CVS linux-2.6.17-rc6/drivers/rtc/interface.c
> > linux-2.6.17-rc/drivers/rtc/interface.c
> > --- linux-2.6.17-rc6/drivers/rtc/interface.c	Tue Jun  6 10:28:05 2006
> > +++ linux-2.6.17-rc/drivers/rtc/interface.c	Wed Jun  7 11:46:28 2006
> > @@ -129,6 +129,10 @@
> >  	int err;
> >  	struct rtc_device *rtc = to_rtc_device(class_dev);
> >  
> > +	err = rtc_valid_tm(&alarm->time);
> > +	if (err != 0)
> > +		return err;
> > +
> >  	err = mutex_lock_interruptible(&rtc->ops_lock);
> >  	if (err)
> >  		return -EBUSY;
> > 
> 
> More details, please.  How can this situation come about?  Buggy kernel
> code?  Userspace action?

 both of them. this call is used by the dev interface
 but might also be used by any in-kernel user (there are none
 at the moment, but we might have them in the future).

 the same kind of check is done in rtc_set_time()
 for the same reason.

 however, the dev interface (RTC_ALM_SET)
 sets some of the tm fields to -1, which is
 invalid for rtc_valid_tm. 

 I haven't thought of this when I suggested to Andrew Victor
 to add this rtc_valid_tm call.

 so we might have to remove this call or to modify
 rtc_valid_tm to detect this. 

 suggestions?

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Turin, Italy

  http://www.towertech.it

Re: [PATCH] RTC: Ensure that time being passed to set_alarm() is valid.

[Russell King]

On Wed, Jun 07, 2006 at 08:20:55PM +0200, Andrew Victor wrote:
> RTC: Ensure that the time being passed to set_alarm() is valid.

NAK.  rtc_valid_tm checks that the time/date is valid (eg, month is
within range).  Alarms can have a "don't care" state for each part -
for example, setting month to 0xff means "alarm every month".

See the API exposed by /dev/rtc on x86 by virtue of being the
MC146818 register set.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [PATCH] RTC: Ensure that time being passed to set_alarm() is valid.

[Andrew Victor]

hi, 

> > RTC: Ensure that the time being passed to set_alarm() is valid.
> 
> NAK.  rtc_valid_tm checks that the time/date is valid (eg, month is
> within range).  Alarms can have a "don't care" state for each part -
> for example, setting month to 0xff means "alarm every month".

OK. Drop this patch.


Regards,
  Andrew Victor