[PATCH 00/15] Rework tty audit

[PATCH 00/15] Rework tty audit

[Peter Hurley]

Hi Greg,

This patch series overhauls tty audit support. The goal was to simplify
and speed up tty auditing, which was a significant performance hit even
when disabled.

The main features of this series are:
* Remove reference counting; the purpose of reference counting the per-
  process tty_audit_buf was to prevent premature deletion if the
  buffer was in-use when tty auditing was exited for the process.
  However, since the process is single-threaded at tty_audit_exit(),
  the buffer cannot be in-use by another thread. Patch 11/15.
* Remove functionally dead code, such as tty_put_user(). Patch 2/15.
* Atomically modify tty audit enable/disable flags to support lockless
  read. Patch 9/15.

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
    for patch 9/15 which removes an audit field from the signal_struct.

Cc: Oleg Nesterov <oleg@redhat.com>
    to confirm my understanding of the single-threadedness of
    if (group_dead) tty_audit_exit(), called from do_exit(). Patch 11/15

Requires: "tty: audit: Fix audit source"

Regards,

Peter Hurley (15):
  tty: audit: Early-out pty master reads earlier
  tty: audit: Never audit packet mode
  tty: audit: Remove icanon mode from call chain
  tty: audit: Defer audit buffer association
  tty: audit: Take siglock directly
  tty: audit: Ignore current association for audit push
  tty: audit: Combine push functions
  tty: audit: Track tty association with dev_t
  tty: audit: Handle tty audit enable atomically
  tty: audit: Remove false memory optimization
  tty: audit: Remove tty_audit_buf reference counting
  tty: audit: Simplify first-use allocation
  tty: audit: Check audit enable first
  tty: audit: Always push audit buffer before TIOCSTI
  tty: audit: Poison tty_audit_buf while process exits

 drivers/tty/n_tty.c     |  25 ++----
 drivers/tty/tty_audit.c | 231 ++++++++++++++----------------------------------
 include/linux/audit.h   |   4 +
 include/linux/sched.h   |   1 -
 include/linux/tty.h     |  12 +--
 kernel/audit.c          |  27 +++---
 6 files changed, 97 insertions(+), 203 deletions(-)

-- 
2.6.3

[PATCH 01/15] tty: audit: Early-out pty master reads earlier

[Peter Hurley]

Reads from pty masters are not logged; early-out before taking
locks.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 3d245cd..ead924e 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -276,16 +276,16 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
 	if (unlikely(size == 0))
 		return;
 
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY
+	    && tty->driver->subtype == PTY_TYPE_MASTER)
+		return;
+
 	spin_lock_irqsave(&current->sighand->siglock, flags);
 	audit_log_tty_passwd = current->signal->audit_tty_log_passwd;
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
 		return;
 
-	if (tty->driver->type == TTY_DRIVER_TYPE_PTY
-	    && tty->driver->subtype == PTY_TYPE_MASTER)
-		return;
-
 	buf = tty_audit_buf_get(tty, icanon);
 	if (!buf)
 		return;
-- 
2.6.3

[PATCH 02/15] tty: audit: Never audit packet mode

[Peter Hurley]

tty audit never logs pty master reads, but packet mode only works for
pty masters, so tty_audit_add_data() was never logging packet mode
anyway.

Don't audit packet mode data. As those are the lone call sites, remove
tty_put_user().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 6342b37..b09c0c1 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -154,15 +154,6 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 	return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
 }
 
-static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
-			       unsigned char __user *ptr)
-{
-	struct n_tty_data *ldata = tty->disc_data;
-
-	tty_audit_add_data(tty, &x, 1, ldata->icanon);
-	return put_user(x, ptr);
-}
-
 static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
 			    size_t tail, size_t n)
 {
@@ -2229,11 +2220,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			cs = tty->link->ctrl_status;
 			tty->link->ctrl_status = 0;
 			spin_unlock_irq(&tty->link->ctrl_lock);
-			if (tty_put_user(tty, cs, b++)) {
+			if (put_user(cs, b)) {
 				retval = -EFAULT;
-				b--;
 				break;
 			}
+			b++;
 			nr--;
 			break;
 		}
@@ -2282,11 +2273,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 
 			/* Deal with packet mode. */
 			if (packet && b == buf) {
-				if (tty_put_user(tty, TIOCPKT_DATA, b++)) {
+				if (put_user(TIOCPKT_DATA, b)) {
 					retval = -EFAULT;
-					b--;
 					break;
 				}
+				b++;
 				nr--;
 			}
 
-- 
2.6.3

[PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Peter Hurley]

The tty termios bits cannot change while n_tty_read() is in the
i/o loop; the termios_rwsem ensures mutual exclusion with termios
changes in n_tty_set_termios(). Check L_ICANON() directly and
eliminate icanon parameter.

NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
have no other callers.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c     |  6 +++---
 drivers/tty/tty_audit.c | 22 +++++++++-------------
 include/linux/tty.h     |  4 ++--
 3 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index b09c0c1..a3ad312 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -163,7 +163,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
 	int uncopied;
 
 	if (n > size) {
-		tty_audit_add_data(tty, from, size, ldata->icanon);
+		tty_audit_add_data(tty, from, size);
 		uncopied = copy_to_user(to, from, size);
 		if (uncopied)
 			return uncopied;
@@ -172,7 +172,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
 		from = ldata->read_buf;
 	}
 
-	tty_audit_add_data(tty, from, n, ldata->icanon);
+	tty_audit_add_data(tty, from, n);
 	return copy_to_user(to, from, n);
 }
 
@@ -2008,7 +2008,7 @@ static int copy_from_read_buf(struct tty_struct *tty,
 		retval = copy_to_user(*b, from, n);
 		n -= retval;
 		is_eof = n == 1 && *from == EOF_CHAR(tty);
-		tty_audit_add_data(tty, from, n, ldata->icanon);
+		tty_audit_add_data(tty, from, n);
 		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
 		/* Turn single EOF into zero-length read */
 		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index ead924e..d2a004a 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -22,8 +22,7 @@ struct tty_audit_buf {
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
 };
 
-static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
-						 unsigned icanon)
+static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
 {
 	struct tty_audit_buf *buf;
 
@@ -35,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
 		goto err_buf;
 	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
-	buf->major = major;
-	buf->minor = minor;
-	buf->icanon = icanon;
+	buf->major = tty->driver->major;
+	buf->minor = tty->driver->minor_start + tty->index;
+	buf->icanon = !!L_ICANON(tty);
 	buf->valid = 0;
 	return buf;
 
@@ -216,8 +215,7 @@ int tty_audit_push_current(void)
  *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
  *	reference to the buffer.
  */
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
-		unsigned icanon)
+static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
 {
 	struct tty_audit_buf *buf, *buf2;
 	unsigned long flags;
@@ -234,9 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
 	}
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	buf2 = tty_audit_buf_alloc(tty->driver->major,
-				   tty->driver->minor_start + tty->index,
-				   icanon);
+	buf2 = tty_audit_buf_alloc(tty);
 	if (buf2 == NULL) {
 		audit_log_lost("out of memory in TTY auditing");
 		return NULL;
@@ -265,13 +261,13 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
  *
  *	Audit @data of @size from @tty, if necessary.
  */
-void tty_audit_add_data(struct tty_struct *tty, const void *data,
-			size_t size, unsigned icanon)
+void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 {
 	struct tty_audit_buf *buf;
 	int major, minor;
 	int audit_log_tty_passwd;
 	unsigned long flags;
+	unsigned int icanon = !!L_ICANON(tty);
 
 	if (unlikely(size == 0))
 		return;
@@ -286,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
 	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
 		return;
 
-	buf = tty_audit_buf_get(tty, icanon);
+	buf = tty_audit_buf_get(tty);
 	if (!buf)
 		return;
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 70f3a9c1..f8a20a8 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -603,7 +603,7 @@ extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
 /* tty_audit.c */
 #ifdef CONFIG_AUDIT
 extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
-			       size_t size, unsigned icanon);
+			       size_t size);
 extern void tty_audit_exit(void);
 extern void tty_audit_fork(struct signal_struct *sig);
 extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
@@ -611,7 +611,7 @@ extern void tty_audit_push(struct tty_struct *tty);
 extern int tty_audit_push_current(void);
 #else
 static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
-				      size_t size, unsigned icanon)
+				      size_t size)
 {
 }
 static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
-- 
2.6.3

Re: [PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Richard Guy Briggs]

On 15/11/10, Peter Hurley wrote:
> The tty termios bits cannot change while n_tty_read() is in the
> i/o loop; the termios_rwsem ensures mutual exclusion with termios
> changes in n_tty_set_termios(). Check L_ICANON() directly and
> eliminate icanon parameter.
> 
> NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
> is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
> have no other callers.

Which tree is this based on?  I don't see where the first chunk applies.

> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/tty/n_tty.c     |  6 +++---
>  drivers/tty/tty_audit.c | 22 +++++++++-------------
>  include/linux/tty.h     |  4 ++--
>  3 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index b09c0c1..a3ad312 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -163,7 +163,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
>  	int uncopied;
>  
>  	if (n > size) {
> -		tty_audit_add_data(tty, from, size, ldata->icanon);
> +		tty_audit_add_data(tty, from, size);
>  		uncopied = copy_to_user(to, from, size);
>  		if (uncopied)
>  			return uncopied;
> @@ -172,7 +172,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
>  		from = ldata->read_buf;
>  	}
>  
> -	tty_audit_add_data(tty, from, n, ldata->icanon);
> +	tty_audit_add_data(tty, from, n);
>  	return copy_to_user(to, from, n);
>  }
>  
> @@ -2008,7 +2008,7 @@ static int copy_from_read_buf(struct tty_struct *tty,
>  		retval = copy_to_user(*b, from, n);
>  		n -= retval;
>  		is_eof = n == 1 && *from == EOF_CHAR(tty);
> -		tty_audit_add_data(tty, from, n, ldata->icanon);
> +		tty_audit_add_data(tty, from, n);
>  		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
>  		/* Turn single EOF into zero-length read */
>  		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
> diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
> index ead924e..d2a004a 100644
> --- a/drivers/tty/tty_audit.c
> +++ b/drivers/tty/tty_audit.c
> @@ -22,8 +22,7 @@ struct tty_audit_buf {
>  	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
>  };
>  
> -static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
> -						 unsigned icanon)
> +static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
>  {
>  	struct tty_audit_buf *buf;
>  
> @@ -35,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
>  		goto err_buf;
>  	atomic_set(&buf->count, 1);
>  	mutex_init(&buf->mutex);
> -	buf->major = major;
> -	buf->minor = minor;
> -	buf->icanon = icanon;
> +	buf->major = tty->driver->major;
> +	buf->minor = tty->driver->minor_start + tty->index;
> +	buf->icanon = !!L_ICANON(tty);
>  	buf->valid = 0;
>  	return buf;
>  
> @@ -216,8 +215,7 @@ int tty_audit_push_current(void)
>   *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
>   *	reference to the buffer.
>   */
> -static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
> -		unsigned icanon)
> +static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
>  {
>  	struct tty_audit_buf *buf, *buf2;
>  	unsigned long flags;
> @@ -234,9 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
>  	}
>  	spin_unlock_irqrestore(&current->sighand->siglock, flags);
>  
> -	buf2 = tty_audit_buf_alloc(tty->driver->major,
> -				   tty->driver->minor_start + tty->index,
> -				   icanon);
> +	buf2 = tty_audit_buf_alloc(tty);
>  	if (buf2 == NULL) {
>  		audit_log_lost("out of memory in TTY auditing");
>  		return NULL;
> @@ -265,13 +261,13 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
>   *
>   *	Audit @data of @size from @tty, if necessary.
>   */
> -void tty_audit_add_data(struct tty_struct *tty, const void *data,
> -			size_t size, unsigned icanon)
> +void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
>  {
>  	struct tty_audit_buf *buf;
>  	int major, minor;
>  	int audit_log_tty_passwd;
>  	unsigned long flags;
> +	unsigned int icanon = !!L_ICANON(tty);
>  
>  	if (unlikely(size == 0))
>  		return;
> @@ -286,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
>  	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
>  		return;
>  
> -	buf = tty_audit_buf_get(tty, icanon);
> +	buf = tty_audit_buf_get(tty);
>  	if (!buf)
>  		return;
>  
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 70f3a9c1..f8a20a8 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -603,7 +603,7 @@ extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
>  /* tty_audit.c */
>  #ifdef CONFIG_AUDIT
>  extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
> -			       size_t size, unsigned icanon);
> +			       size_t size);
>  extern void tty_audit_exit(void);
>  extern void tty_audit_fork(struct signal_struct *sig);
>  extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
> @@ -611,7 +611,7 @@ extern void tty_audit_push(struct tty_struct *tty);
>  extern int tty_audit_push_current(void);
>  #else
>  static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
> -				      size_t size, unsigned icanon)
> +				      size_t size)
>  {
>  }
>  static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
> -- 
> 2.6.3
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Peter Hurley]

