rtc: fix readback from /sys/class/rtc/rtc?/wakealarm

Fix readback of RTC alarms on platforms which return -1 in non-hardware-supported RTC alarm fields. To fill in the missing (-1) values, we grab an RTC timestamp along with the RTC alarm value, and use the timestamp fields to populate the missing alarm fields. To counter field-wrap races (since the timestamp and alarm are not read together atomically), we read the RTC timestamp both before and after reading the RTC alarm value, and then check for wrapped fields --> if any have wrapped, we know we have a possible inconsistency, so we loop and reread the timestamp and alarm again. Wrapped fields in the RTC timestamps are an issue because rtc-cmos.c, for example, also gets/uses an RTC timestamp internally while fetching the RTC alarm. If our timestamp here wasn't the same (minutes and higher) as what was used internally there, then we might end up populating the -1 fields with inconsistent values. This fixes readbacks from /sys/class/rtc/rtc?/wakealarm, as well as other code paths which call rtc_read_alarm(). Signed-off-by: Mark Lord <mlord@pobox.com> Cc: David Brownell <david-b@pacbell.net> Cc: Alessandro Zummo <a.zummo@towertech.it> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

rtc: fix readback from /sys/class/rtc/rtc?/wakealarm
Fix readback of RTC alarms on platforms which return -1 in non-hardware-supported RTC alarm fields. To fill in the missing (-1) values, we grab an RTC timestamp along with the RTC alarm value, and use the timestamp fields to populate the missing alarm fields. To counter field-wrap races (since the timestamp and alarm are not read together atomically), we read the RTC timestamp both before and after reading the RTC alarm value, and then check for wrapped fields --> if any have wrapped, we know we have a possible inconsistency, so we loop and reread the timestamp and alarm again. Wrapped fields in the RTC timestamps are an issue because rtc-cmos.c, for example, also gets/uses an RTC timestamp internally while fetching the RTC alarm. If our timestamp here wasn't the same (minutes and higher) as what was used internally there, then we might end up populating the -1 fields with inconsistent values. This fixes readbacks from /sys/class/rtc/rtc?/wakealarm, as well as other code paths which call rtc_read_alarm(). Signed-off-by: Mark Lord <mlord@pobox.com> Cc: David Brownell <david-b@pacbell.net> Cc: Alessandro Zummo <a.zummo@towertech.it> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Mark Lord · Linus Torvalds
1 parent bf4994d781
Showing 1 changed file with 82 additions and 1 deletions Side-by-side Diff
drivers/rtc/interface.c
@@ -100,7 +100,7 @@
 }
 EXPORT_SYMBOL_GPL(rtc_set_mmss);
  
