rtc: remove "RTC_ALM_SET mode" bugs

This fixes a common glitch in how RTC drivers handle two "set alarm" modes, by getting rid of the surprising/hidden one that was rarely implemented correctly (and which could expose nonportable hardware-specific behavior). The glitch comes from the /dev/rtcX logic implementing the legacy RTC_ALM_SET (limited to 24 hours, needing RTC_AIE_ON) ioctl on top of the RTC driver call providing access to the newer RTC_WKALM_SET (without those limitations) by initializing the day/month/year fields to be invalid ... that second mode. Now, since few RTC drivers check those fields, and most hardware misbehaves when faced with invalid date fields, many RTC drivers will set bogus alarm times on those RTC_ALM_SET code paths. (Several in-tree drivers have that issue, and I also noticed it with code reviews on several new RTC drivers.) This patch ensures that RTC drivers never see such invalid alarm fields, by moving some logic out of rtc-omap into the RTC_ALM_SET code and adding an explicit check (which will prevent the issue on other code paths). Signed-off-by: David Brownell <dbrownell@users.sourceforge.net> Cc: Scott Wood <scottwood@freescale.com> 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: remove "RTC_ALM_SET mode" bugs
This fixes a common glitch in how RTC drivers handle two "set alarm" modes, by getting rid of the surprising/hidden one that was rarely implemented correctly (and which could expose nonportable hardware-specific behavior). The glitch comes from the /dev/rtcX logic implementing the legacy RTC_ALM_SET (limited to 24 hours, needing RTC_AIE_ON) ioctl on top of the RTC driver call providing access to the newer RTC_WKALM_SET (without those limitations) by initializing the day/month/year fields to be invalid ... that second mode. Now, since few RTC drivers check those fields, and most hardware misbehaves when faced with invalid date fields, many RTC drivers will set bogus alarm times on those RTC_ALM_SET code paths. (Several in-tree drivers have that issue, and I also noticed it with code reviews on several new RTC drivers.) This patch ensures that RTC drivers never see such invalid alarm fields, by moving some logic out of rtc-omap into the RTC_ALM_SET code and adding an explicit check (which will prevent the issue on other code paths). Signed-off-by: David Brownell <dbrownell@users.sourceforge.net> Cc: Scott Wood <scottwood@freescale.com> 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>
David Brownell · Linus Torvalds
1 parent 416ce32e70
Showing 4 changed files with 45 additions and 36 deletions Side-by-side Diff
Documentation/rtc.txt
drivers/rtc/interface.c
drivers/rtc/rtc-dev.c
drivers/rtc/rtc-omap.c
@@ -147,7 +147,7 @@
  
     *	RTC_AIE_ON, RTC_AIE_OFF, RTC_ALM_SET, RTC_ALM_READ ... when the RTC
 	is connected to an IRQ line, it can often issue an alarm IRQ up to
-	24 hours in the future.
+	24 hours in the future.  (Use RTC_WKALM_* by preference.)
  
     *	RTC_WKALM_SET, RTC_WKALM_RD ... RTCs that can issue alarms beyond
 	the next 24 hours use a slightly more powerful API, which supports
@@ -175,10 +175,7 @@
 	called with appropriate values.
  
     *	RTC_ALM_SET, RTC_ALM_READ, RTC_WKALM_SET, RTC_WKALM_RD: the
-	set_alarm/read_alarm functions will be called.  To differentiate
-	between the ALM and WKALM, check the larger fields of the rtc_wkalrm
-	struct (like tm_year).  These will be set to -1 when using ALM and
-	will be set to proper values when using WKALM.
+	set_alarm/read_alarm functions will be called.
  
     *	RTC_IRQP_SET, RTC_IRQP_READ: the irq_set_freq function will be called
 	to set the frequency while the framework will handle the read for you