On 11/12/2015 02:10 PM, Richard Guy Briggs wrote:
> On 15/11/10, Peter Hurley wrote:
>> The tty termios bits cannot change while n_tty_read() is in the
>> i/o loop; the termios_rwsem ensures mutual exclusion with termios
>> changes in n_tty_set_termios(). Check L_ICANON() directly and
>> eliminate icanon parameter.
>>
>> NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
>> is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
>> have no other callers.
> 
> Which tree is this based on?  I don't see where the first chunk applies.

4.3

but this series requires a -stable fix [1] posted earlier but not in any
tree yet, which I noted in the cover-letter (was easy to overlook).

Regards,
Peter Hurley

[1] https://lkml.org/lkml/2015/11/8/133

Re: [PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Richard Guy Briggs]

On 15/11/12, Peter Hurley wrote:
> On 11/12/2015 02:10 PM, Richard Guy Briggs wrote:
> > On 15/11/10, Peter Hurley wrote:
> >> The tty termios bits cannot change while n_tty_read() is in the
> >> i/o loop; the termios_rwsem ensures mutual exclusion with termios
> >> changes in n_tty_set_termios(). Check L_ICANON() directly and
> >> eliminate icanon parameter.
> >>
> >> NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
> >> is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
> >> have no other callers.
> > 
> > Which tree is this based on?  I don't see where the first chunk applies.
> 
> 4.3
> 
> but this series requires a -stable fix [1] posted earlier but not in any
> tree yet, which I noted in the cover-letter (was easy to overlook).

Ok, found it.  It appears to also depend on "n_tty: Uninline
tty_copy_to_user()" [2] which I didn't see mentioned and can't find
upstream.  Is that in an upstream tree?

> Regards,
> Peter Hurley
> 
> [1] https://lkml.org/lkml/2015/11/8/133

[2] https://lkml.org/lkml/2015/11/8/146

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Peter Hurley]

On 11/12/2015 09:15 PM, Richard Guy Briggs wrote:
> On 15/11/12, Peter Hurley wrote:
>> On 11/12/2015 02:10 PM, Richard Guy Briggs wrote:
>>> On 15/11/10, Peter Hurley wrote:
>>>> The tty termios bits cannot change while n_tty_read() is in the
>>>> i/o loop; the termios_rwsem ensures mutual exclusion with termios
>>>> changes in n_tty_set_termios(). Check L_ICANON() directly and
>>>> eliminate icanon parameter.
>>>>
>>>> NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
>>>> is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
>>>> have no other callers.
>>>
>>> Which tree is this based on?  I don't see where the first chunk applies.
>>
>> 4.3
>>
>> but this series requires a -stable fix [1] posted earlier but not in any
>> tree yet, which I noted in the cover-letter (was easy to overlook).
> 
> Ok, found it.  It appears to also depend on "n_tty: Uninline
> tty_copy_to_user()" [2] which I didn't see mentioned and can't find
> upstream.

Sorry, my bad; I overlooked that dependency.

>  Is that in an upstream tree?

Not yet. That will end up in Greg's tty-next tree sometime shortly after
the merge window closes.

>> [1] https://lkml.org/lkml/2015/11/8/133
> 
> [2] https://lkml.org/lkml/2015/11/8/146

Re: [PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Richard Guy Briggs]

On 15/11/12, Peter Hurley wrote:
> On 11/12/2015 09:15 PM, Richard Guy Briggs wrote:
> > On 15/11/12, Peter Hurley wrote:
> >> On 11/12/2015 02:10 PM, Richard Guy Briggs wrote:
> >>> On 15/11/10, Peter Hurley wrote:
> >>>> The tty termios bits cannot change while n_tty_read() is in the
> >>>> i/o loop; the termios_rwsem ensures mutual exclusion with termios
> >>>> changes in n_tty_set_termios(). Check L_ICANON() directly and
> >>>> eliminate icanon parameter.
> >>>>
> >>>> NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
> >>>> is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
> >>>> have no other callers.
> >>>
> >>> Which tree is this based on?  I don't see where the first chunk applies.
> >>
> >> 4.3
> >>
> >> but this series requires a -stable fix [1] posted earlier but not in any
> >> tree yet, which I noted in the cover-letter (was easy to overlook).
> > 
> > Ok, found it.  It appears to also depend on "n_tty: Uninline
> > tty_copy_to_user()" [2] which I didn't see mentioned and can't find
> > upstream.
> 
> Sorry, my bad; I overlooked that dependency.

Looks like it also depends on "n_tty: Clarify copy_from_read_buf()" [3].

Any more we should know about?  Or should we just wait for GKH's tree?

> >  Is that in an upstream tree?
> 
> Not yet. That will end up in Greg's tty-next tree sometime shortly after
> the merge window closes.
> 
> >> [1] https://lkml.org/lkml/2015/11/8/133
> > 
> > [2] https://lkml.org/lkml/2015/11/8/146

[3] https://lkml.org/lkml/2015/11/8/145

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Peter Hurley]

On 11/12/2015 10:28 PM, Richard Guy Briggs wrote:
> On 15/11/12, Peter Hurley wrote:
>> On 11/12/2015 09:15 PM, Richard Guy Briggs wrote:
>>> On 15/11/12, Peter Hurley wrote:
>>>> On 11/12/2015 02:10 PM, Richard Guy Briggs wrote:
>>>>> On 15/11/10, Peter Hurley wrote:
>>>>>> The tty termios bits cannot change while n_tty_read() is in the
>>>>>> i/o loop; the termios_rwsem ensures mutual exclusion with termios
>>>>>> changes in n_tty_set_termios(). Check L_ICANON() directly and
>>>>>> eliminate icanon parameter.
>>>>>>
>>>>>> NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
>>>>>> is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
>>>>>> have no other callers.
>>>>>
>>>>> Which tree is this based on?  I don't see where the first chunk applies.
>>>>
>>>> 4.3
>>>>
>>>> but this series requires a -stable fix [1] posted earlier but not in any
>>>> tree yet, which I noted in the cover-letter (was easy to overlook).
>>>
>>> Ok, found it.  It appears to also depend on "n_tty: Uninline
>>> tty_copy_to_user()" [2] which I didn't see mentioned and can't find
>>> upstream.
>>
>> Sorry, my bad; I overlooked that dependency.
> 
> Looks like it also depends on "n_tty: Clarify copy_from_read_buf()" [3].

<sigh>

> Any more we should know about?  Or should we just wait for GKH's tree?

Yes, let's just wait until the dependencies are in Greg's tree.
I'll resend the series as v2 then.

Regards,
Peter Hurley



>>>  Is that in an upstream tree?
>>
>> Not yet. That will end up in Greg's tty-next tree sometime shortly after
>> the merge window closes.
>>
>>>> [1] https://lkml.org/lkml/2015/11/8/133
>>>
>>> [2] https://lkml.org/lkml/2015/11/8/146
> 
> [3] https://lkml.org/lkml/2015/11/8/145
> 
> - RGB

[PATCH 04/15] tty: audit: Defer audit buffer association

[Peter Hurley]

The tty audit buffer used to audit/record tty input is allocated on
the process's first call to tty_audit_add_data(), and not freed until
the process exits. On each call to tty_audit_add_data(), the current
tty is compared (by major:minor) with the last tty associated with
the audit buffer, and if the tty has changed the existing data is
logged to the audit log. The audit buffer is then re-associated with
the new tty.

Currently, the audit buffer is immediately associated with the tty;
however, the association must be re-checked when the buffer is locked
prior to copying the tty input. This extra step is always necessary,
since a concurrent read of a different tty by another thread of the
process may have used the buffer in between allocation and buffer
lock.

Rather than associate the audit buffer with the tty at allocation,
leave the buffer initially un-associated (null dev_t); simply let the
re-association check also perform the initial association.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index d2a004a..9effa81 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -22,7 +22,7 @@ struct tty_audit_buf {
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
 };
 
