[PATCH 1/1] tty: fix up atime/mtime mess, take four

[PATCH 1/1] tty: fix up atime/mtime mess, take four

[Jiri Slaby]

This problem was taken care of three times already in
* b0de59b5733d18b0d1974a060860a8b5c1b36a2e (TTY: do not update
  atime/mtime on read/write),
* 37b7f3c76595e23257f61bd80b223de8658617ee (TTY: fix atime/mtime
  regression), and
* b0b885657b6c8ef63a46bc9299b2a7715d19acde (tty: fix up atime/mtime
  mess, take three)

But it still misses one point. As John Paul correctly points out, we
do not care about setting date. If somebody ever changes wall
time backwards (by mistake for example), tty timestamps are never
updated until the original wall time passes.

So check the absolute difference of times and if it large than "8
seconds or so", always update the time. That means we will update
immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
check, but it was always that way.

Thanks John for serving me this so nicely debugged.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: John Paul Perry <john_paul.perry@alcatel-lucent.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <stable@vger.kernel.org> # all, as b0b885657 was backported
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 drivers/tty/tty_io.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e07f35e14fa2..e31b18a6d576 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1032,8 +1032,8 @@ EXPORT_SYMBOL(start_tty);
 /* We limit tty time update visibility to every 8 seconds or so. */
 static void tty_update_time(struct timespec *time)
 {
-	unsigned long sec = get_seconds() & ~7;
-	if ((long)(sec - time->tv_sec) > 0)
+	unsigned long sec = get_seconds();
+	if (abs(sec - time->tv_sec) & ~7)
 		time->tv_sec = sec;
 }
 
-- 
2.3.0

Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[Linus Torvalds]

Looks sane,

On Fri, Feb 27, 2015 at 9:40 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> Cc: Linus Torvalds <torvalds@linux-foundation.org>

Acked-by: Linus Torvalds <torvalds@linux-foundation.org>

                       Linus

Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[Greg KH]

On Fri, Feb 27, 2015 at 06:40:31PM +0100, Jiri Slaby wrote:
> This problem was taken care of three times already in
> * b0de59b5733d18b0d1974a060860a8b5c1b36a2e (TTY: do not update
>   atime/mtime on read/write),
> * 37b7f3c76595e23257f61bd80b223de8658617ee (TTY: fix atime/mtime
>   regression), and
> * b0b885657b6c8ef63a46bc9299b2a7715d19acde (tty: fix up atime/mtime
>   mess, take three)
> 
> But it still misses one point. As John Paul correctly points out, we
> do not care about setting date. If somebody ever changes wall
> time backwards (by mistake for example), tty timestamps are never
> updated until the original wall time passes.
> 
> So check the absolute difference of times and if it large than "8
> seconds or so", always update the time. That means we will update
> immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
> check, but it was always that way.
> 
> Thanks John for serving me this so nicely debugged.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-by: John Paul Perry <john_paul.perry@alcatel-lucent.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # all, as b0b885657 was backported
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>  drivers/tty/tty_io.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks, I'll queue this up and get it to Linus soon.

greg k-h

RE: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[Perry, John Paul G (John Paul)** CTR **]

Thanks everyone!

John Paul

-----Original Message-----
From: Greg KH [mailto:gregkh@linuxfoundation.org] 
Sent: Friday, February 27, 2015 11:23 AM
To: Jiri Slaby
Cc: linux-kernel@vger.kernel.org; Perry, John Paul G (John Paul)** CTR **; stable@vger.kernel.org; Linus Torvalds
Subject: Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

On Fri, Feb 27, 2015 at 06:40:31PM +0100, Jiri Slaby wrote:
> This problem was taken care of three times already in
> * b0de59b5733d18b0d1974a060860a8b5c1b36a2e (TTY: do not update
>   atime/mtime on read/write),
> * 37b7f3c76595e23257f61bd80b223de8658617ee (TTY: fix atime/mtime
>   regression), and
> * b0b885657b6c8ef63a46bc9299b2a7715d19acde (tty: fix up atime/mtime
>   mess, take three)
> 
> But it still misses one point. As John Paul correctly points out, we 
> do not care about setting date. If somebody ever changes wall time 
> backwards (by mistake for example), tty timestamps are never updated 
> until the original wall time passes.
> 
> So check the absolute difference of times and if it large than "8 
> seconds or so", always update the time. That means we will update 
> immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the 
> check, but it was always that way.
> 
> Thanks John for serving me this so nicely debugged.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-by: John Paul Perry <john_paul.perry@alcatel-lucent.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # all, as b0b885657 was backported
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>  drivers/tty/tty_io.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks, I'll queue this up and get it to Linus soon.

