linux-kernel - Re: [ 02/42] TTY: do not update atime/mtime on read/write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <u4obcuetop.fsf@md.dent.med.uni-muenchen.de>
Date:	Wed, 01 May 2013 15:05:58 +0200
From:	Wolfram Gloger <wmglo@...t.med.uni-muenchen.de>
To:	linux-kernel@...r.kernel.org
CC:	wmglo@...t.med.uni-muenchen.de
Subject: Re: [ 02/42] TTY: do not update atime/mtime on read/write

Hi,

> --- snip snip ---
>  drivers/tty/pty.c    | 3 +++
>  drivers/tty/tty_io.c | 4 ++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index a62798fcc014..59bfaecc4e14 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -681,6 +681,9 @@ static int ptmx_open(struct inode *inode, struct file *filp)
>
>         nonseekable_open(inode, filp);
>
> +       /* We refuse fsnotify events on ptmx, since it's a shared resource */
> +       filp->f_mode |= FMODE_NONOTIFY;
> +
>         retval = tty_alloc_file(filp);
>         if (retval)
>                 return retval;

This is definitely good.  But of course you can still poll on mtime.

> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 97ebc8c5864e..6464029e4860 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -988,10 +988,10 @@ void start_tty(struct tty_struct *tty)
>
>  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();
> -       sec -= sec % 60;
> +       unsigned long sec = get_seconds() & ~7;
>         if ((long)(sec - time->tv_sec) > 0)
>                 time->tv_sec = sec;
>  }

I still find this mildly ugly.  I would prefer this:

--- linux-3.8.10/drivers/tty/tty_io.c~	2013-02-19 00:58:34.000000000 +0100
+++ linux-3.8.10/drivers/tty/tty_io.c	2013-05-01 13:46:16.000000000 +0200
@@ -1080,8 +1080,11 @@
 		cond_resched();
 	}
 	if (written) {
+		if (tty->driver->type != TTY_DRIVER_TYPE_PTY ||
+		    tty->driver->subtype != PTY_TYPE_MASTER) {
 		struct inode *inode = file->f_path.dentry->d_inode;
 		inode->i_mtime = current_fs_time(inode->i_sb);
+		}
 		ret = written;
 	}
 out:

(without the tty_update_time change).  This prevents polling on
/dev/ptmx, but not on /dev/pts/*.  The latter seems unnecessary to me,
because during password entry, echo mode is off so no bytes are read,
and canonical mode is ON so no bytes are written until NL is entered.
So you can only obtain the total time taken to enter the password, not
individual keystrokes.

The canonical mode is also the reason why my suggestion to fix this in
userspace (in the other subthread) is quite problematic (I tried to
change PAM) as it looks impossible to delay or obfuscate the write
events on /dev/ptmx.

Regards,
Wolfram.
--
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/