-static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_alloc(void)
 {
 	struct tty_audit_buf *buf;
 
@@ -34,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
 		goto err_buf;
 	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
-	buf->major = tty->driver->major;
-	buf->minor = tty->driver->minor_start + tty->index;
-	buf->icanon = !!L_ICANON(tty);
+	buf->major = 0;
+	buf->minor = 0;
+	buf->icanon = 0;
 	buf->valid = 0;
 	return buf;
 
@@ -211,11 +211,11 @@ int tty_audit_push_current(void)
 /**
  *	tty_audit_buf_get	-	Get an audit buffer.
  *
- *	Get an audit buffer for @tty, allocate it if necessary.  Return %NULL
+ *	Get an audit buffer, allocate it if necessary.  Return %NULL
  *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
  *	reference to the buffer.
  */
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_get(void)
 {
 	struct tty_audit_buf *buf, *buf2;
 	unsigned long flags;
@@ -232,7 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
 	}
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	buf2 = tty_audit_buf_alloc(tty);
+	buf2 = tty_audit_buf_alloc();
 	if (buf2 == NULL) {
 		audit_log_lost("out of memory in TTY auditing");
 		return NULL;
@@ -282,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
 		return;
 
-	buf = tty_audit_buf_get(tty);
+	buf = tty_audit_buf_get();
 	if (!buf)
 		return;
 
-- 
2.6.3

[PATCH 05/15] tty: audit: Take siglock directly

[Peter Hurley]

lock_task_sighand() is for situations where the struct task_struct*
may disappear while trying to deref the sighand; this never applies
to 'current'.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 9effa81..5f65653 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -180,22 +180,19 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 int tty_audit_push_current(void)
 {
 	struct tty_audit_buf *buf = ERR_PTR(-EPERM);
-	struct task_struct *tsk = current;
 	unsigned long flags;
 
-	if (!lock_task_sighand(tsk, &flags))
-		return -ESRCH;
-
-	if (tsk->signal->audit_tty) {
-		buf = tsk->signal->tty_audit_buf;
+	spin_lock_irqsave(&current->sighand->siglock, flags);
+	if (current->signal->audit_tty) {
+		buf = current->signal->tty_audit_buf;
 		if (buf)
 			atomic_inc(&buf->count);
 	}
-	unlock_task_sighand(tsk, &flags);
+	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
 	/*
 	 * Return 0 when signal->audit_tty set
-	 * but tsk->signal->tty_audit_buf == NULL.
+	 * but current->signal->tty_audit_buf == NULL.
 	 */
 	if (!buf || IS_ERR(buf))
 		return PTR_ERR(buf);
-- 
2.6.3

[PATCH 06/15] tty: audit: Ignore current association for audit push

[Peter Hurley]

In canonical read mode, each line read and logged is pushed separately
with tty_audit_push(). For all single-threaded processes and multi-threaded
processes reading from only one tty, this patch has no effect; the last line
read will still be the entry pushed to the audit log because the tty
association cannot have changed between tty_audit_add_data() and
tty_audit_push().

For multi-threaded processes reading from different ttys concurrently,
the audit log will have mixed log entries anyway. Consider two ttys
audited concurrently:

CPU0                           CPU1
----------                     ------------
tty_audit_add_data(ttyA)
                               tty_audit_add_data(ttyB)
tty_audit_push()
                               tty_audit_add_data(ttyB)
                               tty_audit_push()

This patch will now cause the ttyB output to be split into separate
audit log entries.

However, this possibility is equally likely without this patch:

CPU0                           CPU1
----------                     ------------
                               tty_audit_add_data(ttyB)
tty_audit_add_data(ttyA)
tty_audit_push()
                               tty_audit_add_data(ttyB)
                               tty_audit_push()

Mixed canonical and non-canonical reads have similar races.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c     |  2 +-
 drivers/tty/tty_audit.c | 11 +++--------
 include/linux/tty.h     |  2 +-
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index a3ad312..93f85a6 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2104,7 +2104,7 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
 			ldata->line_start = ldata->read_tail;
 		else
 			ldata->push = 0;
-		tty_audit_push(tty);
+		tty_audit_push();
 	}
 	return eof_push ? -EAGAIN : 0;
 }
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 5f65653..5ae4839 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -313,9 +313,9 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 /**
  *	tty_audit_push	-	Push buffered data out
  *
- *	Make sure no audit data is pending for @tty on the current process.
+ *	Make sure no audit data is pending on the current process.
  */
-void tty_audit_push(struct tty_struct *tty)
+void tty_audit_push(void)
 {
 	struct tty_audit_buf *buf;
 	unsigned long flags;
@@ -331,13 +331,8 @@ void tty_audit_push(struct tty_struct *tty)
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
 	if (buf) {
-		int major, minor;
-
-		major = tty->driver->major;
-		minor = tty->driver->minor_start + tty->index;
 		mutex_lock(&buf->mutex);
-		if (buf->major == major && buf->minor == minor)
-			tty_audit_buf_push(buf);
+		tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
 		tty_audit_buf_put(buf);
 	}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index f8a20a8..8a73d84 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -607,7 +607,7 @@ extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
 extern void tty_audit_exit(void);
 extern void tty_audit_fork(struct signal_struct *sig);
 extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
-extern void tty_audit_push(struct tty_struct *tty);
+extern void tty_audit_push(void);
 extern int tty_audit_push_current(void);
 #else
 static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
-- 
2.6.3

[PATCH 07/15] tty: audit: Combine push functions

[Peter Hurley]

tty_audit_push() and tty_audit_push_current() perform identical
tasks; eliminate the tty_audit_push() implementation and the
tty_audit_push_current() name.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 35 +++--------------------------------
 include/linux/tty.h     |  8 ++------
 kernel/audit.c          |  2 +-
 3 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 5ae4839..6b82c3c 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -172,12 +172,11 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 }
 
 /**
- * tty_audit_push_current -	Flush current's pending audit data
+ *	tty_audit_push	-	Flush current's pending audit data
  *
- * Try to lock sighand and get a reference to the tty audit buffer if available.
- * Flush the buffer or return an appropriate error code.
+ *	Returns 0 if success, -EPERM if tty audit is disabled
  */
-int tty_audit_push_current(void)
+int tty_audit_push(void)
 {
 	struct tty_audit_buf *buf = ERR_PTR(-EPERM);
 	unsigned long flags;
@@ -309,31 +308,3 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	mutex_unlock(&buf->mutex);
 	tty_audit_buf_put(buf);
 }
-
-/**
- *	tty_audit_push	-	Push buffered data out
- *
- *	Make sure no audit data is pending on the current process.
- */
-void tty_audit_push(void)
-{
-	struct tty_audit_buf *buf;
-	unsigned long flags;
-
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (likely(!current->signal->audit_tty)) {
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
-		return;
-	}
-	buf = current->signal->tty_audit_buf;
-	if (buf)
-		atomic_inc(&buf->count);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-
-	if (buf) {
-		mutex_lock(&buf->mutex);
-		tty_audit_buf_push(buf);
-		mutex_unlock(&buf->mutex);
-		tty_audit_buf_put(buf);
-	}
-}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 8a73d84..4116b1d 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -607,8 +607,7 @@ extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
 extern void tty_audit_exit(void);
 extern void tty_audit_fork(struct signal_struct *sig);
 extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
-extern void tty_audit_push(void);
-extern int tty_audit_push_current(void);
+extern int tty_audit_push(void);
 #else
 static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
 				      size_t size)
@@ -623,10 +622,7 @@ static inline void tty_audit_exit(void)
 static inline void tty_audit_fork(struct signal_struct *sig)
 {
 }
-static inline void tty_audit_push(struct tty_struct *tty)
-{
-}
-static inline int tty_audit_push_current(void)
+static inline int tty_audit_push(void)
 {
 	return 0;
 }
diff --git a/kernel/audit.c b/kernel/audit.c
index 662c007..63f87aa 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -907,7 +907,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		if (err == 1) { /* match or error */
 			err = 0;
 			if (msg_type == AUDIT_USER_TTY) {
-				err = tty_audit_push_current();
+				err = tty_audit_push();
 				if (err)
 					break;
 			}
-- 
2.6.3

[PATCH 08/15] tty: audit: Track tty association with dev_t

[Peter Hurley]

Use dev_t instead of separate major/minor fields to track tty
audit buffer association.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 6b82c3c..50380d8 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -16,7 +16,7 @@
 struct tty_audit_buf {
 	atomic_t count;
 	struct mutex mutex;	/* Protects all data below */
-	int major, minor;	/* The TTY which the data is from */
+	dev_t dev;		/* The TTY which the data is from */
 	unsigned icanon:1;
 	size_t valid;
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
@@ -34,8 +34,7 @@ static struct tty_audit_buf *tty_audit_buf_alloc(void)
 		goto err_buf;
 	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
-	buf->major = 0;
-	buf->minor = 0;
+	buf->dev = MKDEV(0, 0);
 	buf->icanon = 0;
 	buf->valid = 0;
 	return buf;
@@ -59,7 +58,7 @@ static void tty_audit_buf_put(struct tty_audit_buf *buf)
 		tty_audit_buf_free(buf);
 }
 
-static void tty_audit_log(const char *description, int major, int minor,
+static void tty_audit_log(const char *description, dev_t dev,
 			  unsigned char *data, size_t size)
 {
 	struct audit_buffer *ab;
@@ -75,7 +74,7 @@ static void tty_audit_log(const char *description, int major, int minor,
 
 		audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d"
 				 " minor=%d comm=", description, pid, uid,
-				 loginuid, sessionid, major, minor);
+				 loginuid, sessionid, MAJOR(dev), MINOR(dev));
 		get_task_comm(name, tsk);
 		audit_log_untrustedstring(ab, name);
 		audit_log_format(ab, " data=");
@@ -98,7 +97,7 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf)
 		buf->valid = 0;
 		return;
 	}
-	tty_audit_log("tty", buf->major, buf->minor, buf->data, buf->valid);
+	tty_audit_log("tty", buf->dev, buf->data, buf->valid);
 	buf->valid = 0;
 }
 
@@ -141,7 +140,8 @@ void tty_audit_fork(struct signal_struct *sig)
 void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
 	struct tty_audit_buf *buf;
-	int major, minor, should_audit;
+	dev_t dev;
+	int should_audit;
 	unsigned long flags;
 
 	spin_lock_irqsave(&current->sighand->siglock, flags);
@@ -151,11 +151,10 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 		atomic_inc(&buf->count);
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	major = tty->driver->major;
-	minor = tty->driver->minor_start + tty->index;
+	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
 	if (buf) {
 		mutex_lock(&buf->mutex);
-		if (buf->major == major && buf->minor == minor)
+		if (buf->dev == dev)
 			tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
 		tty_audit_buf_put(buf);
@@ -167,7 +166,7 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 
 		auid = audit_get_loginuid(current);
 		sessionid = audit_get_sessionid(current);
-		tty_audit_log("ioctl=TIOCSTI", major, minor, &ch, 1);
+		tty_audit_log("ioctl=TIOCSTI", dev, &ch, 1);
 	}
 }
 
