RTC wakealarm write-only, still has 644 permissions

RTC wakealarm write-only, still has 644 permissions

[Pavel Machek]

...should they be changed to 200? Or perhaps file should be readable?

root@amd:/sys/class/rtc/rtc0# cat wakealarm 
root@amd:/sys/class/rtc/rtc0# echo 132719 > wakealarm 
root@amd:/sys/class/rtc/rtc0# ls -al wakealarm 
-rw-r--r-- 1 root root 0 Sep 20 12:30 wakealarm
root@amd:/sys/class/rtc/rtc0# cat wakealarm 
root@amd:/sys/class/rtc/rtc0# cat wakealarm 
root@amd:/sys/class/rtc/rtc0# 


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

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

[Pavel Machek]

Hi!

> ...should they be changed to 200? Or perhaps file should be readable?
> 
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> root@amd:/sys/class/rtc/rtc0# echo 132719 > wakealarm 
> root@amd:/sys/class/rtc/rtc0# ls -al wakealarm 
> -rw-r--r-- 1 root root 0 Sep 20 12:30 wakealarm
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> root@amd:/sys/class/rtc/rtc0# 
> 
> 
> ...standard PC with reasonably recent kernel...

Hmm, something is definitely wrong in here. I sometimes _do_ get
something back.

root@amd:~# s2ram
Switching from vt9 to vt1


switching back to vt9
root@amd:~# 
root@amd:~# 
root@amd:~# cd /sys/class/rtc/rtc0
root@amd:/sys/class/rtc/rtc0# ls
date  dev  device@  name  power/  since_epoch  subsystem@  time
uevent  wakealarm
root@amd:/sys/class/rtc/rtc0# cat wakealarm 
2051629528
root@amd:/sys/class/rtc/rtc0# cat power/wakeup 

root@amd:/sys/class/rtc/rtc0# cat wakealarm 
2051629528
root@amd:/sys/class/rtc/rtc0# date +%s
1190285030
root@amd:/sys/class/rtc/rtc0# echo 1190285050 > wakealarm 
root@amd:/sys/class/rtc/rtc0# s2ram
Switching from vt9 to vt1


switching back to vt9
root@amd:/sys/class/rtc/rtc0# 
root@amd:/sys/class/rtc/rtc0# 
root@amd:/sys/class/rtc/rtc0# cat wakealarm 
root@amd:/sys/class/rtc/rtc0# cat wakealarm 
root@amd:/sys/class/rtc/rtc0# date +%s
1190285229
root@amd:/sys/class/rtc/rtc0# cat wakealarm 
root@amd:/sys/class/rtc/rtc0# 

Also, is there some documentation for wakealarm?

sh-3.1$ cd Documentation/
sh-3.1$ grep -ri wakealarm .

