Thread (5 messages) 5 messages, 3 authors, 2016-07-21

[rtc-linux] Re: __rtc_read_alarm missing month/year field bug?

From: Alexandre Belloni <hidden>
Date: 2016-07-19 10:47:22
Also in: lkml

Hi,

On 20/06/2016 at 12:06:38 -0400, Joe Lawrence wrote :
Hello Alessandro and Alexandre,

I noticed an interesting cmos_rtc.rtc.aie_timer on a Stratus machine
running the 4.6 kernel, with an expiration time that puts the alarm way
out into next year.  This is easily reproducible on this machine by
setting a wakealarm sometime in the near future, then rebooting.

From a fresh boot:

  % cat /proc/driver/rtc
  rtc_time        : 17:55:10
  rtc_date        : 2016-06-09
  alrm_time       : 14:04:37
  alrm_date       : 2017-06-09         << 2017 ?
Well, alrm_time is before rtc_time so it either already expired or is set
for the future. The code assume it is set in the future (hence the
rollover).

What you patch will do in that case is effectively always remove the
rollover.
quoted hunk ↗ jump to hunk
  alarm_IRQ       : no
  alrm_pending    : no
  update IRQ enabled      : no
  periodic IRQ enabled    : no
  periodic IRQ frequency  : 1024
  max user IRQ frequency  : 64
  24hr            : yes
  periodic_IRQ    : no
  update_IRQ      : no
  HPET_emulated   : yes
  BCD             : yes
  DST_enable      : no
  periodic_freq   : 1024
  batt_status     : okay


I added some debugging code to the kernel, saw this on the next boot:

  __rtc_read_alarm: A - alarm->time.tm_year = -1, missing = 0
  __rtc_read_alarm: B - alarm->time.tm_year = 116, missing = 3
  __rtc_read_alarm: C - alarm->time.tm_year = 117


Corresponding to these parts of __rtc_read_alarm:

  int __rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
  ...
  	enum { none, day, month, year } missing = none;
  ...
  		err = rtc_read_alarm_internal(rtc, alarm);
  ...
  	/* Fill in the missing alarm fields using the timestamp; we
  	 * know there's at least one since alarm->time is invalid.
  	 */
  ...
  [A]
  	if (alarm->time.tm_year == -1) {
  		alarm->time.tm_year = now.tm_year;
  		if (missing == none)
  			missing = year;
  	}
  [B]
  ...
  	switch (missing) {
  ...
  	/* Year rollover ... easy except for leap years! */
  	case year:
  		dev_dbg(&rtc->dev, "alarm rollover: %s\n", "year");
  		do {
  			alarm->time.tm_year++;
  		} while (!is_leap_year(alarm->time.tm_year + 1900)
  			&& rtc_valid_tm(&alarm->time) != 0);
  [C]		break;


I noticed that the missing year and month cases increment their
respective time units inside a do ... while (condition) loop, pushing
the default 'filled-in' values to now + 1.

Should this 'roll-over' code check for a valid date before incrementing
the alarm time?  (See attached patch.)  I think this might also apply to
a missing month field as well.

(After the patch + reboot):

  % cat /proc/driver/rtc
  rtc_time        : 18:24:02
  rtc_date        : 2016-06-09
  alrm_time       : 14:04:37
  alrm_date       : 2016-06-09
  alarm_IRQ       : no
  alrm_pending    : no
  update IRQ enabled      : no
  periodic IRQ enabled    : no
  periodic IRQ frequency  : 1024
  max user IRQ frequency  : 64
  24hr            : yes
  periodic_IRQ    : no
  update_IRQ      : no
  HPET_emulated   : yes
  BCD             : yes
  DST_enable      : no
  periodic_freq   : 1024
  batt_status     : okay

-- >8 --

From d6feacf20b312c8ebfee902b8b84f68c1a82f035 Mon Sep 17 00:00:00 2001
From: Joe Lawrence <redacted>
Date: Thu, 9 Jun 2016 14:52:28 -0400
Subject: [PATCH] rtc: check filled-in alarm values before incrementing

In __rtc_read_alarm, check filled-in alarm->time.tm_year values (those
not returned by the RTC and defaulted to now.tm_year) before
incrementing them in the rollover handling case.

Signed-off-by: Joe Lawrence <redacted>
---
 drivers/rtc/interface.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 9ef5f6f89f98..3098ce4167ef 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -258,10 +258,10 @@ int __rtc_read_alarm(struct rtc_device *rtc,
struct rtc_wkalrm *alarm)
 	/* Year rollover ... easy except for leap years! */
 	case year:
 		dev_dbg(&rtc->dev, "alarm rollover: %s\n", "year");
-		do {
+		while (!is_leap_year(alarm->time.tm_year + 1900)
+			&& rtc_valid_tm(&alarm->time) != 0) {
 			alarm->time.tm_year++;
-		} while (!is_leap_year(alarm->time.tm_year + 1900)
-			&& rtc_valid_tm(&alarm->time) != 0);
+		}
 		break;

 	default:
-- 
1.8.3.1
-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help