@@ -260,10 +259,10 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 {
 	struct tty_audit_buf *buf;
-	int major, minor;
 	int audit_log_tty_passwd;
 	unsigned long flags;
 	unsigned int icanon = !!L_ICANON(tty);
+	dev_t dev;
 
 	if (unlikely(size == 0))
 		return;
@@ -283,13 +282,10 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 		return;
 
 	mutex_lock(&buf->mutex);
-	major = tty->driver->major;
-	minor = tty->driver->minor_start + tty->index;
-	if (buf->major != major || buf->minor != minor
-	    || buf->icanon != icanon) {
+	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
+	if (buf->dev != dev || buf->icanon != icanon) {
 		tty_audit_buf_push(buf);
-		buf->major = major;
-		buf->minor = minor;
+		buf->dev = dev;
 		buf->icanon = icanon;
 	}
 	do {
-- 
2.6.3

[PATCH 09/15] tty: audit: Handle tty audit enable atomically

[Peter Hurley]

The audit_tty and audit_tty_log_passwd fields are actually bool
values, so merge into single memory location to access atomically.

NB: audit log operations may still occur after tty audit is disabled
which is consistent with the existing functionality

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 53 ++++++++++++++++++++-----------------------------
 include/linux/audit.h   |  4 ++++
 include/linux/sched.h   |  1 -
 kernel/audit.c          | 25 +++++++++++------------
 4 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 50380d8..3d90f88 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -131,7 +131,6 @@ void tty_audit_exit(void)
 void tty_audit_fork(struct signal_struct *sig)
 {
 	sig->audit_tty = current->signal->audit_tty;
-	sig->audit_tty_log_passwd = current->signal->audit_tty_log_passwd;
 }
 
 /**
@@ -141,11 +140,9 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
 	struct tty_audit_buf *buf;
 	dev_t dev;
-	int should_audit;
 	unsigned long flags;
 
 	spin_lock_irqsave(&current->sighand->siglock, flags);
-	should_audit = current->signal->audit_tty;
 	buf = current->signal->tty_audit_buf;
 	if (buf)
 		atomic_inc(&buf->count);
@@ -160,7 +157,7 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 		tty_audit_buf_put(buf);
 	}
 
-	if (should_audit && audit_enabled) {
+	if (audit_enabled && (current->signal->audit_tty & AUDIT_TTY_ENABLE)) {
 		kuid_t auid;
 		unsigned int sessionid;
 
@@ -177,29 +174,25 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
  */
 int tty_audit_push(void)
 {
-	struct tty_audit_buf *buf = ERR_PTR(-EPERM);
+	struct tty_audit_buf *buf;
 	unsigned long flags;
 
+	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
+		return -EPERM;
+
 	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (current->signal->audit_tty) {
-		buf = current->signal->tty_audit_buf;
-		if (buf)
-			atomic_inc(&buf->count);
-	}
+	buf = current->signal->tty_audit_buf;
+	if (buf)
+		atomic_inc(&buf->count);
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	/*
-	 * Return 0 when signal->audit_tty set
-	 * but current->signal->tty_audit_buf == NULL.
-	 */
-	if (!buf || IS_ERR(buf))
-		return PTR_ERR(buf);
-
-	mutex_lock(&buf->mutex);
-	tty_audit_buf_push(buf);
-	mutex_unlock(&buf->mutex);
+	if (buf) {
+		mutex_lock(&buf->mutex);
+		tty_audit_buf_push(buf);
+		mutex_unlock(&buf->mutex);
 
-	tty_audit_buf_put(buf);
+		tty_audit_buf_put(buf);
+	}
 	return 0;
 }
 
@@ -218,8 +211,6 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 	buf = NULL;
 	buf2 = NULL;
 	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (likely(!current->signal->audit_tty))
-		goto out;
 	buf = current->signal->tty_audit_buf;
 	if (buf) {
 		atomic_inc(&buf->count);
@@ -233,9 +224,10 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 		return NULL;
 	}
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (!current->signal->audit_tty)
+	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
 		goto out;
+
+	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
 	if (!buf) {
 		current->signal->tty_audit_buf = buf2;
@@ -259,9 +251,8 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 {
 	struct tty_audit_buf *buf;
-	int audit_log_tty_passwd;
-	unsigned long flags;
 	unsigned int icanon = !!L_ICANON(tty);
+	unsigned int audit_tty;
 	dev_t dev;
 
 	if (unlikely(size == 0))
@@ -271,10 +262,10 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	    && tty->driver->subtype == PTY_TYPE_MASTER)
 		return;
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	audit_log_tty_passwd = current->signal->audit_tty_log_passwd;
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
+	audit_tty = READ_ONCE(current->signal->audit_tty);
+	if (~audit_tty & AUDIT_TTY_ENABLE)
+		return;
+	if ((~audit_tty & AUDIT_TTY_LOG_PASSWD) && icanon && !L_ECHO(tty))
 		return;
 
 	buf = tty_audit_buf_get();
diff --git a/include/linux/audit.h b/include/linux/audit.h
index b2abc99..5ef8862 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -109,6 +109,10 @@ extern int audit_classify_compat_syscall(int abi, unsigned syscall);
 /* maximized args number that audit_socketcall can process */
 #define AUDITSC_ARGS		6
 
+/* bit values for ->signal->audit_tty */
+#define AUDIT_TTY_ENABLE	BIT(0)
+#define AUDIT_TTY_LOG_PASSWD	BIT(1)
+
 struct filename;
 
 extern void audit_log_session_info(struct audit_buffer *ab);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b7b9501..cd1c0a8 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -759,7 +759,6 @@ struct signal_struct {
 #endif
 #ifdef CONFIG_AUDIT
 	unsigned audit_tty;
-	unsigned audit_tty_log_passwd;
 	struct tty_audit_buf *tty_audit_buf;
 #endif
 #ifdef CONFIG_CGROUPS
diff --git a/kernel/audit.c b/kernel/audit.c
index 63f87aa..ce83589 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1017,20 +1017,19 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		break;
 	case AUDIT_TTY_GET: {
 		struct audit_tty_status s;
-		struct task_struct *tsk = current;
+		unsigned int t;
 
-		spin_lock(&tsk->sighand->siglock);
-		s.enabled = tsk->signal->audit_tty;
-		s.log_passwd = tsk->signal->audit_tty_log_passwd;
-		spin_unlock(&tsk->sighand->siglock);
+		t = READ_ONCE(current->signal->audit_tty);
+		s.enabled = t & AUDIT_TTY_ENABLE;
+		s.log_passwd = !!(t & AUDIT_TTY_LOG_PASSWD);
 
 		audit_send_reply(skb, seq, AUDIT_TTY_GET, 0, 0, &s, sizeof(s));
 		break;
 	}
 	case AUDIT_TTY_SET: {
 		struct audit_tty_status s, old;
-		struct task_struct *tsk = current;
 		struct audit_buffer	*ab;
+		unsigned int t;
 
 		memset(&s, 0, sizeof(s));
 		/* guard against past and future API changes */
@@ -1040,14 +1039,14 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		    (s.log_passwd != 0 && s.log_passwd != 1))
 			err = -EINVAL;
 
-		spin_lock(&tsk->sighand->siglock);
-		old.enabled = tsk->signal->audit_tty;
-		old.log_passwd = tsk->signal->audit_tty_log_passwd;
-		if (!err) {
-			tsk->signal->audit_tty = s.enabled;
-			tsk->signal->audit_tty_log_passwd = s.log_passwd;
+		if (err)
+			t = READ_ONCE(current->signal->audit_tty);
+		else {
+			t = s.enabled | (-s.log_passwd & AUDIT_TTY_LOG_PASSWD);
+			t = xchg(&current->signal->audit_tty, t);
 		}
-		spin_unlock(&tsk->sighand->siglock);
+		old.enabled = t & AUDIT_TTY_ENABLE;
+		old.log_passwd = !!(t & AUDIT_TTY_LOG_PASSWD);
 
 		audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
 		audit_log_format(ab, " op=tty_set old-enabled=%d new-enabled=%d"
-- 
2.6.3

[PATCH 10/15] tty: audit: Remove false memory optimization

[Peter Hurley]

The tty audit buffer is allocated at first use and not freed until
the process exits. If tty audit is turned off after the audit buffer
has been allocated, no effort is made to release the buffer.
So re-checking if tty audit has just been turned off when tty audit
was just on is false optimization; the likelihood of triggering this
condition is exceedingly small.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 3d90f88..7943984 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -200,8 +200,7 @@ int tty_audit_push(void)
  *	tty_audit_buf_get	-	Get an audit buffer.
  *
  *	Get an audit buffer, allocate it if necessary.  Return %NULL
- *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
- *	reference to the buffer.
+ *	if out of memory.  Otherwise, return a new reference to the buffer.
  */
 static struct tty_audit_buf *tty_audit_buf_get(void)
 {
@@ -224,9 +223,6 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 		return NULL;
 	}
 
-	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
-		goto out;
-
 	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
 	if (!buf) {
-- 
2.6.3

[PATCH 11/15] tty: audit: Remove tty_audit_buf reference counting

[Peter Hurley]

When tty_audit_exit() is called from do_exit(), the process is
single-threaded. Since the tty_audit_buf is only shared by threads
of a process, no other thread can be concurrently accessing the
tty_audit_buf during or after tty_audit_exit().

Thus, no other thread can be holding an extra tty_audit_buf reference
which would prevent tty_audit_exit() from freeing the tty_audit_buf.
As that is the only purpose of the ref counting, remove the reference
counting and free the tty_audit_buf directly.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 47 +++++++----------------------------------------
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 7943984..71ba8ba 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -14,7 +14,6 @@
 #include <linux/tty.h>
 
 struct tty_audit_buf {
-	atomic_t count;
 	struct mutex mutex;	/* Protects all data below */
 	dev_t dev;		/* The TTY which the data is from */
 	unsigned icanon:1;
@@ -32,7 +31,6 @@ static struct tty_audit_buf *tty_audit_buf_alloc(void)
 	buf->data = kmalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
 	if (!buf->data)
 		goto err_buf;
-	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
 	buf->dev = MKDEV(0, 0);
 	buf->icanon = 0;
@@ -52,12 +50,6 @@ static void tty_audit_buf_free(struct tty_audit_buf *buf)
 	kfree(buf);
 }
 
-static void tty_audit_buf_put(struct tty_audit_buf *buf)
-{
-	if (atomic_dec_and_test(&buf->count))
-		tty_audit_buf_free(buf);
-}
-
 static void tty_audit_log(const char *description, dev_t dev,
 			  unsigned char *data, size_t size)
 {
@@ -106,6 +98,9 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf)
  *
  *	Make sure all buffered data is written out and deallocate the buffer.
  *	Only needs to be called if current->signal->tty_audit_buf != %NULL.
+ *
+ *	The process is single-threaded at this point; no other threads share
+ *	current->signal.
  */
 void tty_audit_exit(void)
 {
@@ -116,11 +111,8 @@ void tty_audit_exit(void)
 	if (!buf)
 		return;
 
-	mutex_lock(&buf->mutex);
 	tty_audit_buf_push(buf);
-	mutex_unlock(&buf->mutex);
-
-	tty_audit_buf_put(buf);
+	tty_audit_buf_free(buf);
 }
 
 /**
@@ -140,21 +132,14 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
 	struct tty_audit_buf *buf;
 	dev_t dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	buf = current->signal->tty_audit_buf;
-	if (buf)
-		atomic_inc(&buf->count);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
 	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
+	buf = current->signal->tty_audit_buf;
 	if (buf) {
 		mutex_lock(&buf->mutex);
 		if (buf->dev == dev)
 			tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
-		tty_audit_buf_put(buf);
 	}
 
 	if (audit_enabled && (current->signal->audit_tty & AUDIT_TTY_ENABLE)) {
@@ -175,23 +160,15 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 int tty_audit_push(void)
 {
 	struct tty_audit_buf *buf;
-	unsigned long flags;
 
 	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
 		return -EPERM;
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
-	if (buf)
-		atomic_inc(&buf->count);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-
 	if (buf) {
 		mutex_lock(&buf->mutex);
 		tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
-
-		tty_audit_buf_put(buf);
 	}
 	return 0;
 }
@@ -207,15 +184,9 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 	struct tty_audit_buf *buf, *buf2;
 	unsigned long flags;
 
-	buf = NULL;
-	buf2 = NULL;
-	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
-	if (buf) {
-		atomic_inc(&buf->count);
-		goto out;
-	}
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+	if (buf)
+		return buf;
 
 	buf2 = tty_audit_buf_alloc();
 	if (buf2 == NULL) {
@@ -230,9 +201,6 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 		buf = buf2;
 		buf2 = NULL;
 	}
-	atomic_inc(&buf->count);
-	/* Fall through */
- out:
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 	if (buf2)
 		tty_audit_buf_free(buf2);
@@ -289,5 +257,4 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 			tty_audit_buf_push(buf);
 	} while (size != 0);
 	mutex_unlock(&buf->mutex);
-	tty_audit_buf_put(buf);
 }
-- 
2.6.3

[PATCH 12/15] tty: audit: Simplify first-use allocation

[Peter Hurley]

The first-use tty audit buffer allocation is a potential race
amongst multiple attempts at 'first-use'; only one 'winner' is
acceptable.

The successful buffer assignment occurs if tty_audit_buf == NULL
(which will also be the return from cmpxchg()); otherwise, another
racer 'won' and this buffer allocation is freed.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 71ba8ba..6e33e41 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -181,30 +181,22 @@ int tty_audit_push(void)
  */
 static struct tty_audit_buf *tty_audit_buf_get(void)
 {
-	struct tty_audit_buf *buf, *buf2;
-	unsigned long flags;
+	struct tty_audit_buf *buf;
 
 	buf = current->signal->tty_audit_buf;
 	if (buf)
 		return buf;
 
-	buf2 = tty_audit_buf_alloc();
-	if (buf2 == NULL) {
+	buf = tty_audit_buf_alloc();
+	if (buf == NULL) {
 		audit_log_lost("out of memory in TTY auditing");
 		return NULL;
 	}
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	buf = current->signal->tty_audit_buf;
-	if (!buf) {
-		current->signal->tty_audit_buf = buf2;
-		buf = buf2;
-		buf2 = NULL;
-	}
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-	if (buf2)
-		tty_audit_buf_free(buf2);
-	return buf;
+	/* Race to use this buffer, free it if another wins */
+	if (cmpxchg(&current->signal->tty_audit_buf, NULL, buf) != NULL)
+		tty_audit_buf_free(buf);
+	return current->signal->tty_audit_buf;
 }
 
 /**
-- 
2.6.3

[PATCH 13/15] tty: audit: Check audit enable first

[Peter Hurley]

Audit is unlikely to be enabled; check first to exit asap.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 6e33e41..269e41f 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -211,6 +211,10 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	unsigned int audit_tty;
 	dev_t dev;
 
+	audit_tty = READ_ONCE(current->signal->audit_tty);
+	if (~audit_tty & AUDIT_TTY_ENABLE)
+		return;
+
 	if (unlikely(size == 0))
 		return;
 
@@ -218,9 +222,6 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	    && tty->driver->subtype == PTY_TYPE_MASTER)
 		return;
 
-	audit_tty = READ_ONCE(current->signal->audit_tty);
-	if (~audit_tty & AUDIT_TTY_ENABLE)
-		return;
 	if ((~audit_tty & AUDIT_TTY_LOG_PASSWD) && icanon && !L_ECHO(tty))
 		return;
 
-- 
2.6.3

[PATCH 14/15] tty: audit: Always push audit buffer before TIOCSTI

[Peter Hurley]

The data read from another tty may be relevant to the action of
the TIOCSTI ioctl; log the audit buffer immediately.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 269e41f..fa461dc 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -130,19 +130,13 @@ void tty_audit_fork(struct signal_struct *sig)
  */
 void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
-	struct tty_audit_buf *buf;
 	dev_t dev;
 
 	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
-	buf = current->signal->tty_audit_buf;
-	if (buf) {
-		mutex_lock(&buf->mutex);
-		if (buf->dev == dev)
-			tty_audit_buf_push(buf);
-		mutex_unlock(&buf->mutex);
-	}
+	if (tty_audit_push())
+		return;
 
-	if (audit_enabled && (current->signal->audit_tty & AUDIT_TTY_ENABLE)) {
+	if (audit_enabled) {
 		kuid_t auid;
 		unsigned int sessionid;
 
-- 
2.6.3

[PATCH 15/15] tty: audit: Poison tty_audit_buf while process exits

[Peter Hurley]

Warn if tty_audit_buf use is attempted after tty_audit_exit() has
already freed it.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index fa461dc..66d53fc 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -21,6 +21,15 @@ struct tty_audit_buf {
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
 };
 
+static struct tty_audit_buf *tty_audit_buf_ref(void)
+{
+	struct tty_audit_buf *buf;
+
+	buf = current->signal->tty_audit_buf;
+	WARN_ON(buf == ERR_PTR(-ESRCH));
+	return buf;
+}
+
 static struct tty_audit_buf *tty_audit_buf_alloc(void)
 {
 	struct tty_audit_buf *buf;
@@ -106,8 +115,7 @@ void tty_audit_exit(void)
 {
 	struct tty_audit_buf *buf;
 
-	buf = current->signal->tty_audit_buf;
-	current->signal->tty_audit_buf = NULL;
+	buf = xchg(&current->signal->tty_audit_buf, ERR_PTR(-ESRCH));
 	if (!buf)
 		return;
 
@@ -158,8 +166,8 @@ int tty_audit_push(void)
 	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
 		return -EPERM;
 
-	buf = current->signal->tty_audit_buf;
-	if (buf) {
+	buf = tty_audit_buf_ref();
+	if (!IS_ERR_OR_NULL(buf)) {
 		mutex_lock(&buf->mutex);
 		tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
@@ -171,13 +179,14 @@ int tty_audit_push(void)
  *	tty_audit_buf_get	-	Get an audit buffer.
  *
  *	Get an audit buffer, allocate it if necessary.  Return %NULL
- *	if out of memory.  Otherwise, return a new reference to the buffer.
+ *	if out of memory or ERR_PTR(-ESRCH) if tty_audit_exit() has already
+ *	occurred.  Otherwise, return a new reference to the buffer.
  */
 static struct tty_audit_buf *tty_audit_buf_get(void)
 {
 	struct tty_audit_buf *buf;
 
-	buf = current->signal->tty_audit_buf;
+	buf = tty_audit_buf_ref();
 	if (buf)
 		return buf;
 
@@ -190,7 +199,7 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 	/* Race to use this buffer, free it if another wins */
 	if (cmpxchg(&current->signal->tty_audit_buf, NULL, buf) != NULL)
 		tty_audit_buf_free(buf);
-	return current->signal->tty_audit_buf;
+	return tty_audit_buf_ref();
 }
 
 /**
@@ -220,7 +229,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 		return;
 
 	buf = tty_audit_buf_get();
-	if (!buf)
+	if (IS_ERR_OR_NULL(buf))
 		return;
 
 	mutex_lock(&buf->mutex);
-- 
2.6.3

Re: [PATCH 00/15] Rework tty audit

[Peter Hurley]

On 11/10/2015 09:05 PM, Peter Hurley wrote:
> Hi Greg,
> 
> This patch series overhauls tty audit support. The goal was to simplify
> and speed up tty auditing, which was a significant performance hit even
> when disabled.
> 
> The main features of this series are:
> * Remove reference counting; the purpose of reference counting the per-
>   process tty_audit_buf was to prevent premature deletion if the
>   buffer was in-use when tty auditing was exited for the process.
>   However, since the process is single-threaded at tty_audit_exit(),
>   the buffer cannot be in-use by another thread. Patch 11/15.
> * Remove functionally dead code, such as tty_put_user(). Patch 2/15.
> * Atomically modify tty audit enable/disable flags to support lockless
>   read. Patch 9/15.
> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
>     for patch 9/15 which removes an audit field from the signal_struct.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
>     to confirm my understanding of the single-threadedness of
>     if (group_dead) tty_audit_exit(), called from do_exit(). Patch 11/15
> 
> Requires: "tty: audit: Fix audit source"

and as brought to my attention by Richard Guy Briggs also
Requires: "n_tty: Uninline tty_copy_to_user()"

Apologies for any inconvenience caused.


> Regards,
> 
> Peter Hurley (15):
>   tty: audit: Early-out pty master reads earlier
>   tty: audit: Never audit packet mode
>   tty: audit: Remove icanon mode from call chain
>   tty: audit: Defer audit buffer association
>   tty: audit: Take siglock directly
>   tty: audit: Ignore current association for audit push
>   tty: audit: Combine push functions
>   tty: audit: Track tty association with dev_t
>   tty: audit: Handle tty audit enable atomically
>   tty: audit: Remove false memory optimization
>   tty: audit: Remove tty_audit_buf reference counting
>   tty: audit: Simplify first-use allocation
>   tty: audit: Check audit enable first
>   tty: audit: Always push audit buffer before TIOCSTI
>   tty: audit: Poison tty_audit_buf while process exits
> 
>  drivers/tty/n_tty.c     |  25 ++----
>  drivers/tty/tty_audit.c | 231 ++++++++++++++----------------------------------
>  include/linux/audit.h   |   4 +
>  include/linux/sched.h   |   1 -
>  include/linux/tty.h     |  12 +--
>  kernel/audit.c          |  27 +++---
>  6 files changed, 97 insertions(+), 203 deletions(-)
> 

Re: [PATCH 00/15] Rework tty audit

[Paul Moore]

On Tuesday, November 10, 2015 09:05:45 PM Peter Hurley wrote:
> Hi Greg,
> 
> This patch series overhauls tty audit support. The goal was to simplify
> and speed up tty auditing, which was a significant performance hit even
> when disabled.
> 
> The main features of this series are:
> * Remove reference counting; the purpose of reference counting the per-
>   process tty_audit_buf was to prevent premature deletion if the
>   buffer was in-use when tty auditing was exited for the process.
>   However, since the process is single-threaded at tty_audit_exit(),
>   the buffer cannot be in-use by another thread. Patch 11/15.
> * Remove functionally dead code, such as tty_put_user(). Patch 2/15.
> * Atomically modify tty audit enable/disable flags to support lockless
>   read. Patch 9/15.
> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
>     for patch 9/15 which removes an audit field from the signal_struct.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
>     to confirm my understanding of the single-threadedness of
>     if (group_dead) tty_audit_exit(), called from do_exit(). Patch 11/15
> 
> Requires: "tty: audit: Fix audit source"

This is definitely more of a tty patchset than it is an audit patchset, but it 
all looks reasonable to me from an audit perspective.

-- 
paul moore
www.paul-moore.com

[RESEND][PATCH 00/15] Rework tty audit

[Peter Hurley]

Hi Greg,

Here's a resend of the original tty audit series from Nov 10;
no objections from the audit maintainers.

Original message follows:

This patch series overhauls tty audit support. The goal was to simplify
and speed up tty auditing, which was a significant performance hit even
when disabled.

The main features of this series are:
* Remove reference counting; the purpose of reference counting the per-
  process tty_audit_buf was to prevent premature deletion if the
  buffer was in-use when tty auditing was exited for the process.
  However, since the process is single-threaded at tty_audit_exit(),
  the buffer cannot be in-use by another thread. Patch 11/15.
* Remove functionally dead code, such as tty_put_user(). Patch 2/15.
* Atomically modify tty audit enable/disable flags to support lockless
  read. Patch 9/15.

Regards,

Peter Hurley (15):
  tty: audit: Early-out pty master reads earlier
  tty: audit: Never audit packet mode
  tty: audit: Remove icanon mode from call chain
  tty: audit: Defer audit buffer association
  tty: audit: Take siglock directly
  tty: audit: Ignore current association for audit push
  tty: audit: Combine push functions
  tty: audit: Track tty association with dev_t
  tty: audit: Handle tty audit enable atomically
  tty: audit: Remove false memory optimization
  tty: audit: Remove tty_audit_buf reference counting
  tty: audit: Simplify first-use allocation
  tty: audit: Check audit enable first
  tty: audit: Always push audit buffer before TIOCSTI
  tty: audit: Poison tty_audit_buf while process exits

 drivers/tty/n_tty.c     |  25 ++----
 drivers/tty/tty_audit.c | 231 ++++++++++++++----------------------------------
 include/linux/audit.h   |   4 +
 include/linux/sched.h   |   1 -
 include/linux/tty.h     |  12 +--
 kernel/audit.c          |  27 +++---
 6 files changed, 97 insertions(+), 203 deletions(-)

-- 
2.7.0

[RESEND][PATCH 01/15] tty: audit: Early-out pty master reads earlier

[Peter Hurley]

Reads from pty masters are not logged; early-out before taking
locks.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 3d245cd..ead924e 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -276,16 +276,16 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
 	if (unlikely(size == 0))
 		return;
 
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY
+	    && tty->driver->subtype == PTY_TYPE_MASTER)
+		return;
+
 	spin_lock_irqsave(&current->sighand->siglock, flags);
 	audit_log_tty_passwd = current->signal->audit_tty_log_passwd;
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
 		return;
 
-	if (tty->driver->type == TTY_DRIVER_TYPE_PTY
-	    && tty->driver->subtype == PTY_TYPE_MASTER)
-		return;
-
 	buf = tty_audit_buf_get(tty, icanon);
 	if (!buf)
 		return;
-- 
2.7.0

[RESEND][PATCH 02/15] tty: audit: Never audit packet mode

[Peter Hurley]

tty audit never logs pty master reads, but packet mode only works for
pty masters, so tty_audit_add_data() was never logging packet mode
anyway.

Don't audit packet mode data. As those are the lone call sites, remove
tty_put_user().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 90eca26..eeae6bc 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -153,15 +153,6 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 	return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
 }
 
-static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
-			       unsigned char __user *ptr)
-{
-	struct n_tty_data *ldata = tty->disc_data;
-
-	tty_audit_add_data(tty, &x, 1, ldata->icanon);
-	return put_user(x, ptr);
-}
-
 static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
 			    size_t tail, size_t n)
 {
@@ -2203,11 +2194,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			cs = tty->link->ctrl_status;
 			tty->link->ctrl_status = 0;
 			spin_unlock_irq(&tty->link->ctrl_lock);
-			if (tty_put_user(tty, cs, b++)) {
+			if (put_user(cs, b)) {
 				retval = -EFAULT;
-				b--;
 				break;
 			}
+			b++;
 			nr--;
 			break;
 		}
@@ -2253,11 +2244,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 
 			/* Deal with packet mode. */
 			if (packet && b == buf) {
-				if (tty_put_user(tty, TIOCPKT_DATA, b++)) {
+				if (put_user(TIOCPKT_DATA, b)) {
 					retval = -EFAULT;
-					b--;
 					break;
 				}
+				b++;
 				nr--;
 			}
 
-- 
2.7.0

[RESEND][PATCH 03/15] tty: audit: Remove icanon mode from call chain

[Peter Hurley]

The tty termios bits cannot change while n_tty_read() is in the
i/o loop; the termios_rwsem ensures mutual exclusion with termios
changes in n_tty_set_termios(). Check L_ICANON() directly and
eliminate icanon parameter.

NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
have no other callers.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c     |  6 +++---
 drivers/tty/tty_audit.c | 22 +++++++++-------------
 include/linux/tty.h     |  4 ++--
 3 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index eeae6bc..5d060fc 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -162,7 +162,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
 	int uncopied;
 
 	if (n > size) {
-		tty_audit_add_data(tty, from, size, ldata->icanon);
+		tty_audit_add_data(tty, from, size);
 		uncopied = copy_to_user(to, from, size);
 		if (uncopied)
 			return uncopied;
@@ -171,7 +171,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
 		from = ldata->read_buf;
 	}
 
-	tty_audit_add_data(tty, from, n, ldata->icanon);
+	tty_audit_add_data(tty, from, n);
 	return copy_to_user(to, from, n);
 }
 
@@ -1984,7 +1984,7 @@ static int copy_from_read_buf(struct tty_struct *tty,
 		retval = copy_to_user(*b, from, n);
 		n -= retval;
 		is_eof = n == 1 && *from == EOF_CHAR(tty);
-		tty_audit_add_data(tty, from, n, ldata->icanon);
+		tty_audit_add_data(tty, from, n);
 		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
 		/* Turn single EOF into zero-length read */
 		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index ead924e..d2a004a 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -22,8 +22,7 @@ struct tty_audit_buf {
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
 };
 
-static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
-						 unsigned icanon)
+static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
 {
 	struct tty_audit_buf *buf;
 
@@ -35,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
 		goto err_buf;
 	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
-	buf->major = major;
-	buf->minor = minor;
-	buf->icanon = icanon;
+	buf->major = tty->driver->major;
+	buf->minor = tty->driver->minor_start + tty->index;
+	buf->icanon = !!L_ICANON(tty);
 	buf->valid = 0;
 	return buf;
 
@@ -216,8 +215,7 @@ int tty_audit_push_current(void)
  *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
  *	reference to the buffer.
  */
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
-		unsigned icanon)
+static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
 {
 	struct tty_audit_buf *buf, *buf2;
 	unsigned long flags;
@@ -234,9 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
 	}
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	buf2 = tty_audit_buf_alloc(tty->driver->major,
-				   tty->driver->minor_start + tty->index,
-				   icanon);
+	buf2 = tty_audit_buf_alloc(tty);
 	if (buf2 == NULL) {
 		audit_log_lost("out of memory in TTY auditing");
 		return NULL;
@@ -265,13 +261,13 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
  *
  *	Audit @data of @size from @tty, if necessary.
  */
-void tty_audit_add_data(struct tty_struct *tty, const void *data,
-			size_t size, unsigned icanon)
+void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 {
 	struct tty_audit_buf *buf;
 	int major, minor;
 	int audit_log_tty_passwd;
 	unsigned long flags;
+	unsigned int icanon = !!L_ICANON(tty);
 
 	if (unlikely(size == 0))
 		return;
@@ -286,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
 	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
 		return;
 
-	buf = tty_audit_buf_get(tty, icanon);
+	buf = tty_audit_buf_get(tty);
 	if (!buf)
 		return;
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 56d1133..c1d1f08 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -604,7 +604,7 @@ extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
 /* tty_audit.c */
 #ifdef CONFIG_AUDIT
 extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
-			       size_t size, unsigned icanon);
+			       size_t size);
 extern void tty_audit_exit(void);
 extern void tty_audit_fork(struct signal_struct *sig);
 extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
@@ -612,7 +612,7 @@ extern void tty_audit_push(struct tty_struct *tty);
 extern int tty_audit_push_current(void);
 #else
 static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
-				      size_t size, unsigned icanon)
+				      size_t size)
 {
 }
 static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
-- 
2.7.0

[RESEND][PATCH 04/15] tty: audit: Defer audit buffer association

[Peter Hurley]

The tty audit buffer used to audit/record tty input is allocated on
the process's first call to tty_audit_add_data(), and not freed until
the process exits. On each call to tty_audit_add_data(), the current
tty is compared (by major:minor) with the last tty associated with
the audit buffer, and if the tty has changed the existing data is
logged to the audit log. The audit buffer is then re-associated with
the new tty.

Currently, the audit buffer is immediately associated with the tty;
however, the association must be re-checked when the buffer is locked
prior to copying the tty input. This extra step is always necessary,
since a concurrent read of a different tty by another thread of the
process may have used the buffer in between allocation and buffer
lock.

Rather than associate the audit buffer with the tty at allocation,
leave the buffer initially un-associated (null dev_t); simply let the
re-association check also perform the initial association.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index d2a004a..9effa81 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -22,7 +22,7 @@ struct tty_audit_buf {
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
 };
 
-static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_alloc(void)
 {
 	struct tty_audit_buf *buf;
 
@@ -34,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
 		goto err_buf;
 	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
-	buf->major = tty->driver->major;
-	buf->minor = tty->driver->minor_start + tty->index;
-	buf->icanon = !!L_ICANON(tty);
+	buf->major = 0;
+	buf->minor = 0;
+	buf->icanon = 0;
 	buf->valid = 0;
 	return buf;
 
@@ -211,11 +211,11 @@ int tty_audit_push_current(void)
 /**
  *	tty_audit_buf_get	-	Get an audit buffer.
  *
- *	Get an audit buffer for @tty, allocate it if necessary.  Return %NULL
+ *	Get an audit buffer, allocate it if necessary.  Return %NULL
  *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
  *	reference to the buffer.
  */
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_get(void)
 {
 	struct tty_audit_buf *buf, *buf2;
 	unsigned long flags;
@@ -232,7 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
 	}
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	buf2 = tty_audit_buf_alloc(tty);
+	buf2 = tty_audit_buf_alloc();
 	if (buf2 == NULL) {
 		audit_log_lost("out of memory in TTY auditing");
 		return NULL;
@@ -282,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
 		return;
 
-	buf = tty_audit_buf_get(tty);
+	buf = tty_audit_buf_get();
 	if (!buf)
 		return;
 
-- 
2.7.0

[RESEND][PATCH 05/15] tty: audit: Take siglock directly

[Peter Hurley]

lock_task_sighand() is for situations where the struct task_struct*
may disappear while trying to deref the sighand; this never applies
to 'current'.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 9effa81..5f65653 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -180,22 +180,19 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 int tty_audit_push_current(void)
 {
 	struct tty_audit_buf *buf = ERR_PTR(-EPERM);
-	struct task_struct *tsk = current;
 	unsigned long flags;
 
-	if (!lock_task_sighand(tsk, &flags))
-		return -ESRCH;
-
-	if (tsk->signal->audit_tty) {
-		buf = tsk->signal->tty_audit_buf;
+	spin_lock_irqsave(&current->sighand->siglock, flags);
+	if (current->signal->audit_tty) {
+		buf = current->signal->tty_audit_buf;
 		if (buf)
 			atomic_inc(&buf->count);
 	}
-	unlock_task_sighand(tsk, &flags);
+	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
 	/*
 	 * Return 0 when signal->audit_tty set
-	 * but tsk->signal->tty_audit_buf == NULL.
+	 * but current->signal->tty_audit_buf == NULL.
 	 */
 	if (!buf || IS_ERR(buf))
 		return PTR_ERR(buf);
-- 
2.7.0

[RESEND][PATCH 06/15] tty: audit: Ignore current association for audit push

[Peter Hurley]

In canonical read mode, each line read and logged is pushed separately
with tty_audit_push(). For all single-threaded processes and multi-threaded
processes reading from only one tty, this patch has no effect; the last line
read will still be the entry pushed to the audit log because the tty
association cannot have changed between tty_audit_add_data() and
tty_audit_push().

For multi-threaded processes reading from different ttys concurrently,
the audit log will have mixed log entries anyway. Consider two ttys
audited concurrently:

CPU0                           CPU1
----------                     ------------
tty_audit_add_data(ttyA)
                               tty_audit_add_data(ttyB)
tty_audit_push()
                               tty_audit_add_data(ttyB)
                               tty_audit_push()

This patch will now cause the ttyB output to be split into separate
audit log entries.

However, this possibility is equally likely without this patch:

CPU0                           CPU1
----------                     ------------
                               tty_audit_add_data(ttyB)
tty_audit_add_data(ttyA)
tty_audit_push()
                               tty_audit_add_data(ttyB)
                               tty_audit_push()

Mixed canonical and non-canonical reads have similar races.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c     |  2 +-
 drivers/tty/tty_audit.c | 11 +++--------
 include/linux/tty.h     |  2 +-
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 5d060fc..6bab08a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2078,7 +2078,7 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
 			ldata->line_start = ldata->read_tail;
 		else
 			ldata->push = 0;
-		tty_audit_push(tty);
+		tty_audit_push();
 	}
 	return 0;
 }
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 5f65653..5ae4839 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -313,9 +313,9 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 /**
  *	tty_audit_push	-	Push buffered data out
  *
- *	Make sure no audit data is pending for @tty on the current process.
+ *	Make sure no audit data is pending on the current process.
  */
-void tty_audit_push(struct tty_struct *tty)
+void tty_audit_push(void)
 {
 	struct tty_audit_buf *buf;
 	unsigned long flags;
@@ -331,13 +331,8 @@ void tty_audit_push(struct tty_struct *tty)
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
 	if (buf) {
-		int major, minor;
-
-		major = tty->driver->major;
-		minor = tty->driver->minor_start + tty->index;
 		mutex_lock(&buf->mutex);
-		if (buf->major == major && buf->minor == minor)
-			tty_audit_buf_push(buf);
+		tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
 		tty_audit_buf_put(buf);
 	}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index c1d1f08..21e3722 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -608,7 +608,7 @@ extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
 extern void tty_audit_exit(void);
 extern void tty_audit_fork(struct signal_struct *sig);
 extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
-extern void tty_audit_push(struct tty_struct *tty);
+extern void tty_audit_push(void);
 extern int tty_audit_push_current(void);
 #else
 static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
-- 
2.7.0

Re: [RESEND][PATCH 06/15] tty: audit: Ignore current association for audit push

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]

Hi Peter,

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on next-20160108]
[cannot apply to v4.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Hurley/Rework-tty-audit/20160110-130735
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: i386-randconfig-s0-201602 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Peter-Hurley/Rework-tty-audit/20160110-130735 HEAD 3fdd6ed9cf68e96432c554fac7a14ef60e77efc3 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/tty/n_tty.c: In function 'canon_copy_from_read_buf':
>> drivers/tty/n_tty.c:2106:3: error: too few arguments to function 'tty_audit_push'
      tty_audit_push();
      ^
   In file included from drivers/tty/n_tty.c:40:0:
   include/linux/tty.h:626:20: note: declared here
    static inline void tty_audit_push(struct tty_struct *tty)
                       ^

vim +/tty_audit_push +2106 drivers/tty/n_tty.c

  2100	
  2101		if (found) {
  2102			if (!ldata->push)
  2103				ldata->line_start = ldata->read_tail;
  2104			else
  2105				ldata->push = 0;
> 2106			tty_audit_push();
  2107		}
  2108		return 0;
  2109	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23080 bytes --]

Re: [RESEND][PATCH 06/15] tty: audit: Ignore current association for audit push

[Peter Hurley]

On 01/09/2016 09:36 PM, kbuild test robot wrote:
> Hi Peter,

Thanks for the report. Just re-spun the v2 series with a fix for this.

Regards,
Peter Hurley

> [auto build test ERROR on tty/tty-testing]
> [also build test ERROR on next-20160108]
> [cannot apply to v4.4-rc8]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Peter-Hurley/Rework-tty-audit/20160110-130735
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
> config: i386-randconfig-s0-201602 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> Note: the linux-review/Peter-Hurley/Rework-tty-audit/20160110-130735 HEAD 3fdd6ed9cf68e96432c554fac7a14ef60e77efc3 builds fine.
>       It only hurts bisectibility.
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/tty/n_tty.c: In function 'canon_copy_from_read_buf':
>>> drivers/tty/n_tty.c:2106:3: error: too few arguments to function 'tty_audit_push'
>       tty_audit_push();
>       ^
>    In file included from drivers/tty/n_tty.c:40:0:
>    include/linux/tty.h:626:20: note: declared here
>     static inline void tty_audit_push(struct tty_struct *tty)
>                        ^
> 
> vim +/tty_audit_push +2106 drivers/tty/n_tty.c
> 
>   2100	
>   2101		if (found) {
>   2102			if (!ldata->push)
>   2103				ldata->line_start = ldata->read_tail;
>   2104			else
>   2105				ldata->push = 0;
>> 2106			tty_audit_push();
>   2107		}
>   2108		return 0;
>   2109	}
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

[RESEND][PATCH 07/15] tty: audit: Combine push functions

[Peter Hurley]

tty_audit_push() and tty_audit_push_current() perform identical
tasks; eliminate the tty_audit_push() implementation and the
tty_audit_push_current() name.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 35 +++--------------------------------
 include/linux/tty.h     |  8 ++------
 kernel/audit.c          |  2 +-
 3 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 5ae4839..6b82c3c 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -172,12 +172,11 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 }
 
 /**
- * tty_audit_push_current -	Flush current's pending audit data
+ *	tty_audit_push	-	Flush current's pending audit data
  *
- * Try to lock sighand and get a reference to the tty audit buffer if available.
- * Flush the buffer or return an appropriate error code.
+ *	Returns 0 if success, -EPERM if tty audit is disabled
  */
-int tty_audit_push_current(void)
+int tty_audit_push(void)
 {
 	struct tty_audit_buf *buf = ERR_PTR(-EPERM);
 	unsigned long flags;
@@ -309,31 +308,3 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	mutex_unlock(&buf->mutex);
 	tty_audit_buf_put(buf);
 }
-
-/**
- *	tty_audit_push	-	Push buffered data out
- *
- *	Make sure no audit data is pending on the current process.
- */
-void tty_audit_push(void)
-{
-	struct tty_audit_buf *buf;
-	unsigned long flags;
-
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (likely(!current->signal->audit_tty)) {
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
-		return;
-	}
-	buf = current->signal->tty_audit_buf;
-	if (buf)
-		atomic_inc(&buf->count);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-
-	if (buf) {
-		mutex_lock(&buf->mutex);
-		tty_audit_buf_push(buf);
-		mutex_unlock(&buf->mutex);
-		tty_audit_buf_put(buf);
-	}
-}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 21e3722..b17f759 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -608,8 +608,7 @@ extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
 extern void tty_audit_exit(void);
 extern void tty_audit_fork(struct signal_struct *sig);
 extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
-extern void tty_audit_push(void);
-extern int tty_audit_push_current(void);
+extern int tty_audit_push(void);
 #else
 static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
 				      size_t size)
@@ -624,10 +623,7 @@ static inline void tty_audit_exit(void)
 static inline void tty_audit_fork(struct signal_struct *sig)
 {
 }
-static inline void tty_audit_push(struct tty_struct *tty)
-{
-}
-static inline int tty_audit_push_current(void)
+static inline int tty_audit_push(void)
 {
 	return 0;
 }
diff --git a/kernel/audit.c b/kernel/audit.c
index 5ffcbd3..270dfb9 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -921,7 +921,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		if (err == 1) { /* match or error */
 			err = 0;
 			if (msg_type == AUDIT_USER_TTY) {
-				err = tty_audit_push_current();
+				err = tty_audit_push();
 				if (err)
 					break;
 			}
-- 
2.7.0

[RESEND][PATCH 08/15] tty: audit: Track tty association with dev_t

[Peter Hurley]

Use dev_t instead of separate major/minor fields to track tty
audit buffer association.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 6b82c3c..50380d8 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -16,7 +16,7 @@
 struct tty_audit_buf {
 	atomic_t count;
 	struct mutex mutex;	/* Protects all data below */
-	int major, minor;	/* The TTY which the data is from */
+	dev_t dev;		/* The TTY which the data is from */
 	unsigned icanon:1;
 	size_t valid;
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
@@ -34,8 +34,7 @@ static struct tty_audit_buf *tty_audit_buf_alloc(void)
 		goto err_buf;
 	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
-	buf->major = 0;
-	buf->minor = 0;
+	buf->dev = MKDEV(0, 0);
 	buf->icanon = 0;
 	buf->valid = 0;
 	return buf;
@@ -59,7 +58,7 @@ static void tty_audit_buf_put(struct tty_audit_buf *buf)
 		tty_audit_buf_free(buf);
 }
 
-static void tty_audit_log(const char *description, int major, int minor,
+static void tty_audit_log(const char *description, dev_t dev,
 			  unsigned char *data, size_t size)
 {
 	struct audit_buffer *ab;
@@ -75,7 +74,7 @@ static void tty_audit_log(const char *description, int major, int minor,
 
 		audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d"
 				 " minor=%d comm=", description, pid, uid,
-				 loginuid, sessionid, major, minor);
+				 loginuid, sessionid, MAJOR(dev), MINOR(dev));
 		get_task_comm(name, tsk);
 		audit_log_untrustedstring(ab, name);
 		audit_log_format(ab, " data=");
@@ -98,7 +97,7 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf)
 		buf->valid = 0;
 		return;
 	}
-	tty_audit_log("tty", buf->major, buf->minor, buf->data, buf->valid);
+	tty_audit_log("tty", buf->dev, buf->data, buf->valid);
 	buf->valid = 0;
 }
 
@@ -141,7 +140,8 @@ void tty_audit_fork(struct signal_struct *sig)
 void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
 	struct tty_audit_buf *buf;
-	int major, minor, should_audit;
+	dev_t dev;
+	int should_audit;
 	unsigned long flags;
 
 	spin_lock_irqsave(&current->sighand->siglock, flags);
@@ -151,11 +151,10 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 		atomic_inc(&buf->count);
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	major = tty->driver->major;
-	minor = tty->driver->minor_start + tty->index;
+	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
 	if (buf) {
 		mutex_lock(&buf->mutex);
-		if (buf->major == major && buf->minor == minor)
+		if (buf->dev == dev)
 			tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
 		tty_audit_buf_put(buf);
@@ -167,7 +166,7 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 
 		auid = audit_get_loginuid(current);
 		sessionid = audit_get_sessionid(current);
-		tty_audit_log("ioctl=TIOCSTI", major, minor, &ch, 1);
+		tty_audit_log("ioctl=TIOCSTI", dev, &ch, 1);
 	}
 }
 
@@ -260,10 +259,10 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 {
 	struct tty_audit_buf *buf;
-	int major, minor;
 	int audit_log_tty_passwd;
 	unsigned long flags;
 	unsigned int icanon = !!L_ICANON(tty);
+	dev_t dev;
 
 	if (unlikely(size == 0))
 		return;
@@ -283,13 +282,10 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 		return;
 
 	mutex_lock(&buf->mutex);
-	major = tty->driver->major;
-	minor = tty->driver->minor_start + tty->index;
-	if (buf->major != major || buf->minor != minor
-	    || buf->icanon != icanon) {
+	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
+	if (buf->dev != dev || buf->icanon != icanon) {
 		tty_audit_buf_push(buf);
-		buf->major = major;
-		buf->minor = minor;
+		buf->dev = dev;
 		buf->icanon = icanon;
 	}
 	do {
-- 
2.7.0

[RESEND][PATCH 09/15] tty: audit: Handle tty audit enable atomically

[Peter Hurley]

The audit_tty and audit_tty_log_passwd fields are actually bool
values, so merge into single memory location to access atomically.

NB: audit log operations may still occur after tty audit is disabled
which is consistent with the existing functionality

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 53 ++++++++++++++++++++-----------------------------
 include/linux/audit.h   |  4 ++++
 include/linux/sched.h   |  1 -
 kernel/audit.c          | 25 +++++++++++------------
 4 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 50380d8..3d90f88 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -131,7 +131,6 @@ void tty_audit_exit(void)
 void tty_audit_fork(struct signal_struct *sig)
 {
 	sig->audit_tty = current->signal->audit_tty;
-	sig->audit_tty_log_passwd = current->signal->audit_tty_log_passwd;
 }
 
 /**
@@ -141,11 +140,9 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
 	struct tty_audit_buf *buf;
 	dev_t dev;
-	int should_audit;
 	unsigned long flags;
 
 	spin_lock_irqsave(&current->sighand->siglock, flags);
-	should_audit = current->signal->audit_tty;
 	buf = current->signal->tty_audit_buf;
 	if (buf)
 		atomic_inc(&buf->count);
@@ -160,7 +157,7 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 		tty_audit_buf_put(buf);
 	}
 
-	if (should_audit && audit_enabled) {
+	if (audit_enabled && (current->signal->audit_tty & AUDIT_TTY_ENABLE)) {
 		kuid_t auid;
 		unsigned int sessionid;
 
@@ -177,29 +174,25 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
  */
 int tty_audit_push(void)
 {
-	struct tty_audit_buf *buf = ERR_PTR(-EPERM);
+	struct tty_audit_buf *buf;
 	unsigned long flags;
 
+	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
+		return -EPERM;
+
 	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (current->signal->audit_tty) {
-		buf = current->signal->tty_audit_buf;
-		if (buf)
-			atomic_inc(&buf->count);
-	}
+	buf = current->signal->tty_audit_buf;
+	if (buf)
+		atomic_inc(&buf->count);
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
-	/*
-	 * Return 0 when signal->audit_tty set
-	 * but current->signal->tty_audit_buf == NULL.
-	 */
-	if (!buf || IS_ERR(buf))
-		return PTR_ERR(buf);
-
-	mutex_lock(&buf->mutex);
-	tty_audit_buf_push(buf);
-	mutex_unlock(&buf->mutex);
+	if (buf) {
+		mutex_lock(&buf->mutex);
+		tty_audit_buf_push(buf);
+		mutex_unlock(&buf->mutex);
 
-	tty_audit_buf_put(buf);
+		tty_audit_buf_put(buf);
+	}
 	return 0;
 }
 
@@ -218,8 +211,6 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 	buf = NULL;
 	buf2 = NULL;
 	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (likely(!current->signal->audit_tty))
-		goto out;
 	buf = current->signal->tty_audit_buf;
 	if (buf) {
 		atomic_inc(&buf->count);
@@ -233,9 +224,10 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 		return NULL;
 	}
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	if (!current->signal->audit_tty)
+	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
 		goto out;
+
+	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
 	if (!buf) {
 		current->signal->tty_audit_buf = buf2;
@@ -259,9 +251,8 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 {
 	struct tty_audit_buf *buf;
-	int audit_log_tty_passwd;
-	unsigned long flags;
 	unsigned int icanon = !!L_ICANON(tty);
+	unsigned int audit_tty;
 	dev_t dev;
 
 	if (unlikely(size == 0))
@@ -271,10 +262,10 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	    && tty->driver->subtype == PTY_TYPE_MASTER)
 		return;
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	audit_log_tty_passwd = current->signal->audit_tty_log_passwd;
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-	if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
+	audit_tty = READ_ONCE(current->signal->audit_tty);
+	if (~audit_tty & AUDIT_TTY_ENABLE)
+		return;
+	if ((~audit_tty & AUDIT_TTY_LOG_PASSWD) && icanon && !L_ECHO(tty))
 		return;
 
 	buf = tty_audit_buf_get();
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 20eba1e..9ed7254 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -109,6 +109,10 @@ extern int audit_classify_compat_syscall(int abi, unsigned syscall);
 /* maximized args number that audit_socketcall can process */
 #define AUDITSC_ARGS		6
 
+/* bit values for ->signal->audit_tty */
+#define AUDIT_TTY_ENABLE	BIT(0)
+#define AUDIT_TTY_LOG_PASSWD	BIT(1)
+
 struct filename;
 
 extern void audit_log_session_info(struct audit_buffer *ab);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index edad7a4..400a738 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -771,7 +771,6 @@ struct signal_struct {
 #endif
 #ifdef CONFIG_AUDIT
 	unsigned audit_tty;
-	unsigned audit_tty_log_passwd;
 	struct tty_audit_buf *tty_audit_buf;
 #endif
 
diff --git a/kernel/audit.c b/kernel/audit.c
index 270dfb9..dfaa8e7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1031,20 +1031,19 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		break;
 	case AUDIT_TTY_GET: {
 		struct audit_tty_status s;
-		struct task_struct *tsk = current;
+		unsigned int t;
 
-		spin_lock(&tsk->sighand->siglock);
-		s.enabled = tsk->signal->audit_tty;
-		s.log_passwd = tsk->signal->audit_tty_log_passwd;
-		spin_unlock(&tsk->sighand->siglock);
+		t = READ_ONCE(current->signal->audit_tty);
+		s.enabled = t & AUDIT_TTY_ENABLE;
+		s.log_passwd = !!(t & AUDIT_TTY_LOG_PASSWD);
 
 		audit_send_reply(skb, seq, AUDIT_TTY_GET, 0, 0, &s, sizeof(s));
 		break;
 	}
 	case AUDIT_TTY_SET: {
 		struct audit_tty_status s, old;
-		struct task_struct *tsk = current;
 		struct audit_buffer	*ab;
+		unsigned int t;
 
 		memset(&s, 0, sizeof(s));
 		/* guard against past and future API changes */
@@ -1054,14 +1053,14 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		    (s.log_passwd != 0 && s.log_passwd != 1))
 			err = -EINVAL;
 
-		spin_lock(&tsk->sighand->siglock);
-		old.enabled = tsk->signal->audit_tty;
-		old.log_passwd = tsk->signal->audit_tty_log_passwd;
-		if (!err) {
-			tsk->signal->audit_tty = s.enabled;
-			tsk->signal->audit_tty_log_passwd = s.log_passwd;
+		if (err)
+			t = READ_ONCE(current->signal->audit_tty);
+		else {
+			t = s.enabled | (-s.log_passwd & AUDIT_TTY_LOG_PASSWD);
+			t = xchg(&current->signal->audit_tty, t);
 		}
-		spin_unlock(&tsk->sighand->siglock);
+		old.enabled = t & AUDIT_TTY_ENABLE;
+		old.log_passwd = !!(t & AUDIT_TTY_LOG_PASSWD);
 
 		audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
 		audit_log_format(ab, " op=tty_set old-enabled=%d new-enabled=%d"
-- 
2.7.0

[RESEND][PATCH 10/15] tty: audit: Remove false memory optimization

[Peter Hurley]

The tty audit buffer is allocated at first use and not freed until
the process exits. If tty audit is turned off after the audit buffer
has been allocated, no effort is made to release the buffer.
So re-checking if tty audit has just been turned off when tty audit
was just on is false optimization; the likelihood of triggering this
condition is exceedingly small.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 3d90f88..7943984 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -200,8 +200,7 @@ int tty_audit_push(void)
  *	tty_audit_buf_get	-	Get an audit buffer.
  *
  *	Get an audit buffer, allocate it if necessary.  Return %NULL
- *	if TTY auditing is disabled or out of memory.  Otherwise, return a new
- *	reference to the buffer.
+ *	if out of memory.  Otherwise, return a new reference to the buffer.
  */
 static struct tty_audit_buf *tty_audit_buf_get(void)
 {
@@ -224,9 +223,6 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 		return NULL;
 	}
 
-	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
-		goto out;
-
 	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
 	if (!buf) {
-- 
2.7.0

[RESEND][PATCH 11/15] tty: audit: Remove tty_audit_buf reference counting

[Peter Hurley]

When tty_audit_exit() is called from do_exit(), the process is
single-threaded. Since the tty_audit_buf is only shared by threads
of a process, no other thread can be concurrently accessing the
tty_audit_buf during or after tty_audit_exit().

Thus, no other thread can be holding an extra tty_audit_buf reference
which would prevent tty_audit_exit() from freeing the tty_audit_buf.
As that is the only purpose of the ref counting, remove the reference
counting and free the tty_audit_buf directly.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 47 +++++++----------------------------------------
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 7943984..71ba8ba 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -14,7 +14,6 @@
 #include <linux/tty.h>
 
 struct tty_audit_buf {
-	atomic_t count;
 	struct mutex mutex;	/* Protects all data below */
 	dev_t dev;		/* The TTY which the data is from */
 	unsigned icanon:1;
@@ -32,7 +31,6 @@ static struct tty_audit_buf *tty_audit_buf_alloc(void)
 	buf->data = kmalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
 	if (!buf->data)
 		goto err_buf;
-	atomic_set(&buf->count, 1);
 	mutex_init(&buf->mutex);
 	buf->dev = MKDEV(0, 0);
 	buf->icanon = 0;
@@ -52,12 +50,6 @@ static void tty_audit_buf_free(struct tty_audit_buf *buf)
 	kfree(buf);
 }
 
-static void tty_audit_buf_put(struct tty_audit_buf *buf)
-{
-	if (atomic_dec_and_test(&buf->count))
-		tty_audit_buf_free(buf);
-}
-
 static void tty_audit_log(const char *description, dev_t dev,
 			  unsigned char *data, size_t size)
 {
@@ -106,6 +98,9 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf)
  *
  *	Make sure all buffered data is written out and deallocate the buffer.
  *	Only needs to be called if current->signal->tty_audit_buf != %NULL.
+ *
+ *	The process is single-threaded at this point; no other threads share
+ *	current->signal.
  */
 void tty_audit_exit(void)
 {
@@ -116,11 +111,8 @@ void tty_audit_exit(void)
 	if (!buf)
 		return;
 
-	mutex_lock(&buf->mutex);
 	tty_audit_buf_push(buf);
-	mutex_unlock(&buf->mutex);
-
-	tty_audit_buf_put(buf);
+	tty_audit_buf_free(buf);
 }
 
 /**
@@ -140,21 +132,14 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
 	struct tty_audit_buf *buf;
 	dev_t dev;
-	unsigned long flags;
-
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	buf = current->signal->tty_audit_buf;
-	if (buf)
-		atomic_inc(&buf->count);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
 	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
+	buf = current->signal->tty_audit_buf;
 	if (buf) {
 		mutex_lock(&buf->mutex);
 		if (buf->dev == dev)
 			tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
-		tty_audit_buf_put(buf);
 	}
 
 	if (audit_enabled && (current->signal->audit_tty & AUDIT_TTY_ENABLE)) {
@@ -175,23 +160,15 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 int tty_audit_push(void)
 {
 	struct tty_audit_buf *buf;
-	unsigned long flags;
 
 	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
 		return -EPERM;
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
-	if (buf)
-		atomic_inc(&buf->count);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-
 	if (buf) {
 		mutex_lock(&buf->mutex);
 		tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
-
-		tty_audit_buf_put(buf);
 	}
 	return 0;
 }
@@ -207,15 +184,9 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 	struct tty_audit_buf *buf, *buf2;
 	unsigned long flags;
 
-	buf = NULL;
-	buf2 = NULL;
-	spin_lock_irqsave(&current->sighand->siglock, flags);
 	buf = current->signal->tty_audit_buf;
-	if (buf) {
-		atomic_inc(&buf->count);
-		goto out;
-	}
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+	if (buf)
+		return buf;
 
 	buf2 = tty_audit_buf_alloc();
 	if (buf2 == NULL) {
@@ -230,9 +201,6 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 		buf = buf2;
 		buf2 = NULL;
 	}
-	atomic_inc(&buf->count);
-	/* Fall through */
- out:
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 	if (buf2)
 		tty_audit_buf_free(buf2);
@@ -289,5 +257,4 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 			tty_audit_buf_push(buf);
 	} while (size != 0);
 	mutex_unlock(&buf->mutex);
-	tty_audit_buf_put(buf);
 }
-- 
2.7.0

[RESEND][PATCH 12/15] tty: audit: Simplify first-use allocation

[Peter Hurley]

The first-use tty audit buffer allocation is a potential race
amongst multiple attempts at 'first-use'; only one 'winner' is
acceptable.

The successful buffer assignment occurs if tty_audit_buf == NULL
(which will also be the return from cmpxchg()); otherwise, another
racer 'won' and this buffer allocation is freed.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 71ba8ba..6e33e41 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -181,30 +181,22 @@ int tty_audit_push(void)
  */
 static struct tty_audit_buf *tty_audit_buf_get(void)
 {
-	struct tty_audit_buf *buf, *buf2;
-	unsigned long flags;
+	struct tty_audit_buf *buf;
 
 	buf = current->signal->tty_audit_buf;
 	if (buf)
 		return buf;
 
-	buf2 = tty_audit_buf_alloc();
-	if (buf2 == NULL) {
+	buf = tty_audit_buf_alloc();
+	if (buf == NULL) {
 		audit_log_lost("out of memory in TTY auditing");
 		return NULL;
 	}
 
-	spin_lock_irqsave(&current->sighand->siglock, flags);
-	buf = current->signal->tty_audit_buf;
-	if (!buf) {
-		current->signal->tty_audit_buf = buf2;
-		buf = buf2;
-		buf2 = NULL;
-	}
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
-	if (buf2)
-		tty_audit_buf_free(buf2);
-	return buf;
+	/* Race to use this buffer, free it if another wins */
+	if (cmpxchg(&current->signal->tty_audit_buf, NULL, buf) != NULL)
+		tty_audit_buf_free(buf);
+	return current->signal->tty_audit_buf;
 }
 
 /**
-- 
2.7.0

[RESEND][PATCH 13/15] tty: audit: Check audit enable first

[Peter Hurley]

Audit is unlikely to be enabled; check first to exit asap.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 6e33e41..269e41f 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -211,6 +211,10 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	unsigned int audit_tty;
 	dev_t dev;
 
+	audit_tty = READ_ONCE(current->signal->audit_tty);
+	if (~audit_tty & AUDIT_TTY_ENABLE)
+		return;
+
 	if (unlikely(size == 0))
 		return;
 
@@ -218,9 +222,6 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 	    && tty->driver->subtype == PTY_TYPE_MASTER)
 		return;
 
-	audit_tty = READ_ONCE(current->signal->audit_tty);
-	if (~audit_tty & AUDIT_TTY_ENABLE)
-		return;
 	if ((~audit_tty & AUDIT_TTY_LOG_PASSWD) && icanon && !L_ECHO(tty))
 		return;
 
-- 
2.7.0

[RESEND][PATCH 14/15] tty: audit: Always push audit buffer before TIOCSTI

[Peter Hurley]

The data read from another tty may be relevant to the action of
the TIOCSTI ioctl; log the audit buffer immediately.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 269e41f..fa461dc 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -130,19 +130,13 @@ void tty_audit_fork(struct signal_struct *sig)
  */
 void tty_audit_tiocsti(struct tty_struct *tty, char ch)
 {
-	struct tty_audit_buf *buf;
 	dev_t dev;
 
 	dev = MKDEV(tty->driver->major, tty->driver->minor_start) + tty->index;
-	buf = current->signal->tty_audit_buf;
-	if (buf) {
-		mutex_lock(&buf->mutex);
-		if (buf->dev == dev)
-			tty_audit_buf_push(buf);
-		mutex_unlock(&buf->mutex);
-	}
+	if (tty_audit_push())
+		return;
 
-	if (audit_enabled && (current->signal->audit_tty & AUDIT_TTY_ENABLE)) {
+	if (audit_enabled) {
 		kuid_t auid;
 		unsigned int sessionid;
 
-- 
2.7.0

[RESEND][PATCH 15/15] tty: audit: Poison tty_audit_buf while process exits

[Peter Hurley]

Warn if tty_audit_buf use is attempted after tty_audit_exit() has
already freed it.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_audit.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index fa461dc..66d53fc 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -21,6 +21,15 @@ struct tty_audit_buf {
 	unsigned char *data;	/* Allocated size N_TTY_BUF_SIZE */
 };
 
+static struct tty_audit_buf *tty_audit_buf_ref(void)
+{
+	struct tty_audit_buf *buf;
+
+	buf = current->signal->tty_audit_buf;
+	WARN_ON(buf == ERR_PTR(-ESRCH));
+	return buf;
+}
+
 static struct tty_audit_buf *tty_audit_buf_alloc(void)
 {
 	struct tty_audit_buf *buf;
@@ -106,8 +115,7 @@ void tty_audit_exit(void)
 {
 	struct tty_audit_buf *buf;
 
-	buf = current->signal->tty_audit_buf;
-	current->signal->tty_audit_buf = NULL;
+	buf = xchg(&current->signal->tty_audit_buf, ERR_PTR(-ESRCH));
 	if (!buf)
 		return;
 
@@ -158,8 +166,8 @@ int tty_audit_push(void)
 	if (~current->signal->audit_tty & AUDIT_TTY_ENABLE)
 		return -EPERM;
 
-	buf = current->signal->tty_audit_buf;
-	if (buf) {
+	buf = tty_audit_buf_ref();
+	if (!IS_ERR_OR_NULL(buf)) {
 		mutex_lock(&buf->mutex);
 		tty_audit_buf_push(buf);
 		mutex_unlock(&buf->mutex);
@@ -171,13 +179,14 @@ int tty_audit_push(void)
  *	tty_audit_buf_get	-	Get an audit buffer.
  *
  *	Get an audit buffer, allocate it if necessary.  Return %NULL
- *	if out of memory.  Otherwise, return a new reference to the buffer.
+ *	if out of memory or ERR_PTR(-ESRCH) if tty_audit_exit() has already
+ *	occurred.  Otherwise, return a new reference to the buffer.
  */
 static struct tty_audit_buf *tty_audit_buf_get(void)
 {
 	struct tty_audit_buf *buf;
 
-	buf = current->signal->tty_audit_buf;
+	buf = tty_audit_buf_ref();
 	if (buf)
 		return buf;
 
@@ -190,7 +199,7 @@ static struct tty_audit_buf *tty_audit_buf_get(void)
 	/* Race to use this buffer, free it if another wins */
 	if (cmpxchg(&current->signal->tty_audit_buf, NULL, buf) != NULL)
 		tty_audit_buf_free(buf);
-	return current->signal->tty_audit_buf;
+	return tty_audit_buf_ref();
 }
 
 /**
@@ -220,7 +229,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
 		return;
 
 	buf = tty_audit_buf_get();
-	if (!buf)
+	if (IS_ERR_OR_NULL(buf))
 		return;
 
 	mutex_lock(&buf->mutex);
-- 
2.7.0