Re: [PATCH] TTY: fix atime/mtime regression

Re: [PATCH] TTY: fix atime/mtime regression

[Linus Torvalds]

On Thu, Apr 25, 2013 at 5:40 AM, Jiri Slaby <jslaby@suse.cz> wrote:
>
> To revert to the old behaviour while still preventing attackers to
> guess the password length, we update the timestamps in ten-second
> intervals by this patch.

Hmm. Why ten seconds? Wouldn't it make more sense to use some natural
boundary, like a full minute?

Also, if I read the code correctly, this can actually make time go
*backwards* for the inode when the tty is first created. So it would
seem to be much better for tty_get_inode_time() to be passed in the
pointer to the tty time to be updated, so that it can avoid doing a
backwards jump.

Finally, if you're just interested in the seconds (like you are),
don't use "current_kernel_time()" that unnecessarily does the whole
nsec calculation. Just do "get_seconds()".

IOW, the end result would be something like

  void tty_update_time(struct timespec *time)
  {
    unsigned long sec = get_seconds();
    sec -= sec % 60;
    if ((long)(sec - time->tv_sec) > 0)
      time->tv_sec = sec;
  }

(That whole "(long)(sec - time->tv_sec)" is to handle wrapping time
correctly, we don't want to stop updating the inode in 2038 on 32-bit
machines).

Hmm?

             Linus

Re: [PATCH v2] TTY: fix atime/mtime regression

[Linus Torvalds]

On Fri, Apr 26, 2013 at 4:48 AM, Jiri Slaby <jslaby@suse.cz> wrote:
>
> To revert to the old behaviour while still preventing attackers to
> guess the password length, we update the timestamps in one-minute
> intervals by this patch.

Thanks, applied.

And now that I see the behavior of "w", I can kind of understand why
you picked 10s intervals. That "w" output is really really quite ugly.
Talking about "27.00s" idle for the current terminal when we only
update at even minutes ends up not being sensible.

Giving the resulting (now completely bogus) second-level accuracy looks odd.

But at this point it's just a visual oddity, and not really a bug. The
fact that "w" tries to display things with way more precision than the
kernel actually really gives it just results in slightly odd-looking
output. The "old-style" output of w ("-o") seems a much better
interface. Maybe some day procps can go back to doing that as the
default.

I'm adding Craig Small to the cc, since he seems to be the active
procps-ng developer.

Craig, background: the current git kernel (so 3.9, and these commits
will presumably be back-ported) does not update tty timestamps very
often, because you can use the timestamps to look at peoples typing
behavior. Initially it didn't update the timestamps AT ALL, but that
broke the whole idle routine. Now it updates it only at minute
boundaries, so things like "w" _work_, but the hundreth-of-a-second
idle precision is obviously just totally random noise.

Not a biggie, I doubt I would even have noticed unless I was
explicitly looking at that field, but....

                 Linus

Re: [PATCH v2] TTY: fix atime/mtime regression

[Simon Kirby]

On Fri, Apr 26, 2013 at 10:02:12AM -0700, Linus Torvalds wrote:

> On Fri, Apr 26, 2013 at 4:48 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> >
> > To revert to the old behaviour while still preventing attackers to
> > guess the password length, we update the timestamps in one-minute
> > intervals by this patch.
> 
> Thanks, applied.
> 
> And now that I see the behavior of "w", I can kind of understand why
> you picked 10s intervals. That "w" output is really really quite ugly.
> Talking about "27.00s" idle for the current terminal when we only
> update at even minutes ends up not being sensible.

Ah, so it was your suggestion to go with one minute.

I objected to the stable-backporting of this, since it was broken and
didn't actually fix the inotify path, but I care more about the time
granularity chosen here.

> Craig, background: the current git kernel (so 3.9, and these commits
> will presumably be back-ported) does not update tty timestamps very
> often, because you can use the timestamps to look at peoples typing
> behavior. Initially it didn't update the timestamps AT ALL, but that
> broke the whole idle routine. Now it updates it only at minute
> boundaries, so things like "w" _work_, but the hundreth-of-a-second
> idle precision is obviously just totally random noise.
> 
> Not a biggie, I doubt I would even have noticed unless I was
> explicitly looking at that field, but....

I look at this field all the time, and would really like to see seconds.
Surely anybody typing a password types it faster than 1 character per
second. Why stretch it out so much? Can we at least make it 10 seconds?

Simon-

---

Subject: [PATCH] TTY: increase atime/mtime update rate

37b7f3c76595 introduces an update interval for TTY atime updates, making
"w"'s IDLE column less useful than in the past. Since this is often used
for checking to see if other users are actually using the system, reduce
the time to 10 seconds.

Signed-off-by: Simon Kirby <sim@hostway.ca>
Cc: <stable@vger.kernel.org> # follow 37b7f3c76595e23257f61bd80b223de865
---
 drivers/tty/tty_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b045268..dee88ff 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -944,7 +944,7 @@ EXPORT_SYMBOL(start_tty);
 static void tty_update_time(struct timespec *time)
 {
 	unsigned long sec = get_seconds();
-	sec -= sec % 60;
+	sec -= sec % 10;
 	if ((long)(sec - time->tv_sec) > 0)
 		time->tv_sec = sec;
 }
-- 
1.7.10.4

Re: [PATCH v2] TTY: fix atime/mtime regression

[Craig Small]

On Fri, Apr 26, 2013 at 10:02:12AM -0700, Linus Torvalds wrote:
> Craig, background: the current git kernel (so 3.9, and these commits
> will presumably be back-ported) does not update tty timestamps very
> often, because you can use the timestamps to look at peoples typing
> behavior. Initially it didn't update the timestamps AT ALL, but that
> broke the whole idle routine. Now it updates it only at minute
> boundaries, so things like "w" _work_, but the hundreth-of-a-second
> idle precision is obviously just totally random noise.
I've always personally got confused with that field!  Joey Hess kept
bugging me for the -o option which was derived from w.bassman though
its been so long ago I forget where that came from.

I saw someone wants the 10 second resolution, can I get it confirmed
that the kernel will only be updating at minute intervals (so the .1 
second stuff is completely useless) or 10 second (so the .1 second
stuff is just confusing for most of us).

I'll then make some sort of change to w, possibly flipping the logic of
the -o flag.

 - Craig
-- 
Craig Small VK2XLZ   http://enc.com.au/          csmall at : enc.com.au
Debian GNU/Linux     http://www.debian.org/      csmall at : debian.org
GPG fingerprint:     5D2F B320 B825 D939 04D2  0519 3938 F96B DF50 FEA5

Re: [PATCH v2] TTY: fix atime/mtime regression

[Linus Torvalds]

On Sat, May 4, 2013 at 5:30 PM, Craig Small <csmall@enc.com.au> wrote:
>
> I saw someone wants the 10 second resolution, can I get it confirmed
> that the kernel will only be updating at minute intervals (so the .1
> second stuff is completely useless) or 10 second (so the .1 second
> stuff is just confusing for most of us).

Yeah, it's roughly 10s right now (it's actually 8 seconds, because the
power-of-two rounding is cheaper, whatever), and I think that's where
it will remain. It seems sufficient for people to know if somebody is
"active right now", while being seldom enough that you can't actually
get any useful information (ie you can get the "hey, somebody *just*
pressed a key by continually stat'ing the file, and get the timing of
that one key to almost arbitrary precision, but you won't know the
timing of any other keys for another eight seconds, so there's data
there but no useful *information*).

                Linus