greg k-h

Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[Raymond Jennings]

On Fri, 2015-02-27 at 18:40 +0100, Jiri Slaby wrote:
> So check the absolute difference of times and if it large than "8
> seconds or so", always update the time. That means we will update
> immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
> check, but it was always that way.

If I may ask, what is supposed to happen normally when you write to a
tty device?  I always thought the tty device was treated just like a
normal file wrt. timestamps.

Now I see a patch for 8 seconds something.
> 
> Thanks John for serving me this so nicely debugged.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-by: John Paul Perry <john_paul.perry@alcatel-lucent.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # all, as b0b885657 was backported
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>  drivers/tty/tty_io.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index e07f35e14fa2..e31b18a6d576 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1032,8 +1032,8 @@ EXPORT_SYMBOL(start_tty);
>  /* We limit tty time update visibility to every 8 seconds or so. */
>  static void tty_update_time(struct timespec *time)
>  {
> -	unsigned long sec = get_seconds() & ~7;
> -	if ((long)(sec - time->tv_sec) > 0)
> +	unsigned long sec = get_seconds();
> +	if (abs(sec - time->tv_sec) & ~7)
>  		time->tv_sec = sec;
>  }
>  

Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[Jiri Slaby]

On 03/06/2015, 02:16 PM, Raymond Jennings wrote:
> On Fri, 2015-02-27 at 18:40 +0100, Jiri Slaby wrote:
>> So check the absolute difference of times and if it large than "8
>> seconds or so", always update the time. That means we will update
>> immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
>> check, but it was always that way.
> 
> If I may ask, what is supposed to happen normally when you write to a
> tty device?  I always thought the tty device was treated just like a
> normal file wrt. timestamps.
> 
> Now I see a patch for 8 seconds something.

Yes, because you do not want to be given any clue when users are typing
passwords. You could intercept the length of the password from the
pauses between key strokes (tty timestamps).

thanks,
-- 
js
suse labs

Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[One Thousand Gnomes]

On Mon, 09 Mar 2015 11:01:12 +0100
Jiri Slaby <jslaby@suse.cz> wrote:

> On 03/06/2015, 02:16 PM, Raymond Jennings wrote:
> > On Fri, 2015-02-27 at 18:40 +0100, Jiri Slaby wrote:
> >> So check the absolute difference of times and if it large than "8
> >> seconds or so", always update the time. That means we will update
> >> immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
> >> check, but it was always that way.
> > 
> > If I may ask, what is supposed to happen normally when you write to a
> > tty device?  I always thought the tty device was treated just like a
> > normal file wrt. timestamps.
> > 
> > Now I see a patch for 8 seconds something.
> 
> Yes, because you do not want to be given any clue when users are typing
> passwords. You could intercept the length of the password from the
> pauses between key strokes (tty timestamps).

On any vaguely idle box I can do the same and in fact probably far
better by measuring latencies via rdtsc and continually forcing a dword
out of cache in a tight loop.

It's a pointless change, second granularities are not useful for most
kinds of attack of this nature.

Alan

Re: [PATCH 1/1] tty: fix up atime/mtime mess, take four

[Jiri Slaby]

On 03/10/2015, 11:41 PM, One Thousand Gnomes wrote:
> On Mon, 09 Mar 2015 11:01:12 +0100
> Jiri Slaby <jslaby@suse.cz> wrote:
> 
>> On 03/06/2015, 02:16 PM, Raymond Jennings wrote:
>>> On Fri, 2015-02-27 at 18:40 +0100, Jiri Slaby wrote:
>>>> So check the absolute difference of times and if it large than "8
>>>> seconds or so", always update the time. That means we will update
>>>> immediatelly when changing time. Ergo, CAP_SYS_TIME can foul the
>>>> check, but it was always that way.
>>>
>>> If I may ask, what is supposed to happen normally when you write to a
>>> tty device?  I always thought the tty device was treated just like a
>>> normal file wrt. timestamps.
>>>
>>> Now I see a patch for 8 seconds something.
>>
>> Yes, because you do not want to be given any clue when users are typing
>> passwords. You could intercept the length of the password from the
>> pauses between key strokes (tty timestamps).
> 
> On any vaguely idle box I can do the same and in fact probably far
> better by measuring latencies via rdtsc and continually forcing a dword
> out of cache in a tight loop.

I don't know, I have to study and try this first, before I can take any
action.

> It's a pointless change, second granularities are not useful for most
> kinds of attack of this nature.

Yes, that was actually the whole point of the exercise: move from
current_fs_time() (one nanosecond granularity (for devtmpfs)) to
get_seconds() & 7 (8 seconds).

thanks,
-- 
js
suse labs