* [PATCH 08/15] tty: audit: Track tty association with dev_t
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->sighand->siglock, flags);
@@ -151,11 +151,10 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
atomic_inc(&buf->count);
spin_unlock_irqrestore(¤t->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
^ permalink raw reply related
* [PATCH 07/15] tty: audit: Combine push functions
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->sighand->siglock, flags);
- if (likely(!current->signal->audit_tty)) {
- spin_unlock_irqrestore(¤t->sighand->siglock, flags);
- return;
- }
- buf = current->signal->tty_audit_buf;
- if (buf)
- atomic_inc(&buf->count);
- spin_unlock_irqrestore(¤t->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
^ permalink raw reply related
* [PATCH 06/15] tty: audit: Ignore current association for audit push
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->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
^ permalink raw reply related
* [PATCH 05/15] tty: audit: Take siglock directly
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->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(¤t->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
^ permalink raw reply related
* [PATCH 04/15] tty: audit: Defer audit buffer association
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->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
^ permalink raw reply related
* [PATCH 03/15] tty: audit: Remove icanon mode from call chain
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->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
^ permalink raw reply related
* [PATCH 02/15] tty: audit: Never audit packet mode
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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
^ permalink raw reply related
* [PATCH 00/15] Rework tty audit
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Peter Zijlstra, Oleg Nesterov, Ingo Molnar, linux-audit,
Jiri Slaby, 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
^ permalink raw reply
* [PATCH 01/15] tty: audit: Early-out pty master reads earlier
From: Peter Hurley @ 2015-11-11 2:05 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-audit, Jiri Slaby, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
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(¤t->sighand->siglock, flags);
audit_log_tty_passwd = current->signal->audit_tty_log_passwd;
spin_unlock_irqrestore(¤t->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
^ permalink raw reply related
* Re: audit log still getting rotated even with max_log_file_action = ignore?
From: Bond Masuda @ 2015-11-08 6:05 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit
In-Reply-To: <37102497.kx1YEe0YIN@x2>
On 11/06/2015 11:12 AM, Steve Grubb wrote:
> On Friday, November 06, 2015 10:07:24 AM Bond Masuda wrote:
>> On 11/02/2015 03:32 PM, Steve Grubb wrote:
>>> I took a quick look at the code. I can't see how this is happening
>>> unless auditd is receiving a SIGUSR1 signal. You might want to put
>>> some syslog calls in to auditd-event.c log when auditd gets told to
>>> rotate so that it can be correlated to other system activities. -Steve
>> Hi Steve,
>>
>> The cron script i mention below does use "service auditd rotate", which
>> does send a SIGUSR1. But these rotations are happening outside the time
>> frame when that cron job runs.
> Can you find any other cron job running around that time?
>
I'm still trying to hunt this down; haven't found anything thus far, but
its possible I could have missed something.
>> Additionally, they seem to rotate around when the log file reaches about
>> 90MB. It almost seems like there's some default behavior?
> The settings to note in your email are these:
>
> num_logs = 5
> max_log_file = 6
> max_log_file_action = ignore
> admin_space_left = 50
> admin_space_left_action = exec /usr/local/bin/remove_oldest_audit_log
>
> This means you would have 6 log files that 5 MB each. However, the max_log_file
> action says ignore.
>
>
>> I was wondering if maybe my syntax in the config file was wrong and auditd was
>> ignoring my setting and just using defaults?
> It might be that you are hitting the admin_space_left_action which runs
> remove_oldest_audit_log. That is my only guess. Does the math work out for
> partition size - size of all logs being approximayely 50MB? If so, this is
> your problem and you might need a bigger partition.
I don't think that's the case:
# df -h /var/log/audit/
Filesystem Size Used Avail Use% Mounted on
/dev/mapper/vg_system-lv_audit
1014M 413M 602M 41% /audit
> But based on a quick review of the man page, you might set num_logs = 0. That
> is supposed to disable rotating as long as max_log_size_action != rotate. You
> have ignore, so that should work if you don't have the problem noted above.
>
> -Steve
>
Ok, will try this and see if that unexpected rotation goes away.
Thanks,
Bond
^ permalink raw reply
* Re: seccomp and audit_enabled
From: Tony Jones @ 2015-11-06 21:45 UTC (permalink / raw)
To: Steve Grubb, linux-audit; +Cc: Paul Moore, linux-security-module
In-Reply-To: <1489773.CGhBT1IxtY@x2>
On 10/13/2015 01:03 PM, Steve Grubb wrote:
>> No, it's the default audit.rules (-D, -b320). No actual rules loaded.
>> Let me add some instrumentation and figure out what's going on. auditd
>> is masked (via systemd) but systemd-journal seems to set audit_enabled=1
>> during startup (at least on our systems).
>
> Tony,
>
> We have bz 1227379
> https://bugzilla.redhat.com/show_bug.cgi?id=1227379
>
> There is a patch attached to disable systemd's propensity to turn on the audit
> system. Are people complaining and opening bugs in your distribution? If so,
> that might add more ammunition to get that fixed.
Hi Steve
we only have the one bug and it's related to:
1) noisy klog between when systemd enables audit and user manually disables it (rh bz#1160046)
2) after user manually disables audit (audit_enabled=0) seccomp messages still are output.
tony
^ permalink raw reply
* Re: seccomp and audit_enabled
From: Tony Jones @ 2015-11-06 21:36 UTC (permalink / raw)
To: Paul Moore; +Cc: Kees Cook, linux-audit, linux-security-module
In-Reply-To: <CAHC9VhQgDJAW0RrORwzRT0T1BaV7BbqCQvNmW7F6n2v6_=0K6A@mail.gmail.com>
On 10/13/2015 12:19 PM, Paul Moore wrote:
> Yes, if systemd is involved it enables audit; we've had some
> discussions with the systemd folks about fixing that, but they haven't
> gone very far. I'm still a little curious as to why
> audit_dummy_context() is false in this case, but I haven't looked at
> how systemd/auditctl start/config the system too closely.
Sorry for the delay here.
A context is allocated by audit_alloc() because there is no uid/gid filter for the task
but the dummy flag is left false. Because audit has been disabled (manually following systemd enabling),
dummy never gets set in the syscall entry path (based on !audit_n_rules). So the unlikely(!audit_dummy_context())
in audit_seccomp succeeds.
Tony
^ permalink raw reply
* Re: audit log still getting rotated even with max_log_file_action = ignore?
From: Steve Grubb @ 2015-11-06 19:12 UTC (permalink / raw)
To: Bond Masuda; +Cc: linux-audit
In-Reply-To: <563CEC5C.5010301@jlbond.com>
On Friday, November 06, 2015 10:07:24 AM Bond Masuda wrote:
> On 11/02/2015 03:32 PM, Steve Grubb wrote:
> > I took a quick look at the code. I can't see how this is happening
> > unless auditd is receiving a SIGUSR1 signal. You might want to put
> > some syslog calls in to auditd-event.c log when auditd gets told to
> > rotate so that it can be correlated to other system activities. -Steve
>
> Hi Steve,
>
> The cron script i mention below does use "service auditd rotate", which
> does send a SIGUSR1. But these rotations are happening outside the time
> frame when that cron job runs.
Can you find any other cron job running around that time?
> Additionally, they seem to rotate around when the log file reaches about
> 90MB. It almost seems like there's some default behavior?
The settings to note in your email are these:
num_logs = 5
max_log_file = 6
max_log_file_action = ignore
admin_space_left = 50
admin_space_left_action = exec /usr/local/bin/remove_oldest_audit_log
This means you would have 6 log files that 5 MB each. However, the max_log_file
action says ignore.
> I was wondering if maybe my syntax in the config file was wrong and auditd was
> ignoring my setting and just using defaults?
It might be that you are hitting the admin_space_left_action which runs
remove_oldest_audit_log. That is my only guess. Does the math work out for
partition size - size of all logs being approximayely 50MB? If so, this is
your problem and you might need a bigger partition.
But based on a quick review of the man page, you might set num_logs = 0. That
is supposed to disable rotating as long as max_log_size_action != rotate. You
have ignore, so that should work if you don't have the problem noted above.
-Steve
> >> I have a cron job in /etc/cron.daily/auditd that I use to rotate +
> >> compress the audit logs, but this is not what is causing the audit log
> >> rotation.
> >>
> >> Is there another setting I must set in order for it to not automatically
> >> rotate the audit log? How do I achieve the desired effect, where the
> >> audit log is only rotated when my cron script runs?
^ permalink raw reply
* Re: audit log still getting rotated even with max_log_file_action = ignore?
From: Bond Masuda @ 2015-11-06 18:07 UTC (permalink / raw)
To: Steve Grubb, linux-audit
In-Reply-To: <2015477.b7QVr7lf9X@x2>
On 11/02/2015 03:32 PM, Steve Grubb wrote:
> I took a quick look at the code. I can't see how this is happening
> unless auditd is receiving a SIGUSR1 signal. You might want to put
> some syslog calls in to auditd-event.c log when auditd gets told to
> rotate so that it can be correlated to other system activities. -Steve
Hi Steve,
The cron script i mention below does use "service auditd rotate", which
does send a SIGUSR1. But these rotations are happening outside the time
frame when that cron job runs. Additionally, they seem to rotate around
when the log file reaches about 90MB. It almost seems like there's some
default behavior? I was wondering if maybe my syntax in the config file
was wrong and auditd was ignoring my setting and just using defaults?
Bond
>> I have a cron job in /etc/cron.daily/auditd that I use to rotate +
>> compress the audit logs, but this is not what is causing the audit log
>> rotation.
>>
>> Is there another setting I must set in order for it to not automatically
>> rotate the audit log? How do I achieve the desired effect, where the
>> audit log is only rotated when my cron script runs?
^ permalink raw reply
* Re: SELinux policy reload cannot be sent to audit system
From: Paul Moore @ 2015-11-06 1:25 UTC (permalink / raw)
To: Laurent Bigonville, Steve Grubb; +Cc: linux-audit
In-Reply-To: <563BE3F5.6030808@debian.org>
Thanks guys, it looks like you found the root cause. It was on my
todo list to play with this on Rawhide but I wanted to get through
Richard's patches first.
On Thu, Nov 5, 2015 at 6:19 PM, Laurent Bigonville <bigon@debian.org> wrote:
> Le 06/11/15 00:03, Steve Grubb a écrit :
>
>> On Thursday, November 05, 2015 09:32:09 AM Laurent Bigonville wrote:
>>>
>>> Le 05/11/15 04:23, Steve Grubb a écrit :
>>>>
>>>> On Tuesday, November 03, 2015 09:48:31 PM Laurent Bigonville wrote:
>>>>>
>>>>> Le 03/11/15 21:08, Richard Guy Briggs a écrit :
>>>>>>
>>>>>> On 15/11/03, Steve Grubb wrote:
>>>>>>>
>>>>>>> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
>>>>>>>>
>>>>>>>> I'm running in permissive mode.
>>>>>>>>
>>>>>>>> I'm seeing a netlink open to the audit:
>>>>>>>>
>>>>>>>> dbus-daem 1057 messagebus 7u netlink 0t0 15248 AUDIT
>>>>>>>>
>>>>>>>> Apparently audit_send() returns -1
>>>>>>>
>>>>>>> Since its -1, that would be an EPERM. No idea where this is coming
>>>>>>> from
>>>>>>> if you have CAP_AUDIT_WRITE. I use pscap to check that.
>>>>>>
>>>>>> Are you in a container of any kind or any non-init USER namespace? I
>>>>>> can't see it being denied otherwise assuming it is only trying to send
>>>>>> AUDIT_USER_* class messages. (This assumes upstream kernel.)
>>>>>
>>>>> No, I initially saw this on my laptop and then tested on F23 in kvm.
>>>>
>>>> I tested this on Fedora 22 and did not get a USER_AVC from dbus, but I
>>>> also
>>>> did not get an error message in syslog. So, I don't know what to make of
>>>> it. (And for the record, I have a bz open saying that USER_AVC is the
>>>> wrong event type. They are blaming libselinux but I blame them for not
>>>> using
>>>> AUDIT_USER_MAC_POLICY_LOAD.)
>>>
>>> The audit code in dbus has been refactored a bit in the version present
>>> F23 and debian unstable, so it might be related to this that.
>>
>>
>> I filed a bz to get this fixed:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1278602
>>
>> The root cause is listed in the bug. Dbus has 2 threads, one with
>> CAP_AUDIT_WRITE and one without. The one without is the one trying to send
>> the
>> event.
>
> Thanks,
>
> I've opened a bug upstream too:
> https://bugs.freedesktop.org/show_bug.cgi?id=92832
>
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply
* Re: [RFC PATCH 7/7] audit: wake up kauditd_thread after auditd registers
From: Paul Moore @ 2015-11-06 1:23 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <0561e1479db8d7ac6d58e6b7d37647dc1cb62fb0.1445539473.git.rgb@redhat.com>
On Thursday, October 22, 2015 02:53:20 PM Richard Guy Briggs wrote:
> When auditd is restarted, even though the kauditd_thread is present, it
> remains dormant until the next audit log message is queued.
>
> Wake up the kauditd_thread in the kauditd_wait queue immediately when
> auditd registers its availability to drain the queue.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/audit.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
See my 6/7 comment ... this could/should go in the auditd_register() function.
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 688fa1e..369cfcc 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -885,6 +885,8 @@ static int audit_receive_msg(struct sk_buff *skb, struct
> nlmsghdr *nlh) audit_sock = skb->sk;
> if (!audit_pid)
> wake_up(&audit_backlog_wait);
> + if (audit_pid)
> + wake_up_interruptible(&kauditd_wait);
> }
> if (s.mask & AUDIT_STATUS_RATE_LIMIT) {
> err = audit_set_rate_limit(s.rate_limit);
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 6/7] audit: wake up audit_backlog_wait queue when auditd goes away.
From: Paul Moore @ 2015-11-06 1:21 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <473f52eb9d2c5d218106447084f72f6cb61245de.1445539473.git.rgb@redhat.com>
On Thursday, October 22, 2015 02:53:19 PM Richard Guy Briggs wrote:
> When auditd goes away (died, killed or shutdown, or net namespace shut
> down), there is no point in sleeping waiting for auditd to drain the
> queue since that message would be distined for the hold queue after the
> timeout anyways. This will needlessly have those processes wait the
> full default timeout of 60 seconds (audit_backlog_wait_time).
>
> Wake up the processes caught in the audit_backlog_wait queue when auditd
> is no longer present so they can be sent instead to the hold queue.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/audit.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 34411af..688fa1e 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -425,6 +425,7 @@ restart:
> audit_log_lost(s);
> audit_pid = 0;
> audit_sock = NULL;
> + wake_up(&audit_backlog_wait);
> } else {
> pr_warn("re-scheduling(#%d) write to audit_pid=%d\n",
> attempts, audit_pid);
> @@ -882,6 +883,8 @@ static int audit_receive_msg(struct sk_buff *skb, struct
> nlmsghdr *nlh) audit_pid = new_pid;
> audit_nlk_portid = NETLINK_CB(skb).portid;
> audit_sock = skb->sk;
> + if (!audit_pid)
> + wake_up(&audit_backlog_wait);
> }
> if (s.mask & AUDIT_STATUS_RATE_LIMIT) {
> err = audit_set_rate_limit(s.rate_limit);
I'm thinking it might be time for two small, static helper functions,
auditd_register() and auditd_unregister() (or similar, feel free to suggest
other names), that set/reset the various auditd state variables and handle the
wake_up() call. We're duplicating some code that is starting to get non-
trivial.
I'd also add a comment about why you are calling wake_up() in the unregister
function.
> @@ -1154,6 +1157,7 @@ static void __net_exit audit_net_exit(struct net *net)
> if (sock == audit_sock) {
> audit_pid = 0;
> audit_sock = NULL;
> + wake_up(&audit_backlog_wait);
> }
>
> RCU_INIT_POINTER(aunet->nlsk, NULL);
> @@ -1393,7 +1397,7 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask, sleep_time = timeout_start +
> audit_backlog_wait_time - jiffies; if (sleep_time > 0) {
> sleep_time = wait_for_auditd(sleep_time);
> - if (sleep_time > 0)
> + if (audit_pid && sleep_time > 0)
> continue;
Perhaps handle this in wait_for_auditd()? Right now this is the only caller,
but if we use it elsewhere it seems like we would want the same logic.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 5/7] audit: allow audit_cmd_mutex holders to use reserves
From: Paul Moore @ 2015-11-06 0:48 UTC (permalink / raw)
To: linux-audit; +Cc: Richard Guy Briggs, linux-kernel, v.rathor
In-Reply-To: <7f62a8c1d8cf031dda3c41bdf39ee4902e6f897e.1445539473.git.rgb@redhat.com>
On Thursday, October 22, 2015 02:53:18 PM Richard Guy Briggs wrote:
> If we hold the audit_cmd_mutex, we should never sleep waiting for auditd
> to drain the queue since auditd may need the mutex to shut down.
>
> This was first implemented with mutex_trylock(), but since
> audit_log_start() can be called in softirq context, that won't work.
> Next, owner_running() was used to check audit_cmd_mutex but another
> process could have this locked on another cpu. Use rcu_read_lock() and
> ACCESS_ONCE() to check audit_cmd_mutex.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/audit.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
Ungh. This is painful ... and I'm talking about the problem, not necessarily
the solution your proposing here. I'm going to pass on this patch for now
because I'd like to see us step back and reexamine our approach here.
When it comes down to it, audit_cmd_mutex is really just there because we
don't have proper, granular locking in audit_receive_msg(), right? Looking
quickly at it, it appears that AUDIT_GET/SET could be dealt with via a
spinlock (we could add RCU if GET is frequent) ... similar could be done with
AUDIT_GET/SET_FEATURE ... AUDIT_USER is a little more complex and not
immediately obvious, but it looks like most of the pain points
(audit_filter_user() and tty_audit_push_current() are already safe ...
AUDIT_ADD/DEL_RULE look to be already protected via the audit_filter_mutex ...
same with AUDIT_LIST_RULES ... same with AUDIT_TRIM ... same with
AUDIT_MAKE_EQUIV ... AUDIT_SIGNAL_INFO shouldn't be a problem ...
AUDIT_TTY_GET/SET already have spinlocks.
Am I missing something?
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 02a5ec0..34411af 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1376,12 +1376,15 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask, return NULL;
>
> if (gfp_mask & __GFP_WAIT) {
> - if (current->tgid == 1 || (audit_pid && audit_pid == current->tgid))
> + rcu_read_lock();
> + if (ACCESS_ONCE(audit_cmd_mutex.owner) == current ||
> + current->tgid == 1 ||
> + (audit_pid && audit_pid == current->tgid))
> gfp_mask &= ~__GFP_WAIT;
> else
> reserve = 0;
> + rcu_read_unlock();
> }
> -
> while (audit_backlog_limit
> && skb_queue_len(&audit_skb_queue) > audit_backlog_limit + reserve)
> { if (gfp_mask & __GFP_WAIT && audit_backlog_wait_time) {
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 4/7] audit: wake up threads if queue switched from limited to unlimited
From: Paul Moore @ 2015-11-06 0:05 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <8a0bd940090b5c4cc19869c6126bdcc85e3be940.1445539473.git.rgb@redhat.com>
On Thursday, October 22, 2015 02:53:17 PM Richard Guy Briggs wrote:
> If the audit_backlog_limit is changed from a limited value to an
> unlimited value (zero) while the queue was overflowed, wake up the
> audit_backlog_wait queue to allow those processes to continue.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/audit.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
Looks like the right thing to do, merged to audit#next-queue.
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 384a1a1..02a5ec0 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -523,7 +523,8 @@ static int kauditd_thread(void *dummy)
> skb = skb_dequeue(&audit_skb_queue);
>
> if (skb) {
> - if (skb_queue_len(&audit_skb_queue) <= audit_backlog_limit)
> + if (!audit_backlog_limit ||
> + (skb_queue_len(&audit_skb_queue) <= audit_backlog_limit))
> wake_up(&audit_backlog_wait);
> if (audit_pid)
> kauditd_send_skb(skb);
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-05 23:19 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <4121374.MmbOH8Er09@x2>
Le 06/11/15 00:03, Steve Grubb a écrit :
> On Thursday, November 05, 2015 09:32:09 AM Laurent Bigonville wrote:
>> Le 05/11/15 04:23, Steve Grubb a écrit :
>>> On Tuesday, November 03, 2015 09:48:31 PM Laurent Bigonville wrote:
>>>> Le 03/11/15 21:08, Richard Guy Briggs a écrit :
>>>>> On 15/11/03, Steve Grubb wrote:
>>>>>> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
>>>>>>> I'm running in permissive mode.
>>>>>>>
>>>>>>> I'm seeing a netlink open to the audit:
>>>>>>>
>>>>>>> dbus-daem 1057 messagebus 7u netlink 0t0 15248 AUDIT
>>>>>>>
>>>>>>> Apparently audit_send() returns -1
>>>>>> Since its -1, that would be an EPERM. No idea where this is coming from
>>>>>> if you have CAP_AUDIT_WRITE. I use pscap to check that.
>>>>> Are you in a container of any kind or any non-init USER namespace? I
>>>>> can't see it being denied otherwise assuming it is only trying to send
>>>>> AUDIT_USER_* class messages. (This assumes upstream kernel.)
>>>> No, I initially saw this on my laptop and then tested on F23 in kvm.
>>> I tested this on Fedora 22 and did not get a USER_AVC from dbus, but I
>>> also
>>> did not get an error message in syslog. So, I don't know what to make of
>>> it. (And for the record, I have a bz open saying that USER_AVC is the
>>> wrong event type. They are blaming libselinux but I blame them for not
>>> using
>>> AUDIT_USER_MAC_POLICY_LOAD.)
>> The audit code in dbus has been refactored a bit in the version present
>> F23 and debian unstable, so it might be related to this that.
>
> I filed a bz to get this fixed:
> https://bugzilla.redhat.com/show_bug.cgi?id=1278602
>
> The root cause is listed in the bug. Dbus has 2 threads, one with
> CAP_AUDIT_WRITE and one without. The one without is the one trying to send the
> event.
Thanks,
I've opened a bug upstream too:
https://bugs.freedesktop.org/show_bug.cgi?id=92832
^ permalink raw reply
* Re: SELinux policy reload cannot be sent to audit system
From: Steve Grubb @ 2015-11-05 23:03 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <563B1409.3030803@debian.org>
On Thursday, November 05, 2015 09:32:09 AM Laurent Bigonville wrote:
> Le 05/11/15 04:23, Steve Grubb a écrit :
> > On Tuesday, November 03, 2015 09:48:31 PM Laurent Bigonville wrote:
> >> Le 03/11/15 21:08, Richard Guy Briggs a écrit :
> >>> On 15/11/03, Steve Grubb wrote:
> >>>> On Tuesday, November 03, 2015 06:12:07 PM Laurent Bigonville wrote:
> >>>>> I'm running in permissive mode.
> >>>>>
> >>>>> I'm seeing a netlink open to the audit:
> >>>>>
> >>>>> dbus-daem 1057 messagebus 7u netlink 0t0 15248 AUDIT
> >>>>>
> >>>>> Apparently audit_send() returns -1
> >>>>
> >>>> Since its -1, that would be an EPERM. No idea where this is coming from
> >>>> if you have CAP_AUDIT_WRITE. I use pscap to check that.
> >>>
> >>> Are you in a container of any kind or any non-init USER namespace? I
> >>> can't see it being denied otherwise assuming it is only trying to send
> >>> AUDIT_USER_* class messages. (This assumes upstream kernel.)
> >>
> >> No, I initially saw this on my laptop and then tested on F23 in kvm.
> >
> > I tested this on Fedora 22 and did not get a USER_AVC from dbus, but I
> > also
> > did not get an error message in syslog. So, I don't know what to make of
> > it. (And for the record, I have a bz open saying that USER_AVC is the
> > wrong event type. They are blaming libselinux but I blame them for not
> > using
> > AUDIT_USER_MAC_POLICY_LOAD.)
>
> The audit code in dbus has been refactored a bit in the version present
> F23 and debian unstable, so it might be related to this that.
I filed a bz to get this fixed:
https://bugzilla.redhat.com/show_bug.cgi?id=1278602
The root cause is listed in the bug. Dbus has 2 threads, one with
CAP_AUDIT_WRITE and one without. The one without is the one trying to send the
event.
-Steve
^ permalink raw reply
* Re: [RFC PATCH 3/7] audit: allow systemd to use queue reserves
From: Paul Moore @ 2015-11-05 22:38 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit
In-Reply-To: <20151022195159.GD16212@madcap2.tricolour.ca>
On Thursday, October 22, 2015 03:51:59 PM Richard Guy Briggs wrote:
> On 15/10/22, Steve Grubb wrote:
> > On Thursday, October 22, 2015 02:53:16 PM Richard Guy Briggs wrote:
> > > Treat systemd the same way as auditd, allowing it to overrun the queue
> > > to avoid blocking.
> >
> > Do you mind explaining this a little more? I'm having a hard time
> > understanding how systemd is involved.
>
> systemd should only have CAP_AUDIT_READ for the multicast socket and
> otherwise behaves as a user client, sending AUDIT_USER_* messages. It
> starts and stops auditd and we don't want it blocking trying to allocate
> a buffer on the standard queue in audit_log_start() while it is tasked
> with telling auditd to start or stop.
Is this something we are hearing reports about? Starting and stopping auditd
should be rare in normal use, and by rare I mean start it at boot and don't
touch it again ... although I suspect you might update/patch it at some point
if your system is long running.
If this is a common problem we can look at doing something like this, but if
it isn't - and I don't think it is - I'd like to avoid special casing init
(it's even more specialized since we are basically talking about just systemd,
although others could have similar problems).
> > -Steve
> >
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > >
> > > kernel/audit.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index 3917aad..384a1a1 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -1375,7 +1375,7 @@ struct audit_buffer *audit_log_start(struct
> > > audit_context *ctx, gfp_t gfp_mask, return NULL;
> > >
> > > if (gfp_mask & __GFP_WAIT) {
> > >
> > > - if (audit_pid && audit_pid == current->tgid)
> > > + if (current->tgid == 1 || (audit_pid && audit_pid == current-
>tgid))
> > >
> > > gfp_mask &= ~__GFP_WAIT;
> > >
> > > else
> > >
> > > reserve = 0;
>
> - 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
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 1/7] audit: don't needlessly reset valid wait time
From: Paul Moore @ 2015-11-05 15:17 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, linux-kernel, v.rathor
In-Reply-To: <20151105031347.GD12430@madcap2.tricolour.ca>
On Wed, Nov 4, 2015 at 10:13 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 15/11/04, Paul Moore wrote:
>> On Thursday, October 22, 2015 02:53:14 PM Richard Guy Briggs wrote:
>> > After auditd has recovered from an overflowed queue, the first process
>> > that doesn't use reserves to make it through the queue checks should
>> > reset the audit backlog wait time to the configured value. After that,
>> > there is no need to keep resetting it.
>> >
>> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> > ---
>> > kernel/audit.c | 2 +-
>> > 1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/kernel/audit.c b/kernel/audit.c
>> > index a72ad37..daefd81 100644
>> > --- a/kernel/audit.c
>> > +++ b/kernel/audit.c
>> > @@ -1403,7 +1403,7 @@ struct audit_buffer *audit_log_start(struct
>> > audit_context *ctx, gfp_t gfp_mask, return NULL;
>> > }
>> >
>> > - if (!reserve)
>> > + if (!reserve && !audit_backlog_wait_time)
>> > audit_backlog_wait_time = audit_backlog_wait_time_master;
>> >
>> > ab = audit_buffer_alloc(ctx, gfp_mask, type);
>>
>> This looks fine to me, I'm going to add it to audit#next-queue.
>>
>> Also, can you think of a good reason why "audit_backlog_wait_overflow" exists?
>> I'm going to replace it with the simple "audit_backlog_wait_time = 0;" unless
>> you can think of a solid reason not to do so. It seems much more obvious and
>> readable to me.
>
> That goes back to ac4cec44, DWMW, July 2005. Best answer I can come up
> with is that it labels magic values and puts them up front at the top of
> the file.
Yeah, I can see that from git blame, I was hoping for some thread I
may have missed. Oh well, not terribly important.
> I'd suggest instead replacing it with a macro. I don't have
> an significant objection to just assigning zero where you suggest.
If it weren't zero I would agree with you, magic numbers in general
are a bit scary. However, in this particular case I don't consider
zero to be a magic number and its use seems pretty clear given the
context.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: SELinux policy reload cannot be sent to audit system
From: Steve Grubb @ 2015-11-05 13:20 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <563B20B9.6050209@debian.org>
On Thursday, November 05, 2015 10:26:17 AM Laurent Bigonville wrote:
> Le 05/11/15 09:32, Laurent Bigonville a écrit :
> > Le 05/11/15 04:23, Steve Grubb a écrit :
> >> I tested this on Fedora 22 and did not get a USER_AVC from dbus, but
> >> I also did not get an error message in syslog. So, I don't know what to
> >> make of it.
> >> (And for the record, I have a bz open saying that USER_AVC is the
> >> wrong event type. They are blaming libselinux but I blame them for not
> >> using AUDIT_USER_MAC_POLICY_LOAD.)
> >
> > The audit code in dbus has been refactored a bit in the version
> > present F23 and debian unstable, so it might be related to this that.
> >
> > Do you still have the number of that bz bug?
>
> BTW, systemd is also apparently sending a USER_AVC event when the policy
> is reloaded.
This is bz 1195330.
-Steve
^ permalink raw reply
* Re: SELinux policy reload cannot be sent to audit system
From: Laurent Bigonville @ 2015-11-05 9:26 UTC (permalink / raw)
To: linux-audit
In-Reply-To: <563B1409.3030803@debian.org>
Le 05/11/15 09:32, Laurent Bigonville a écrit :
> Le 05/11/15 04:23, Steve Grubb a écrit :
>> I tested this on Fedora 22 and did not get a USER_AVC from dbus, but
>> I also
>> did not get an error message in syslog. So, I don't know what to make
>> of it.
>> (And for the record, I have a bz open saying that USER_AVC is the
>> wrong event
>> type. They are blaming libselinux but I blame them for not using
>> AUDIT_USER_MAC_POLICY_LOAD.)
> The audit code in dbus has been refactored a bit in the version
> present F23 and debian unstable, so it might be related to this that.
>
> Do you still have the number of that bz bug?
BTW, systemd is also apparently sending a USER_AVC event when the policy
is reloaded.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox