3:42 p.m.
Re-posting patches to clean up auditing for syscall objects.
- stop logging bogus object labels
- handle edge cases for xattr and mqueue calls
- try harder to log only 1 PATH record per object
Rebased to current git and folded memory corruption fix from Al.
3:43 p.m.
New subject: [PATCH 1/4] initialize name osid
Audit contexts can be reused, so initialize a name's osid to the
default in audit_getname(). This ensures we don't log a bogus object
label when no inode data is collected for a name.
Signed-off-by: Amy Griffis <amy.griffis(a)hp.com>
---
kernel/auditsc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 628c7ac..863479c 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1226,6 +1226,7 @@ void __audit_getname(const char *name)
context->names[context->name_count].name_len = AUDIT_NAME_FULL;
context->names[context->name_count].name_put = 1;
context->names[context->name_count].ino = (unsigned long)-1;
+ context->names[context->name_count].osid = 0;
++context->name_count;
if (!context->pwd) {
read_lock(¤t->fs->lock);
--
1.4.4.4
3:43 p.m.
New subject: [PATCH 2/4] audit inode for all xattr syscalls
Collect inode info for the remaining xattr syscalls that operate on
a file descriptor. These don't call a path_lookup variant, so they
aren't covered by the general audit hook.
Signed-off-by: Amy Griffis <amy.griffis(a)hp.com>
---
fs/xattr.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index 3864613..4f21fc9 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -346,11 +346,14 @@ asmlinkage ssize_t
sys_fgetxattr(int fd, char __user *name, void __user *value, size_t size)
{
struct file *f;
+ struct dentry *dentry;
ssize_t error = -EBADF;
f = fget(fd);
if (!f)
return error;
+ dentry = f->f_path.dentry;
+ audit_inode(NULL, dentry->d_inode);
error = getxattr(f->f_path.dentry, name, value, size);
fput(f);
return error;
@@ -418,11 +421,14 @@ asmlinkage ssize_t
sys_flistxattr(int fd, char __user *list, size_t size)
{
struct file *f;
+ struct dentry *dentry;
ssize_t error = -EBADF;
f = fget(fd);
if (!f)
return error;
+ dentry = f->f_path.dentry;
+ audit_inode(NULL, dentry->d_inode);
error = listxattr(f->f_path.dentry, list, size);
fput(f);
return error;
--
1.4.4.4
3:43 p.m.
New subject: [PATCH 3/4] complete message queue auditing
Handle the edge cases for POSIX message queue auditing. Collect inode
info when opening an existing mq, and for send/receive operations.
Remove audit_inode_update() as it has really evolved into the
equivalent of audit_inode().
Signed-off-by: Amy Griffis <amy.griffis(a)hp.com>
---
fs/namei.c | 2 +-
include/linux/audit.h | 7 -------
ipc/mqueue.c | 4 ++++
kernel/auditsc.c | 27 ---------------------------
4 files changed, 5 insertions(+), 35 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index ee60cc4..33af652 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1701,7 +1701,7 @@ do_last:
* It already exists.
*/
mutex_unlock(&dir->d_inode->i_mutex);
- audit_inode_update(path.dentry->d_inode);
+ audit_inode(pathname, path.dentry->d_inode);
error = -EEXIST;
if (flag & O_EXCL)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 773e30d..ab289bd 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -351,7 +351,6 @@ extern void audit_putname(const char *name);
extern void __audit_inode(const char *name, const struct inode *inode);
extern void __audit_inode_child(const char *dname, const struct inode *inode,
const struct inode *parent);
-extern void __audit_inode_update(const struct inode *inode);
static inline int audit_dummy_context(void)
{
void *p = current->audit_context;
@@ -372,10 +371,6 @@ static inline void audit_inode_child(const char *dname,
if (unlikely(!audit_dummy_context()))
__audit_inode_child(dname, inode, parent);
}
-static inline void audit_inode_update(const struct inode *inode) {
- if (unlikely(!audit_dummy_context()))
- __audit_inode_update(inode);
-}
/* Private API (for audit.c only) */
extern unsigned int audit_serial(void);
@@ -457,10 +452,8 @@ extern int audit_n_rules;
#define audit_putname(n) do { ; } while (0)
#define __audit_inode(n,i) do { ; } while (0)
#define __audit_inode_child(d,i,p) do { ; } while (0)
-#define __audit_inode_update(i) do { ; } while (0)
#define audit_inode(n,i) do { ; } while (0)
#define audit_inode_child(d,i,p) do { ; } while (0)
-#define audit_inode_update(i) do { ; } while (0)
#define auditsc_get_stamp(c,t,s) do { BUG(); } while (0)
#define audit_get_loginuid(c) ({ -1; })
#define audit_log_task_context(b) do { ; } while (0)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 554ac36..db8f11d 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -682,6 +682,7 @@ asmlinkage long sys_mq_open(const char __user *u_name, int oflag,
mode_t mode,
if (oflag & O_CREAT) {
if (dentry->d_inode) { /* entry already exists */
+ audit_inode(name, dentry->d_inode);
error = -EEXIST;
if (oflag & O_EXCL)
goto out;
@@ -694,6 +695,7 @@ asmlinkage long sys_mq_open(const char __user *u_name, int oflag,
mode_t mode,
error = -ENOENT;
if (!dentry->d_inode)
goto out;
+ audit_inode(name, dentry->d_inode);
filp = do_open(dentry, oflag);
}
@@ -841,6 +843,7 @@ asmlinkage long sys_mq_timedsend(mqd_t mqdes, const char __user
*u_msg_ptr,
if (unlikely(filp->f_op != &mqueue_file_operations))
goto out_fput;
info = MQUEUE_I(inode);
+ audit_inode(NULL, inode);
if (unlikely(!(filp->f_mode & FMODE_WRITE)))
goto out_fput;
@@ -924,6 +927,7 @@ asmlinkage ssize_t sys_mq_timedreceive(mqd_t mqdes, char __user
*u_msg_ptr,
if (unlikely(filp->f_op != &mqueue_file_operations))
goto out_fput;
info = MQUEUE_I(inode);
+ audit_inode(NULL, inode);
if (unlikely(!(filp->f_mode & FMODE_READ)))
goto out_fput;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 863479c..e241541 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1413,33 +1413,6 @@ update_context:
}
/**
- * audit_inode_update - update inode info for last collected name
- * @inode: inode being audited
- *
- * When open() is called on an existing object with the O_CREAT flag, the inode
- * data audit initially collects is incorrect. This additional hook ensures
- * audit has the inode data for the actual object to be opened.
- */
-void __audit_inode_update(const struct inode *inode)
-{
- struct audit_context *context = current->audit_context;
- int idx;
-
- if (!context->in_syscall || !inode)
- return;
-
- if (context->name_count == 0) {
- context->name_count++;
-#if AUDIT_DEBUG
- context->ino_count++;
-#endif
- }
- idx = context->name_count - 1;
-
- audit_copy_inode(&context->names[idx], inode);
-}
-
-/**
* auditsc_get_stamp - get local copies of audit_context values
* @ctx: audit_context for the task
* @t: timespec to store time recorded in the audit_context
--
1.4.4.4
3:44 p.m.
New subject: [PATCH 4/4] match audit name data
Make more effort to detect previously collected names, so we don't log
multiple PATH records for a single filesystem object. Add
audit_inc_name_count() to reduce duplicate code.
Signed-off-by: Amy Griffis <amy.griffis(a)hp.com>
---
kernel/auditsc.c | 143 +++++++++++++++++++++++++++++++----------------------
1 files changed, 84 insertions(+), 59 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e241541..3cfa38f 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -78,11 +78,6 @@ extern int audit_enabled;
* for saving names from getname(). */
#define AUDIT_NAMES 20
-/* AUDIT_NAMES_RESERVED is the number of slots we reserve in the
- * audit_context from being used for nameless inodes from
- * path_lookup. */
-#define AUDIT_NAMES_RESERVED 7
-
/* Indicates that audit should log the full pathname. */
#define AUDIT_NAME_FULL -1
@@ -1280,6 +1275,28 @@ void audit_putname(const char *name)
#endif
}
+static int audit_inc_name_count(struct audit_context *context,
+ const struct inode *inode)
+{
+ if (context->name_count >= AUDIT_NAMES) {
+ if (inode)
+ printk(KERN_DEBUG "name_count maxed, losing inode data: "
+ "dev=%02x:%02x, inode=%lu",
+ MAJOR(inode->i_sb->s_dev),
+ MINOR(inode->i_sb->s_dev),
+ inode->i_ino);
+
+ else
+ printk(KERN_DEBUG "name_count maxed, losing inode data");
+ return 1;
+ }
+ context->name_count++;
+#if AUDIT_DEBUG
+ context->ino_count++;
+#endif
+ return 0;
+}
+
/* Copy inode data into an audit_names. */
static void audit_copy_inode(struct audit_names *name, const struct inode *inode)
{
@@ -1317,13 +1334,10 @@ void __audit_inode(const char *name, const struct inode *inode)
else {
/* FIXME: how much do we care about inodes that have no
* associated name? */
- if (context->name_count >= AUDIT_NAMES - AUDIT_NAMES_RESERVED)
+ if (audit_inc_name_count(context, inode))
return;
- idx = context->name_count++;
+ idx = context->name_count - 1;
context->names[idx].name = NULL;
-#if AUDIT_DEBUG
- ++context->ino_count;
-#endif
}
audit_copy_inode(&context->names[idx], inode);
}
@@ -1347,69 +1361,80 @@ void __audit_inode_child(const char *dname, const struct inode
*inode,
{
int idx;
struct audit_context *context = current->audit_context;
- const char *found_name = NULL;
+ const char *found_parent = NULL, *found_child = NULL;
int dirlen = 0;
if (!context->in_syscall)
return;
- /* determine matching parent */
if (!dname)
- goto update_context;
- for (idx = 0; idx < context->name_count; idx++)
- if (context->names[idx].ino == parent->i_ino) {
- const char *name = context->names[idx].name;
+ goto add_names;
- if (!name)
- continue;
+ /* parent is more likely, look for it first */
+ for (idx = 0; idx < context->name_count; idx++) {
+ struct audit_names *n = &context->names[idx];
- if (audit_compare_dname_path(dname, name, &dirlen) == 0) {
- context->names[idx].name_len = dirlen;
- found_name = name;
- break;
- }
+ if (!n->name)
+ continue;
+
+ if ((n->ino == parent->i_ino) &&
+ !audit_compare_dname_path(dname, n->name, &dirlen)) {
+ n->name_len = dirlen; /* update parent data in place */
+ found_parent = n->name;
+ goto add_names;
}
+ }
-update_context:
- idx = context->name_count;
- if (context->name_count == AUDIT_NAMES) {
- printk(KERN_DEBUG "name_count maxed and losing %s\n",
- found_name ?: "(null)");
- return;
+ /* no matching parent, look for matching child */
+ for (idx = 0; idx < context->name_count; idx++) {
+ struct audit_names *n = &context->names[idx];
+
+ if (!n->name)
+ continue;
+
+ /* strcmp() is the more likely scenario */
+ if (!strcmp(dname, n->name) ||
+ !audit_compare_dname_path(dname, n->name, &dirlen)) {
+ if (inode)
+ audit_copy_inode(n, inode);
+ else
+ n->ino = (unsigned long)-1;
+ found_child = n->name;
+ goto add_names;
+ }
}
- context->name_count++;
-#if AUDIT_DEBUG
- context->ino_count++;
-#endif
- /* Re-use the name belonging to the slot for a matching parent directory.
- * All names for this context are relinquished in audit_free_names() */
- context->names[idx].name = found_name;
- context->names[idx].name_len = AUDIT_NAME_FULL;
- context->names[idx].name_put = 0; /* don't call __putname() */
-
- if (!inode)
- context->names[idx].ino = (unsigned long)-1;
- else
- audit_copy_inode(&context->names[idx], inode);
-
- /* A parent was not found in audit_names, so copy the inode data for the
- * provided parent. */
- if (!found_name) {
- idx = context->name_count;
- if (context->name_count == AUDIT_NAMES) {
- printk(KERN_DEBUG
- "name_count maxed and losing parent inode data: dev=%02x:%02x, inode=%lu",
- MAJOR(parent->i_sb->s_dev),
- MINOR(parent->i_sb->s_dev),
- parent->i_ino);
+
+add_names:
+ if (!found_parent) {
+ if (audit_inc_name_count(context, parent))
return;
- }
- context->name_count++;
-#if AUDIT_DEBUG
- context->ino_count++;
-#endif
+ idx = context->name_count - 1;
+ context->names[idx].name = NULL;
audit_copy_inode(&context->names[idx], parent);
}
+
+ if (!found_child) {
+ if (audit_inc_name_count(context, inode))
+ return;
+ idx = context->name_count - 1;
+
+ /* Re-use the name belonging to the slot for a matching parent
+ * directory. All names for this context are relinquished in
+ * audit_free_names() */
+ if (found_parent) {
+ context->names[idx].name = found_parent;
+ context->names[idx].name_len = AUDIT_NAME_FULL;
+ /* don't call __putname() */
+ context->names[idx].name_put = 0;
+ } else {
+ context->names[idx].name = NULL;
+ }
+
+ if (inode)
+ audit_copy_inode(&context->names[idx], inode);
+ else
+ context->names[idx].ino = (unsigned long)-1;
+ }
}
/**
--
1.4.4.4
6487
days inactive
6487
days old
linux-audit@lists.linux-audit.osci.io
4 comments
1 participants
participants (1)
-
Amy Griffis