linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gabriele Mazzotta <gabriele.mzt@gmail.com>
To: a.zummo@towertech.it, alexandre.belloni@free-electrons.com
Cc: rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org,
	Gabriele Mazzotta <gabriele.mzt@gmail.com>
Subject: [PATCH v2] rtc-cmos: Reject unsupported alarm values
Date: Fri,  2 Sep 2016 21:55:16 +0200	[thread overview]
Message-ID: <20160902195516.7068-1-gabriele.mzt@gmail.com> (raw)
In-Reply-To: <20160831225929.20336-1-gabriele.mzt@gmail.com>

Some platforms allows to specify the month and day of the month in
which an alarm should go off, some others the day of the month and
some others just the time.

Currently any given value is accepted by the driver and only the
supported fields are used to program the hardware. As consequence,
alarms are potentially programmed to go off in the wrong moment.

Fix this by rejecting any value not supported by the hardware.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---

I revisited the naive implementation of v1. I tested the new
algorithm using some dates that and verified that it behaved as
expected, but I might have missed some corner cases.

I made some assumptions that maybe should be dropped, at least
two of them. They are commented in the code, but I didn't mention
that they are assumptions:

 - If the day can't be specified, the alarm can only be set to go
   off 24 hours minus 1 second in the future. I'm worried things
   would go wrong if the current time is used to set an alarm that
   should go off the next day.
 - If the mday can be specified and the next month has more days
   than the current month, the alarm can be set to go off in the
   extra days of the next month.
 - If the month can be specified, it's the 28th of February and the
   next year is a leap year, the alarm can be set for the 29th of
   February of the next year.

Basically I'm assuming that the hardware decides when an alarm should
go off comparing the current date with the one programmed. If they
match, the alarm goes off. This seemed reasonable to me, but it's
not easy to verify.

 drivers/rtc/rtc-cmos.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 4cdb335..37cb7c1 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -328,14 +328,118 @@ static void cmos_irq_disable(struct cmos_rtc *cmos, unsigned char mask)
 	cmos_checkintr(cmos, rtc_control);
 }
 
+static int cmos_validate_alarm(struct device *dev, struct rtc_wkalrm *t)
+{
+	struct cmos_rtc *cmos = dev_get_drvdata(dev);
+	struct rtc_time now;
+
+	cmos_read_time(dev, &now);
+
+	if (!cmos->day_alrm) {
+		time64_t t_max_date;
+		time64_t t_alrm;
+
+		t_alrm = rtc_tm_to_time64(&t->time);
+		t_max_date = rtc_tm_to_time64(&now);
+		/*
+		 * Subtract 1 second to ensure that the alarm time is
+		 * different from the current time.
+		 */
+		t_max_date += 24 * 60 * 60 - 1;
+		if (t_alrm > t_max_date) {
+			dev_err(dev,
+				"Alarms can be up to one day in the future\n");
+			return -EINVAL;
+		}
+	} else if (!cmos->mon_alrm) {
+		struct rtc_time max_date = now;
+		time64_t t_max_date;
+		time64_t t_alrm;
+		int max_mday;
+		bool is_max_mday = false;
+
+		/*
+		 * If the next month has more days than the current month
+		 * and we are at the max mday of this month, we can program
+		 * the alarm to go off the max mday of the next month without
+		 * it going off sooner than expected.
+		 */
+		max_mday = rtc_month_days(now.tm_mon, now.tm_year);
+		if (now.tm_mday == max_mday)
+			is_max_mday = true;
+
+		if (max_date.tm_mon == 11) {
+			max_date.tm_mon = 0;
+			max_date.tm_year += 1;
+		} else {
+			max_date.tm_mon += 1;
+		}
+		max_mday = rtc_month_days(max_date.tm_mon, max_date.tm_year);
+		if (max_date.tm_mday > max_mday || is_max_mday)
+			max_date.tm_mday = max_mday;
+
+		max_date.tm_hour = 23;
+		max_date.tm_min = 59;
+		max_date.tm_sec = 59;
+
+		t_max_date = rtc_tm_to_time64(&max_date);
+		t_alrm = rtc_tm_to_time64(&t->time);
+		if (t_alrm > t_max_date) {
+			dev_err(dev,
+				"Alarms can be up to one month in the future\n");
+			return -EINVAL;
+		}
+	} else {
+		struct rtc_time max_date = now;
+		time64_t t_max_date;
+		time64_t t_alrm;
+		int max_mday;
+		bool allow_leap_day = false;
+
+		/*
+		 * If it's the 28th of February and the next year is a leap
+		 * year, allow to set alarms for the 29th of February.
+		 */
+		if (now.tm_mon == 1) {
+			max_mday = rtc_month_days(now.tm_mon, now.tm_year);
+			if (now.tm_mday == max_mday)
+				allow_leap_day = true;
+		}
+
+		max_date.tm_year += 1;
+		max_mday = rtc_month_days(max_date.tm_mon, max_date.tm_year);
+		if (max_date.tm_mday > max_mday || allow_leap_day)
+			max_date.tm_mday = max_mday;
+
+		max_date.tm_hour = 23;
+		max_date.tm_min = 59;
+		max_date.tm_sec = 59;
+
+		t_max_date = rtc_tm_to_time64(&max_date);
+		t_alrm = rtc_tm_to_time64(&t->time);
+		if (t_alrm > t_max_date) {
+			dev_err(dev,
+				"Alarms can be up to one year in the future\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
 	unsigned char mon, mday, hrs, min, sec, rtc_control;
+	int ret;
 
 	if (!is_valid_irq(cmos->irq))
 		return -EIO;
 
+	ret = cmos_validate_alarm(dev, t);
+	if (ret < 0)
+		return ret;
+
 	mon = t->time.tm_mon + 1;
 	mday = t->time.tm_mday;
 	hrs = t->time.tm_hour;
-- 
2.9.3

  parent reply	other threads:[~2016-09-02 19:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-31 22:59 [PATCH] rtc-cmos: Reject unsupported alarm values Gabriele Mazzotta
2016-08-31 23:41 ` Gabriele Mazzotta
2016-09-02 19:55 ` Gabriele Mazzotta [this message]
2016-09-21 23:29   ` [PATCH v2] " Alexandre Belloni
2016-09-22 19:47     ` Gabriele Mazzotta
2016-10-03 22:50   ` [PATCH v3] " Gabriele Mazzotta
2016-10-19  7:41     ` Alexandre Belloni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160902195516.7068-1-gabriele.mzt@gmail.com \
    --to=gabriele.mzt@gmail.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rtc-linux@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).