linux-kernel - __rtc_read_alarm missing month/year field bug?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <5768148E.8090304@stratus.com>
Date:	Mon, 20 Jun 2016 12:06:38 -0400
From:	Joe Lawrence <joe.lawrence@...atus.com>
To:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	<rtc-linux@...glegroups.com>
CC:	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Joe Lawrence <joe.lawrence@...atus.com>
Subject: __rtc_read_alarm missing month/year field bug?

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 ?
  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 <joe.lawrence@...atus.com>
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 <joe.lawrence@...atus.com>
---
 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