-int rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
+static int rtc_read_alarm_internal(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
 {
 	int err;
  
@@ -119,6 +119,87 @@
  
 	mutex_unlock(&rtc->ops_lock);
 	return err;
+}
+
+int rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
+{
+	int err;
+	struct rtc_time before, now;
+	int first_time = 1;
+
+	/* The lower level RTC driver may not be capable of filling
+	 * in all fields of the rtc_time struct (eg. rtc-cmos),
+	 * and so might instead return -1 in some fields.
+	 * We deal with that here by grabbing a current RTC timestamp
+	 * and using values from that for any missing (-1) values.
+	 *
+	 * But this can be racey, because some fields of the RTC timestamp
+	 * may have wrapped in the interval since we read the RTC alarm,
+	 * which would lead to us inserting inconsistent values in place
+	 * of the -1 fields.
+	 *
+	 * Reading the alarm and timestamp in the reverse sequence
+	 * would have the same race condition, and not solve the issue.
+	 *
+	 * So, we must first read the RTC timestamp,
+	 * then read the RTC alarm value,
+	 * and then read a second RTC timestamp.
+	 *
+	 * If any fields of the second timestamp have changed
+	 * when compared with the first timestamp, then we know
+	 * our timestamp may be inconsistent with that used by
+	 * the low-level rtc_read_alarm_internal() function.
+	 *
+	 * So, when the two timestamps disagree, we just loop and do
+	 * the process again to get a fully consistent set of values.
+	 *
+	 * This could all instead be done in the lower level driver,
+	 * but since more than one lower level RTC implementation needs it,
+	 * then it's probably best best to do it here instead of there..
+	 */
+
+	/* Get the "before" timestamp */
+	err = rtc_read_time(rtc, &before);
+	if (err < 0)
+		return err;
+	do {
+		if (!first_time)
+			memcpy(&before, &now, sizeof(struct rtc_time));
+		first_time = 0;
+
+		/* get the RTC alarm values, which may be incomplete */
+		err = rtc_read_alarm_internal(rtc, alarm);
+		if (err)
+			return err;
+		if (!alarm->enabled)
+			return 0;
+
+		/* get the "after" timestamp, to detect wrapped fields */
+		err = rtc_read_time(rtc, &now);
+		if (err < 0)
+			return err;
+
+		/* note that tm_sec is a "don't care" value here: */
+	} while (   before.tm_min   != now.tm_min
+		 || before.tm_hour  != now.tm_hour
+		 || before.tm_mon   != now.tm_mon
+		 || before.tm_year  != now.tm_year
+		 || before.tm_isdst != now.tm_isdst);
+
+	/* Fill in any missing alarm fields using the timestamp */
+	if (alarm->time.tm_sec == -1)
+		alarm->time.tm_sec = now.tm_sec;
+	if (alarm->time.tm_min == -1)
+		alarm->time.tm_min = now.tm_min;
+	if (alarm->time.tm_hour == -1)
+		alarm->time.tm_hour = now.tm_hour;
+	if (alarm->time.tm_mday == -1)
+		alarm->time.tm_mday = now.tm_mday;
+	if (alarm->time.tm_mon == -1)
+		alarm->time.tm_mon = now.tm_mon;
+	if (alarm->time.tm_year == -1)
+		alarm->time.tm_year = now.tm_year;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(rtc_read_alarm);
...	...	@@ -100,7 +100,7 @@
100	100	}
101	101	EXPORT_SYMBOL_GPL(rtc_set_mmss);
102	102
103		-int rtc_read_alarm(struct rtc_device rtc, struct rtc_wkalrm alarm)
	103	+static int rtc_read_alarm_internal(struct rtc_device rtc, struct rtc_wkalrm alarm)
104	104	{
105	105	int err;
106	106
...	...	@@ -119,6 +119,87 @@
119	119
120	120	mutex_unlock(&rtc->ops_lock);
121	121	return err;
	122	+}
	123	+
	124	+int rtc_read_alarm(struct rtc_device rtc, struct rtc_wkalrm alarm)
	125	+{
	126	+ int err;
	127	+ struct rtc_time before, now;
	128	+ int first_time = 1;
	129	+
	130	+ /* The lower level RTC driver may not be capable of filling
	131	+ * in all fields of the rtc_time struct (eg. rtc-cmos),
	132	+ * and so might instead return -1 in some fields.
	133	+ * We deal with that here by grabbing a current RTC timestamp
	134	+ * and using values from that for any missing (-1) values.
	135	+ *
	136	+ * But this can be racey, because some fields of the RTC timestamp
	137	+ * may have wrapped in the interval since we read the RTC alarm,
	138	+ * which would lead to us inserting inconsistent values in place
	139	+ * of the -1 fields.
	140	+ *
	141	+ * Reading the alarm and timestamp in the reverse sequence
	142	+ * would have the same race condition, and not solve the issue.
	143	+ *
	144	+ * So, we must first read the RTC timestamp,
	145	+ * then read the RTC alarm value,
	146	+ * and then read a second RTC timestamp.
	147	+ *
	148	+ * If any fields of the second timestamp have changed
	149	+ * when compared with the first timestamp, then we know
	150	+ * our timestamp may be inconsistent with that used by
	151	+ * the low-level rtc_read_alarm_internal() function.
	152	+ *
	153	+ * So, when the two timestamps disagree, we just loop and do
	154	+ * the process again to get a fully consistent set of values.
	155	+ *
	156	+ * This could all instead be done in the lower level driver,
	157	+ * but since more than one lower level RTC implementation needs it,
	158	+ * then it's probably best best to do it here instead of there..
	159	+ */
	160	+
	161	+ /* Get the "before" timestamp */
	162	+ err = rtc_read_time(rtc, &before);
	163	+ if (err < 0)
	164	+ return err;
	165	+ do {
	166	+ if (!first_time)
	167	+ memcpy(&before, &now, sizeof(struct rtc_time));
	168	+ first_time = 0;
	169	+
	170	+ /* get the RTC alarm values, which may be incomplete */
	171	+ err = rtc_read_alarm_internal(rtc, alarm);
	172	+ if (err)
	173	+ return err;
	174	+ if (!alarm->enabled)
	175	+ return 0;
	176	+
	177	+ /* get the "after" timestamp, to detect wrapped fields */
	178	+ err = rtc_read_time(rtc, &now);
	179	+ if (err < 0)
	180	+ return err;
	181	+
	182	+ /* note that tm_sec is a "don't care" value here: */
	183	+ } while ( before.tm_min != now.tm_min
	184	+ \|\| before.tm_hour != now.tm_hour
	185	+ \|\| before.tm_mon != now.tm_mon
	186	+ \|\| before.tm_year != now.tm_year
	187	+ \|\| before.tm_isdst != now.tm_isdst);
	188	+
	189	+ /* Fill in any missing alarm fields using the timestamp */
	190	+ if (alarm->time.tm_sec == -1)
	191	+ alarm->time.tm_sec = now.tm_sec;
	192	+ if (alarm->time.tm_min == -1)
	193	+ alarm->time.tm_min = now.tm_min;
	194	+ if (alarm->time.tm_hour == -1)
	195	+ alarm->time.tm_hour = now.tm_hour;
	196	+ if (alarm->time.tm_mday == -1)
	197	+ alarm->time.tm_mday = now.tm_mday;
	198	+ if (alarm->time.tm_mon == -1)
	199	+ alarm->time.tm_mon = now.tm_mon;
	200	+ if (alarm->time.tm_year == -1)
	201	+ alarm->time.tm_year = now.tm_year;
	202	+ return 0;
122	203	}
123	204	EXPORT_SYMBOL_GPL(rtc_read_alarm);
124	205