...is it time in seconds? In UTC? In whatever timezone rtc clocks are
using? It does not seem to work for me :-(.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[David Brownell]

On Thursday 20 September 2007, Pavel Machek wrote:
> Hi!
> 
> > ...should they be changed to 200? Or perhaps file should be readable?

No, mode 644 is fine.  No reason to prevent "other" people from
reading the alarm time (is there?) and if you write a legal value,
that will work.  So $SUBJECT is no problem at all.


> > 
> > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > root@amd:/sys/class/rtc/rtc0# echo 132719 > wakealarm 

At which point I'd expect

# echo $?

would indicate the write failed.  That's a LONG time in the
past (January 2, 1970), so that setting would be rejected.


> > root@amd:/sys/class/rtc/rtc0# ls -al wakealarm 
> > -rw-r--r-- 1 root root 0 Sep 20 12:30 wakealarm
> > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > root@amd:/sys/class/rtc/rtc0# cat wakealarm 

The alarm isn't set; so no value gets displayed.


> > root@amd:/sys/class/rtc/rtc0# 
> > 
> > 
> > ...standard PC with reasonably recent kernel...

Yeah, well a "standard PC" is chock full of fairly bizarrely
glitchey hardware.  Clocks and timers have more than their
fair share, or x86_64 NOHZ support would be merged by now!


> Hmm, something is definitely wrong in here. I sometimes _do_ get
> something back.
> 
> root@amd:~# s2ram
> Switching from vt9 to vt1
> 
> 
> switching back to vt9
> root@amd:~# 
> root@amd:~# 
> root@amd:~# cd /sys/class/rtc/rtc0
> root@amd:/sys/class/rtc/rtc0# ls
> date  dev  device@  name  power/  since_epoch  subsystem@  time
> uevent  wakealarm
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> 2051629528
> root@amd:/sys/class/rtc/rtc0# cat power/wakeup 
> 
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> 2051629528
> root@amd:/sys/class/rtc/rtc0# date +%s
> 1190285030

OK, in that situation you've definitely got some buglike behavior.
My question is:  how to fix it?

The problem is that the RTC is reporting an alarm value with some
fields flagged as "wildcard" -- e.g. day/month/year "out of range"
so the hardware ignores those fields.  This is very common on PC
based RTCs, and much less common on embedded systems.  (Which for
some reason don't tend to cheap out on full date specs like PCs.)

And those cause date reports to look like garbage; /proc/driver/rtc
would show "**" in those fields, rather than trying to display the
canonical "seconds since POSIX epoch" value.  But the wakealarm code
just calls rtc_tm_to_time(), which doesn't validate its fields and
so will gladly spew the garbage you saw.  (On PCs especially.  This
code was originally tested on sane embedded hardware.)

Now, in the /dev/rtcX code there's some code working with a similar
problem:  ioctl(RTC_ALM_SET) morphs partial alarm dates into valid
form before passing them down.  This needs the same kind of fix,
but going in the other direction -- and not always kicking in.  That
could go into either the wakealarm display code, or rtc_read_alarm(),
or maybe someplace else.

I'm not sure which fix would be best; maybe Alessandro has an opinion.

I'd lean towards just fixing the wakealarm display code, except that
would force anyone using that other routine to know about this rude
"wildcard" convention, which is rather hardware-specific... and that's
not really aligned with the goal of an RTC framework that "just works"
without needing to know about such quirks.


> root@amd:/sys/class/rtc/rtc0# echo 1190285050 > wakealarm 

That is, 20 seconds from "now" modulo timezone offsets.

Better might be

	echo $(( $(cat since_epoch) + 20 )) > wakealarm

which has no timezone offset issues.


> root@amd:/sys/class/rtc/rtc0# s2ram
> Switching from vt9 to vt1
> 
> 
> switching back to vt9
> root@amd:/sys/class/rtc/rtc0# 
> root@amd:/sys/class/rtc/rtc0# 
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> root@amd:/sys/class/rtc/rtc0# cat wakealarm 

There's some wierdness related to ACPI, that crept in sometime
late in 2.6.21 (or thereabouts) ... where the RTC wake mechanism
got broken by redefining the pm_ops functions, for hibernation
at least.

That MIGHT be related to what you observe here ... unclear what
that was supposed to show.  If the RTC alarm woke that system
after 20 seconds, that's what you requested and all is fine.  If
not, and you had to wake it by hand, then you're seeing that issue
with the redefinition of hibernation ops having borked the RTC wake
mechanism interactions with ACPI.

In both cases, I'd expect that the result is that no alarm is
pending any more, so there's nothing to display.


> root@amd:/sys/class/rtc/rtc0# date +%s
> 1190285229

... which BTW should be what the "since_epoch" file shows,
other than the timezone offsets on some system RTCs.


> root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> root@amd:/sys/class/rtc/rtc0# 
> 
> Also, is there some documentation for wakealarm?

"git show 3925a5ce44330767f7f0de5c58c6a797009f0f75" has some.


> sh-3.1$ cd Documentation/
> sh-3.1$ grep -ri wakealarm .
> 
> ...is it time in seconds? In UTC? In whatever timezone rtc clocks are
> using? It does not seem to work for me :-(.

Time in seconds since the POSIX epoch.  Same units as "since_epoch",
which has even less documentation...


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.

- Dave

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

[Pavel Machek]

Hi!

> > > ...should they be changed to 200? Or perhaps file should be readable?
> 
> No, mode 644 is fine.  No reason to prevent "other" people from
> reading the alarm time (is there?) and if you write a legal value,
> that will work.  So $SUBJECT is no problem at all.

Yep, agreed. I was confused by fact that it does not give invalid
values back. 

> > > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > > root@amd:/sys/class/rtc/rtc0# echo 132719 > wakealarm 
> 
> At which point I'd expect
> 
> # echo $?
> 
> would indicate the write failed.  That's a LONG time in the
> past (January 2, 1970), so that setting would be rejected.

echo $? says 0 here :-(.

> > > root@amd:/sys/class/rtc/rtc0# 
> > > 
> > > ...standard PC with reasonably recent kernel...
> 
> Yeah, well a "standard PC" is chock full of fairly bizarrely
> glitchey hardware.  Clocks and timers have more than their
> fair share, or x86_64 NOHZ support would be merged by now!

:-). Ok. Thinkpad x60.
								Pavel


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

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

[David Brownell]

> > > > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > > > root@amd:/sys/class/rtc/rtc0# echo 132719 > wakealarm 
> > 
> > At which point I'd expect
> > 
> > # echo $?
> > 
> > would indicate the write failed.  That's a LONG time in the
> > past (January 2, 1970), so that setting would be rejected.
>
> echo $? says 0 here :-(.

I stand corrected.  What it should do -- and does -- in that case
involves disabling the alarm, then succeeding.

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

[Pavel Machek]

Hi!

Sorry for long delay. I got rtc wakeup to work... once per boot. On
2.6.24-rc3.


> > > root@amd:/sys/class/rtc/rtc0# ls -al wakealarm 
> > > -rw-r--r-- 1 root root 0 Sep 20 12:30 wakealarm
> > > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> 
> The alarm isn't set; so no value gets displayed.
> 
> 
> > > root@amd:/sys/class/rtc/rtc0# 
> > > 
> > > 
> > > ...standard PC with reasonably recent kernel...
> 
> Yeah, well a "standard PC" is chock full of fairly bizarrely
> glitchey hardware.  Clocks and timers have more than their
> fair share, or x86_64 NOHZ support would be merged by now!

Oops. Ok, this is thinkpad x60, if that helps.

> > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > 2051629528
> > root@amd:/sys/class/rtc/rtc0# date +%s
> > 1190285030
> 
> OK, in that situation you've definitely got some buglike behavior.
> My question is:  how to fix it?
> 
> The problem is that the RTC is reporting an alarm value with some
> fields flagged as "wildcard" -- e.g. day/month/year "out of range"
> so the hardware ignores those fields.  This is very common on PC
> based RTCs, and much less common on embedded systems.  (Which for
> some reason don't tend to cheap out on full date specs like PCs.)

I see... so situation is not nice :-(.

> I'm not sure which fix would be best; maybe Alessandro has an
> opinion.

Alessandro?

> Better might be
> 
> 	echo $(( $(cat since_epoch) + 20 )) > wakealarm
> 
> which has no timezone offset issues.

Good. that actually works.

> > root@amd:/sys/class/rtc/rtc0# cat wakealarm 
> > root@amd:/sys/class/rtc/rtc0# 
> > 
> > Also, is there some documentation for wakealarm?
> 
> "git show 3925a5ce44330767f7f0de5c58c6a797009f0f75" has some.

Thanks. Will put it into Doc*/rtc.txt.

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

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...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Tino Keitel]

On Thu, Nov 29, 2007 at 00:26:47 +0100, Pavel Machek wrote:

[...]

> > > Also, is there some documentation for wakealarm?
> > 
> > "git show 3925a5ce44330767f7f0de5c58c6a797009f0f75" has some.
> 
> Thanks. Will put it into Doc*/rtc.txt.

It would be nice if you mention the differences to the old
/proc/acpi/alarm, to help users who want to migrate. Something like in
http://lkml.org/lkml/2007/6/19/264.

Regards,
Tino

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

[David Brownell]

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

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

[Alessandro Zummo]

On Thu, 29 Nov 2007 10:10:12 -0800
David Brownell <david-b@pacbell.net> wrote:

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

 /me agrees.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

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

[Pavel Machek]

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

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

[David Brownell]

On Friday 30 November 2007, Pavel Machek wrote:
> > 
> > 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.

Having three interfaces is bad enough ... ensuring that none of
them can ever be used safely would be stupid.


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

I had earlier pointed out a different issue, whereby "oneshot"
semantics weren't consistently followed.  I've been working on
some patches to address that.  The ACPI bits still need work,
but I'll forward one part soonish.


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

    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
That's the issue addressed by those patches.  (Specific to rtc-cmos,
not to RTCs on saner hardware.)


> I think we should just remove the 'avoid-clobber' logic. If userland
> wants to somehow arbitrate access, it can.

Pray tell, *HOW* could it arbitrate?


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

Yeah, what ever happened to timerfd?  :)


> and enable kernel to automatically detect when it can sleep 
> and when it has to wake up.

There's the "rtcwake" thing.  That got merged into util-linux-ng, but
I happened to notice its setup_alarm() is calling gmtime() instead
of localtime() ... my suspicion is that the uClibc version I was using
had some timezone bugs, or else there's some other bug lurking in the
vicinity of dual-bootiness.

Note that "wakealarm" is not intende to be a "timed sleep" interface
at all ... it's just to set the wakealarm.  Something else is in charge
of putting the system to sleep, such as "echo mem > /sys/power/state"
or some GUI thing hooked up to logic that knows about how to make frame
buffers recover.

- Dave

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

[Mark Lord]

David Brownell wrote:
> On Friday 30 November 2007, Pavel Machek wrote:
>>> 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.
> 
> Having three interfaces is bad enough ... ensuring that none of
> them can ever be used safely would be stupid.
> 
> 
>>> 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.
> 
> I had earlier pointed out a different issue, whereby "oneshot"
> semantics weren't consistently followed.  I've been working on
> some patches to address that.  The ACPI bits still need work,
> but I'll forward one part soonish.
> 
> 
>>>> 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...
> 
>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> That's the issue addressed by those patches.  (Specific to rtc-cmos,
> not to RTCs on saner hardware.)
> 
> 
>> I think we should just remove the 'avoid-clobber' logic. If userland
>> wants to somehow arbitrate access, it can.
> 
> Pray tell, *HOW* could it arbitrate?
...

This is really a non-issue in practice.  The thing requires root access,
and there's only a single user at most for it on a given system.

This is used by media boxes to power off (or suspend) between recording times.
And similar stuff.

It might be nice to fix it all, but the current state really isn't hurting anything.

Cheers

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

[Pavel Machek]

On Fri 2007-11-30 16:20:58, Mark Lord wrote:
> David Brownell wrote:
>> On Friday 30 November 2007, Pavel Machek wrote:
>>>> 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.
>> Having three interfaces is bad enough ... ensuring that none of
>> them can ever be used safely would be stupid.
>>>> 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.
>> I had earlier pointed out a different issue, whereby "oneshot"
>> semantics weren't consistently followed.  I've been working on
>> some patches to address that.  The ACPI bits still need work,
>> but I'll forward one part soonish.
>>>>> 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...
>>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> That's the issue addressed by those patches.  (Specific to rtc-cmos,
>> not to RTCs on saner hardware.)
>>> I think we should just remove the 'avoid-clobber' logic. If userland
>>> wants to somehow arbitrate access, it can.
>> Pray tell, *HOW* could it arbitrate?
> ...
>
> This is really a non-issue in practice.  The thing requires root access,
> and there's only a single user at most for it on a given system.
>
> This is used by media boxes to power off (or suspend) between recording 
> times.
> And similar stuff.
>
> It might be nice to fix it all, but the current state really isn't hurting 
> anything.

Exactly. If you wanted arbitration, just create "rtcd", and make users
talk to it over sockets or something. Actually openmoko has neod,
which does that iirc.
								Pavel

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

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

[Pavel Machek]

Hi!

> > > 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.
> 
> Having three interfaces is bad enough ... ensuring that none of
> them can ever be used safely would be stupid.

They can be used safely. You just have to pick one and stay with
it. (And no, no-clobber hack does not help here. Or do you have
specific application where no-clobber hack helps?)

Anyway, with wildcarded dates, no-clobber is a problem -- because you
need to kill the alarm after you waken up, or it will repeat.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[David Brownell]

On Sunday 02 December 2007, Pavel Machek wrote:
> Anyway, with wildcarded dates, no-clobber is a problem -- because you
> need to kill the alarm after you waken up, or it will repeat.

And I've started to fix that problem.  Alarms need to
act only in oneshot mode for other reasons too.

- Dave

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

[Pavel Machek]

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

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

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/drivers/rtc/rtc-sysfs.c b/drivers/rtc/rtc-sysfs.c
index 2ae0e83..ba5e806 100644
--- a/drivers/rtc/rtc-sysfs.c
+++ b/drivers/rtc/rtc-sysfs.c
@@ -149,16 +149,6 @@ rtc_sysfs_set_wakealarm(struct device *d
 
 	alarm = simple_strtoul(buf, NULL, 0);
 	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;
 	} else {
 		alm.enabled = 0;


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