@@ -125,6 +125,10 @@
 {
 	int err;
  
+	err = rtc_valid_tm(&alarm->time);
+	if (err != 0)
+		return err;
+
 	err = mutex_lock_interruptible(&rtc->ops_lock);
 	if (err)
 		return -EBUSY;
@@ -277,12 +277,48 @@
  
 		alarm.enabled = 0;
 		alarm.pending = 0;
-		alarm.time.tm_mday = -1;
-		alarm.time.tm_mon = -1;
-		alarm.time.tm_year = -1;
 		alarm.time.tm_wday = -1;
 		alarm.time.tm_yday = -1;
 		alarm.time.tm_isdst = -1;
+
+		/* RTC_ALM_SET alarms may be up to 24 hours in the future.
+		 * Rather than expecting every RTC to implement "don't care"
+		 * for day/month/year fields, just force the alarm to have
+		 * the right values for those fields.
+		 *
+		 * RTC_WKALM_SET should be used instead.  Not only does it
+		 * eliminate the need for a separate RTC_AIE_ON call, it
+		 * doesn't have the "alarm 23:59:59 in the future" race.
+		 *
+		 * NOTE:  some legacy code may have used invalid fields as
+		 * wildcards, exposing hardware "periodic alarm" capabilities.
+		 * Not supported here.
+		 */
+		{
+			unsigned long now, then;
+
+			err = rtc_read_time(rtc, &tm);
+			if (err < 0)
+				return err;
+			rtc_tm_to_time(&tm, &now);
+
+			alarm.time.tm_mday = tm.tm_mday;
+			alarm.time.tm_mon = tm.tm_mon;
+			alarm.time.tm_year = tm.tm_year;
+			err  = rtc_valid_tm(&alarm.time);
+			if (err < 0)
+				return err;
+			rtc_tm_to_time(&alarm.time, &then);
+
+			/* alarm may need to wrap into tomorrow */
+			if (then < now) {
+				rtc_time_to_tm(now + 24 * 60 * 60, &tm);
+				alarm.time.tm_mday = tm.tm_mday;
+				alarm.time.tm_mon = tm.tm_mon;
+				alarm.time.tm_year = tm.tm_year;
+			}
+		}
+
 		err = rtc_set_alarm(rtc, &alarm);
 		break;
  
@@ -289,34 +289,6 @@
 {
 	u8 reg;
  
-	/* Much userspace code uses RTC_ALM_SET, thus "don't care" for
-	 * day/month/year specifies alarms up to 24 hours in the future.
-	 * So we need to handle that ... but let's ignore the "don't care"
-	 * values for hours/minutes/seconds.
-	 */
-	if (alm->time.tm_mday <= 0
-			&& alm->time.tm_mon < 0
-			&& alm->time.tm_year < 0) {
-		struct rtc_time tm;
-		unsigned long now, then;
-
-		omap_rtc_read_time(dev, &tm);
-		rtc_tm_to_time(&tm, &now);
-
-		alm->time.tm_mday = tm.tm_mday;
-		alm->time.tm_mon = tm.tm_mon;
-		alm->time.tm_year = tm.tm_year;
-		rtc_tm_to_time(&alm->time, &then);
-
-		/* sometimes the alarm wraps into tomorrow */
-		if (then < now) {
-			rtc_time_to_tm(now + 24 * 60 * 60, &tm);
-			alm->time.tm_mday = tm.tm_mday;
-			alm->time.tm_mon = tm.tm_mon;
-			alm->time.tm_year = tm.tm_year;
-		}
-	}
-
 	if (tm2bcd(&alm->time) < 0)
 		return -EINVAL;
...	...	@@ -147,7 +147,7 @@
147	147
148	148	* RTC_AIE_ON, RTC_AIE_OFF, RTC_ALM_SET, RTC_ALM_READ ... when the RTC
149	149	is connected to an IRQ line, it can often issue an alarm IRQ up to
150		- 24 hours in the future.
	150	+ 24 hours in the future. (Use RTC_WKALM_* by preference.)
151	151
152	152	* RTC_WKALM_SET, RTC_WKALM_RD ... RTCs that can issue alarms beyond
153	153	the next 24 hours use a slightly more powerful API, which supports
...	...	@@ -175,10 +175,7 @@
175	175	called with appropriate values.
176	176
177	177	* RTC_ALM_SET, RTC_ALM_READ, RTC_WKALM_SET, RTC_WKALM_RD: the
178		- set_alarm/read_alarm functions will be called. To differentiate
179		- between the ALM and WKALM, check the larger fields of the rtc_wkalrm
180		- struct (like tm_year). These will be set to -1 when using ALM and
181		- will be set to proper values when using WKALM.
	178	+ set_alarm/read_alarm functions will be called.
182	179
183	180	* RTC_IRQP_SET, RTC_IRQP_READ: the irq_set_freq function will be called
184	181	to set the frequency while the framework will handle the read for you
...	...	@@ -125,6 +125,10 @@
125	125	{
126	126	int err;
127	127
	128	+ err = rtc_valid_tm(&alarm->time);
	129	+ if (err != 0)
	130	+ return err;
	131	+
128	132	err = mutex_lock_interruptible(&rtc->ops_lock);
129	133	if (err)
130	134	return -EBUSY;
...	...	@@ -277,12 +277,48 @@
277	277
278	278	alarm.enabled = 0;
279	279	alarm.pending = 0;
280		- alarm.time.tm_mday = -1;
281		- alarm.time.tm_mon = -1;
282		- alarm.time.tm_year = -1;
283	280	alarm.time.tm_wday = -1;
284	281	alarm.time.tm_yday = -1;
285	282	alarm.time.tm_isdst = -1;
	283	+
	284	+ /* RTC_ALM_SET alarms may be up to 24 hours in the future.
	285	+ * Rather than expecting every RTC to implement "don't care"
	286	+ * for day/month/year fields, just force the alarm to have
	287	+ * the right values for those fields.
	288	+ *
	289	+ * RTC_WKALM_SET should be used instead. Not only does it
	290	+ * eliminate the need for a separate RTC_AIE_ON call, it
	291	+ * doesn't have the "alarm 23:59:59 in the future" race.
	292	+ *
	293	+ * NOTE: some legacy code may have used invalid fields as
	294	+ * wildcards, exposing hardware "periodic alarm" capabilities.
	295	+ * Not supported here.
	296	+ */
	297	+ {
	298	+ unsigned long now, then;
	299	+
	300	+ err = rtc_read_time(rtc, &tm);
	301	+ if (err < 0)
	302	+ return err;
	303	+ rtc_tm_to_time(&tm, &now);
	304	+
	305	+ alarm.time.tm_mday = tm.tm_mday;
	306	+ alarm.time.tm_mon = tm.tm_mon;
	307	+ alarm.time.tm_year = tm.tm_year;
	308	+ err = rtc_valid_tm(&alarm.time);
	309	+ if (err < 0)
	310	+ return err;
	311	+ rtc_tm_to_time(&alarm.time, &then);
	312	+
	313	+ /* alarm may need to wrap into tomorrow */
	314	+ if (then < now) {
	315	+ rtc_time_to_tm(now + 24 * 60 * 60, &tm);
	316	+ alarm.time.tm_mday = tm.tm_mday;
	317	+ alarm.time.tm_mon = tm.tm_mon;
	318	+ alarm.time.tm_year = tm.tm_year;
	319	+ }
	320	+ }
	321	+
286	322	err = rtc_set_alarm(rtc, &alarm);
287	323	break;
288	324
...	...	@@ -289,34 +289,6 @@
289	289	{
290	290	u8 reg;
291	291
292		- /* Much userspace code uses RTC_ALM_SET, thus "don't care" for
293		- * day/month/year specifies alarms up to 24 hours in the future.
294		- * So we need to handle that ... but let's ignore the "don't care"
295		- * values for hours/minutes/seconds.
296		- */
297		- if (alm->time.tm_mday <= 0
298		- && alm->time.tm_mon < 0
299		- && alm->time.tm_year < 0) {
300		- struct rtc_time tm;
301		- unsigned long now, then;
302		-
303		- omap_rtc_read_time(dev, &tm);
304		- rtc_tm_to_time(&tm, &now);
305		-
306		- alm->time.tm_mday = tm.tm_mday;
307		- alm->time.tm_mon = tm.tm_mon;
308		- alm->time.tm_year = tm.tm_year;
309		- rtc_tm_to_time(&alm->time, &then);
310		-
311		- /* sometimes the alarm wraps into tomorrow */
312		- if (then < now) {
313		- rtc_time_to_tm(now + 24 * 60 * 60, &tm);
314		- alm->time.tm_mday = tm.tm_mday;
315		- alm->time.tm_mon = tm.tm_mon;
316		- alm->time.tm_year = tm.tm_year;
317		- }
318		- }
319		-
320	292	if (tm2bcd(&alm->time) < 0)
321	293	return -EINVAL;
322	294