[bug report] [PATCH] RTC subsystem: class

[bug report] [PATCH] RTC subsystem: class

[Dan Carpenter]

Hello Alessandro Zummo,

The patch 0c86edc0d497: "[PATCH] RTC subsystem: class" from Mar 27,
2006, leads to the following static checker warning:

	drivers/rtc/rtc-r9701.c:109 r9701_set_datetime()
	error: undefined (user controlled) shift '1 << dt->tm_wday'

drivers/rtc/rtc-r9701.c
    95  static int r9701_set_datetime(struct device *dev, struct rtc_time *dt)
    96  {
    97          int ret, year;
    98  
    99          year = dt->tm_year + 1900;
   100          if (year >= 2100 || year < 2000)
   101                  return -EINVAL;
   102  
   103          ret = write_reg(dev, RHRCNT, bin2bcd(dt->tm_hour));
   104          ret = ret ? ret : write_reg(dev, RMINCNT, bin2bcd(dt->tm_min));
   105          ret = ret ? ret : write_reg(dev, RSECCNT, bin2bcd(dt->tm_sec));
   106          ret = ret ? ret : write_reg(dev, RDAYCNT, bin2bcd(dt->tm_mday));
   107          ret = ret ? ret : write_reg(dev, RMONCNT, bin2bcd(dt->tm_mon + 1));
   108          ret = ret ? ret : write_reg(dev, RYRCNT, bin2bcd(dt->tm_year - 100));
   109          ret = ret ? ret : write_reg(dev, RWKCNT, 1 << dt->tm_wday);

I would have expected that rtc_valid_tm() would check that dt->tm_wday
as valid but it doesn't.  As far as I can see dt->tm_wday can be set to
any int value in the rtc_dev_ioctl(). 

   110  
   111          return ret;
   112  }

regards,
dan carpenter

Re: [bug report] [PATCH] RTC subsystem: class

[Alexandre Belloni]

On 15/10/2020 12:58:51+0300, Dan Carpenter wrote:
> Hello Alessandro Zummo,
> 
> The patch 0c86edc0d497: "[PATCH] RTC subsystem: class" from Mar 27,
> 2006, leads to the following static checker warning:
> 

Wow, 14 years ago :)

> 	drivers/rtc/rtc-r9701.c:109 r9701_set_datetime()
> 	error: undefined (user controlled) shift '1 << dt->tm_wday'
> 
> drivers/rtc/rtc-r9701.c
>     95  static int r9701_set_datetime(struct device *dev, struct rtc_time *dt)
>     96  {
>     97          int ret, year;
>     98  
>     99          year = dt->tm_year + 1900;
>    100          if (year >= 2100 || year < 2000)
>    101                  return -EINVAL;
>    102  
>    103          ret = write_reg(dev, RHRCNT, bin2bcd(dt->tm_hour));
>    104          ret = ret ? ret : write_reg(dev, RMINCNT, bin2bcd(dt->tm_min));
>    105          ret = ret ? ret : write_reg(dev, RSECCNT, bin2bcd(dt->tm_sec));
>    106          ret = ret ? ret : write_reg(dev, RDAYCNT, bin2bcd(dt->tm_mday));
>    107          ret = ret ? ret : write_reg(dev, RMONCNT, bin2bcd(dt->tm_mon + 1));
>    108          ret = ret ? ret : write_reg(dev, RYRCNT, bin2bcd(dt->tm_year - 100));
>    109          ret = ret ? ret : write_reg(dev, RWKCNT, 1 << dt->tm_wday);
> 
> I would have expected that rtc_valid_tm() would check that dt->tm_wday
> as valid but it doesn't.  As far as I can see dt->tm_wday can be set to
> any int value in the rtc_dev_ioctl(). 
> 

Nobody cares about tm_wday and userspace usually passes 0. For now, it
is left to individual rtc drivers to check and correct tm_wday when they
care in order to avoid having more calculations is the path for all the
other RTCs that don't care.

Honestly, for this RTC, this doesn't matter much because the value
isn't even read back.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com