Message-Id: <200711291010.12707.david-b@pacbell.net>
Date:	Thu, 29 Nov 2007 10:10:12 -0800
From:	David Brownell <david-b@...bell.net>
To:	Pavel Machek <pavel@....cz>
Cc:	rtc-linux@...glegroups.com,
	kernel list <linux-kernel@...r.kernel.org>,
	Alessandro Zummo <alessandro.zummo@...ertech.it>
Subject: Re: RTC wakealarm write-only, still has 644 permissions

On Wednesday 28 November 2007, Pavel Machek wrote:
> > Are you sure it's not working?  Other than the two issues I noted
> > above -- borkage w.r.t. ACPI (which wasn't necessarily shown in your
> > scripts above), and with wildcarding -- it looked to be correct.
> 
> It seems to be working now, not sure what changed.

There were some patches from Mark Lord to update the wildcarding,
and to hibernation to work better with ACPI.  I still don't think
the wildcarding is done quite right; there are some date-wrap cases
that looked wrong.


> rtc-sysfs.c: why this?
> 
>         if (alarm > now) {
>                 /* Avoid accidentally clobbering active alarms; we can't
>                  * entirely prevent that here, without even the minimal
>                  * locking from the /dev/rtcN api.
>                  */
>                 retval = rtc_read_alarm(rtc, &alm);
>                 if (retval < 0)
>                         return retval;
>                 if (alm.enabled)
>                         return -EBUSY;
> 
>                 alm.enabled = 1;
> 
> People should not be "accidentally" writing to sysfs files...

It's not an issue of accidental writes, it's an issue of there being
no other synchronization for setting those alarms.  Remember that both
RTC_WKALM_SET and RTC_ALM_SET ioctls can set that same alarm, and so
could a different userspace activity ...

If there's no alarm set, it's clear that changing the (single) alarm
can't cause event lossage.  But if an alarm *is* set it sure seems
that changing it would discard an event that something was expecting,
and thus cause breakage.

As written, this allows one userspace activity to clobber another if
it does so explicitly, by first disabling the other one and then
setting its own alarm.  But the idea is to minimize "accidents" like
unintentionally clobbering an alarm set by someone else.


> If I remove "accidental alarm modify" logic, I can actually use rtc to
> wake up more than once per boot.

Evidently the alarm isn't being disabled then...

I think the issue there is that the alarm isn't acting as a one-shot
event.  There are some model questions lurking there ... if the RTC
has a sane alarm model, "fire at YYYY-MM-DD at HH:MM:SS", then it's
naturally one-shot.

But if the YYYY-MM-DD part is a wildcard (or equivalently, ignored)
at the hardware level, that model is awkward.  You can't easily look
at such alarms and know if they already triggered -- especially after
hibernation.  With respect to rtc-cmos, it'd always be practical to
disable the alarm after it triggers while the system is running.
(Not currently done, pending resolution of such issues.)  And there's
an ACPI flag -- currently ignored by Linux, or maybe trashed -- to
report whether the system wakeup was triggered by an alarm; that
could be read, and used to disable the alarm.

I think the right thing to do there is just insist that in the RTC
framework, alarms should always follow the one-shot model.

- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists