Re: RTC wakealarm write-only, still has 644 permissions

[Pavel Machek <pavel@ucw.cz>]

Hi!

> > 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 ...

We have 3 interfaces to one hardware resource. I do not think kernel
should try to arbitrate it here. There's just one alarm clock with
three interfaces.

> 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.

I do not believe current interface has any users. ...or any users that
would break anyway.

> 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.

Well, I could not get it to work with this "avoid-clobber" feature.

> > 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 we should just remove the 'avoid-clobber' logic. If userland
wants to somehow arbitrate access, it can.

Now, if we want to provide "nice" interface for timed sleep, I think
we can. Actually, I'd like to use normal select() as such an
interface, and enable kernel to automatically detect when it can sleep
and when it has to wake up.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html