Skip to contentSkip to conversation

RTC subsystem: class

PATCH In review Dan Carpenter 2 participants 2 messages Oct 15, 2020 Real time clock (RTC)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RTC subsystem: class

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

#1 lore raw
Quoted 5 lines · Dan Carpenter

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 :)

Quoted 20 lines
	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);
Quoted 4 lines

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.

Signature · 3 lines
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com