Discussion:
[RFC][PATCH] (#7U4) file system auditing by location and name
(too old to reply)
Timothy R. Chavez
2005-05-06 21:16:02 UTC
Permalink
Hello,

This patch is against 12-rc3-mm3 and incorporates feedback I received on
#7U3 and does not include fixes based on Viro feedback (I'll be working
on this over the weekend).

Thanks for all your patience. Patch appears below change log.

-tim

Notable changes:

audit.h:
Changed struct audit_transport to struct watch_transport
Changed struct audit_data to struct audit_inode_data

auditfs.c:
Added Steve Grubb's watch_to_transport and transport_to_watch helper
functions to auditfs.c as audit_to_transport and audit_to_watch
respectively to ensure consistency

Put length checking prior to audit_to_watch transformation in
audit_receive_watch (as we have that information available to us from
watch_transport)

Changed audit_insert/remove_watch parameters to struct audit_watch's

Altered audit_send_watch to use audit_to_transport transformation

Added likely(!audit_enabled) check to the audit_update_watch hook

Removed string comparison check in audit_update_watch in favor of
pointer comparison, but I've not tested this yet. The string comparison
seemed a bit wasteful, though

Made sure selinux nlmsgtab entries for AUDIT_WATCH_INS/REM/LIST were
correct (thanks David for cleaning up after me in the past ;-))

auditsc.c

Added likely(!audit_enabled) check to the audit_notify_watch hook
Removed context check from the audit_notify_watch hook

---

diff -Nurp linux-2.6.12-rc3-mm3~orig/fs/dcache.c linux-2.6.12-rc3-mm3~audit/fs/dcache.c
--- linux-2.6.12-rc3-mm3~orig/fs/dcache.c 2005-05-06 13:05:08.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/fs/dcache.c 2005-05-06 14:29:04.000000000 -0500
@@ -32,6 +32,7 @@
#include <linux/seqlock.h>
#include <linux/swap.h>
#include <linux/bootmem.h>
+#include <linux/audit.h>

/* #define DCACHE_DEBUG 1 */

@@ -97,6 +98,7 @@ static inline void dentry_iput(struct de
{
struct inode *inode = dentry->d_inode;
if (inode) {
+ audit_update_watch(dentry, 1);
dentry->d_inode = NULL;
list_del_init(&dentry->d_alias);
spin_unlock(&dentry->d_lock);
@@ -802,6 +804,7 @@ void d_instantiate(struct dentry *entry,
if (inode)
list_add(&entry->d_alias, &inode->i_dentry);
entry->d_inode = inode;
+ audit_update_watch(entry, 0);
spin_unlock(&dcache_lock);
security_d_instantiate(entry, inode);
}
@@ -978,6 +981,7 @@ struct dentry *d_splice_alias(struct ino
new = __d_find_alias(inode, 1);
if (new) {
BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
+ audit_update_watch(new, 0);
spin_unlock(&dcache_lock);
security_d_instantiate(new, inode);
d_rehash(dentry);
@@ -987,6 +991,7 @@ struct dentry *d_splice_alias(struct ino
/* d_instantiate takes dcache_lock, so we do it by hand */
list_add(&dentry->d_alias, &inode->i_dentry);
dentry->d_inode = inode;
+ audit_update_watch(dentry, 0);
spin_unlock(&dcache_lock);
security_d_instantiate(dentry, inode);
d_rehash(dentry);
@@ -1090,6 +1095,7 @@ struct dentry * __d_lookup(struct dentry
if (!d_unhashed(dentry)) {
atomic_inc(&dentry->d_count);
found = dentry;
+ audit_update_watch(found, 0);
}
spin_unlock(&dentry->d_lock);
break;
@@ -1299,6 +1305,8 @@ void d_move(struct dentry * dentry, stru
spin_lock(&target->d_lock);
}

+ audit_update_watch(dentry, 1);
+
/* Move the dentry to the target hash queue, if on different bucket */
if (dentry->d_flags & DCACHE_UNHASHED)
goto already_unhashed;
@@ -1332,6 +1340,7 @@ already_unhashed:
list_add(&target->d_child, &target->d_parent->d_subdirs);
}

+ audit_update_watch(dentry, 0);
list_add(&dentry->d_child, &dentry->d_parent->d_subdirs);
spin_unlock(&target->d_lock);
spin_unlock(&dentry->d_lock);
diff -Nurp linux-2.6.12-rc3-mm3~orig/fs/inode.c linux-2.6.12-rc3-mm3~audit/fs/inode.c
--- linux-2.6.12-rc3-mm3~orig/fs/inode.c 2005-05-06 13:05:08.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/fs/inode.c 2005-05-06 14:29:04.000000000 -0500
@@ -22,6 +22,7 @@
#include <linux/cdev.h>
#include <linux/bootmem.h>
#include <linux/inotify.h>
+#include <linux/audit.h>

/*
* This is needed for the following functions:
@@ -139,9 +140,11 @@ static struct inode *alloc_inode(struct
inode->i_bdev = NULL;
inode->i_cdev = NULL;
inode->i_rdev = 0;
+ inode->i_audit = NULL;
inode->i_security = NULL;
inode->dirtied_when = 0;
- if (security_inode_alloc(inode)) {
+ if (audit_inode_alloc(inode) || security_inode_alloc(inode)) {
+ audit_inode_free(inode);
if (inode->i_sb->s_op->destroy_inode)
inode->i_sb->s_op->destroy_inode(inode);
else
@@ -179,6 +182,7 @@ void destroy_inode(struct inode *inode)
{
if (inode_has_buffers(inode))
BUG();
+ audit_inode_free(inode);
security_inode_free(inode);
if (inode->i_sb->s_op->destroy_inode)
inode->i_sb->s_op->destroy_inode(inode);
diff -Nurp linux-2.6.12-rc3-mm3~orig/fs/namei.c linux-2.6.12-rc3-mm3~audit/fs/namei.c
--- linux-2.6.12-rc3-mm3~orig/fs/namei.c 2005-05-06 13:05:09.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/fs/namei.c 2005-05-06 14:29:04.000000000 -0500
@@ -225,6 +225,8 @@ int permission(struct inode *inode, int
{
int retval, submask;

+ audit_notify_watch(inode, mask);
+
if (mask & MAY_WRITE) {
umode_t mode = inode->i_mode;

@@ -358,6 +360,8 @@ static inline int exec_permission_lite(s
if (inode->i_op && inode->i_op->permission)
return -EAGAIN;

+ audit_notify_watch(inode, MAY_EXEC);
+
if (current->fsuid == inode->i_uid)
mode >>= 6;
else if (in_group_p(inode->i_gid))
@@ -1172,6 +1176,8 @@ static inline int may_delete(struct inod

BUG_ON(victim->d_parent->d_inode != dir);

+ audit_notify_watch(victim->d_inode, MAY_WRITE);
+
error = permission(dir,MAY_WRITE | MAY_EXEC, NULL);
if (error)
return error;
@@ -1296,6 +1302,7 @@ int vfs_create(struct inode *dir, struct
DQUOT_INIT(dir);
error = dir->i_op->create(dir, dentry, mode, nd);
if (!error) {
+ audit_notify_watch(dentry->d_inode, MAY_WRITE);
fsnotify_create(dir, dentry->d_name.name);
security_inode_post_create(dir, dentry, mode);
}
@@ -1601,6 +1608,7 @@ int vfs_mknod(struct inode *dir, struct
DQUOT_INIT(dir);
error = dir->i_op->mknod(dir, dentry, mode, dev);
if (!error) {
+ audit_notify_watch(dentry->d_inode, MAY_WRITE);
fsnotify_create(dir, dentry->d_name.name);
security_inode_post_mknod(dir, dentry, mode, dev);
}
@@ -1674,6 +1682,7 @@ int vfs_mkdir(struct inode *dir, struct
DQUOT_INIT(dir);
error = dir->i_op->mkdir(dir, dentry, mode);
if (!error) {
+ audit_notify_watch(dentry->d_inode, MAY_WRITE);
fsnotify_mkdir(dir, dentry->d_name.name);
security_inode_post_mkdir(dir,dentry, mode);
}
@@ -1915,6 +1924,7 @@ int vfs_symlink(struct inode *dir, struc
DQUOT_INIT(dir);
error = dir->i_op->symlink(dir, dentry, oldname);
if (!error) {
+ audit_notify_watch(dentry->d_inode, MAY_WRITE);
fsnotify_create(dir, dentry->d_name.name);
security_inode_post_symlink(dir, dentry, oldname);
}
@@ -1988,6 +1998,7 @@ int vfs_link(struct dentry *old_dentry,
error = dir->i_op->link(old_dentry, dir, new_dentry);
up(&old_dentry->d_inode->i_sem);
if (!error) {
+ audit_notify_watch(new_dentry->d_inode, MAY_WRITE);
fsnotify_create(dir, new_dentry->d_name.name);
security_inode_post_link(old_dentry, dir, new_dentry);
}
@@ -2111,6 +2122,7 @@ static int vfs_rename_dir(struct inode *
}
if (!error) {
d_move(old_dentry,new_dentry);
+ audit_notify_watch(old_dentry->d_inode, MAY_WRITE);
security_inode_post_rename(old_dir, old_dentry,
new_dir, new_dentry);
}
@@ -2139,6 +2151,7 @@ static int vfs_rename_other(struct inode
/* The following d_move() should become unconditional */
if (!(old_dir->i_sb->s_type->fs_flags & FS_ODD_RENAME))
d_move(old_dentry, new_dentry);
+ audit_notify_watch(old_dentry->d_inode, MAY_WRITE);
security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry);
}
if (target)
diff -Nurp linux-2.6.12-rc3-mm3~orig/include/linux/audit.h linux-2.6.12-rc3-mm3~audit/include/linux/audit.h
--- linux-2.6.12-rc3-mm3~orig/include/linux/audit.h 2005-05-06 13:05:14.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/include/linux/audit.h 2005-05-06 14:29:04.000000000 -0500
@@ -24,18 +24,29 @@
#ifndef _LINUX_AUDIT_H_
#define _LINUX_AUDIT_H_

+#ifdef __KERNEL__
#include <linux/sched.h>
#include <linux/elf.h>

+
+struct hlist_head;
+struct hlist_node;
+struct spinlock_t;
+struct atomic_t;
+#endif
+
/* Request and reply types */
-#define AUDIT_GET 1000 /* Get status */
-#define AUDIT_SET 1001 /* Set status (enable/disable/auditd) */
-#define AUDIT_LIST 1002 /* List filtering rules */
-#define AUDIT_ADD 1003 /* Add filtering rule */
-#define AUDIT_DEL 1004 /* Delete filtering rule */
-#define AUDIT_USER 1005 /* Send a message from user-space */
-#define AUDIT_LOGIN 1006 /* Define the login id and informaiton */
-#define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */
+#define AUDIT_GET 1000 /* Get status */
+#define AUDIT_SET 1001 /* Set status (enable/disable/auditd) */
+#define AUDIT_LIST 1002 /* List filtering rules */
+#define AUDIT_ADD 1003 /* Add filtering rule */
+#define AUDIT_DEL 1004 /* Delete filtering rule */
+#define AUDIT_USER 1005 /* Send a message from user-space */
+#define AUDIT_LOGIN 1006 /* Define the login id and informaiton */
+#define AUDIT_WATCH_INS 1007 /* Insert file/dir watch entry */
+#define AUDIT_WATCH_REM 1008 /* Remove file/dir watch entry */
+#define AUDIT_WATCH_LIST 1009 /* List all watches */
+#define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */

/* Rule flags */
#define AUDIT_PER_TASK 0x01 /* Apply rule at task creation (not syscall) */
@@ -132,6 +143,9 @@
#define AUDIT_ARCH_V850 (EM_V850|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_X86_64 (EM_X86_64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)

+/* 32 byte max key size */
+#define AUDIT_FILTERKEY_MAX 32
+
#ifndef __KERNEL__
struct audit_message {
struct nlmsghdr nlh;
@@ -159,8 +173,44 @@ struct audit_rule { /* for AUDIT_LIST,
__u32 values[AUDIT_MAX_FIELDS];
};

+/* Structure to transport watch data to and from the kernel */
+
+struct watch_transport {
+ __u32 dev_major;
+ __u32 dev_minor;
+ __u32 perms;
+ __u32 valid;
+ __u32 pathlen;
+ __u32 fklen;
+ char buf[0];
+};
+
#ifdef __KERNEL__

+/* Structure associated with inode->i_audit */
+
+struct audit_inode_data {
+ struct audit_wentry *wentry;
+ struct hlist_head watchlist;
+ rwlock_t lock;
+};
+
+struct audit_watch {
+ dev_t dev; /* Superblock device */
+ __u32 perms; /* Permissions filtering */
+ char *name; /* Watch point beneath parent */
+ char *path; /* Insertion path */
+ char *filterkey; /* An arbitrary filtering key */
+};
+
+struct audit_wentry {
+ struct hlist_node w_node;
+ struct hlist_node w_master;
+ struct audit_watch *w_watch;
+ atomic_t w_count;
+
+};
+
struct audit_buffer;
struct audit_context;
struct inode;
@@ -190,6 +240,7 @@ extern void audit_get_stamp(struct audit
extern int audit_set_loginuid(struct task_struct *task, uid_t loginuid);
extern uid_t audit_get_loginuid(struct audit_context *ctx);
extern int audit_ipc_perms(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode);
+extern int audit_notify_watch(struct inode *inode, int mask);
#else
#define audit_alloc(t) ({ 0; })
#define audit_free(t) do { ; } while (0)
@@ -200,6 +251,28 @@ extern int audit_ipc_perms(unsigned long
#define audit_inode(n,i) do { ; } while (0)
#define audit_get_loginuid(c) ({ -1; })
#define audit_ipc_perms(q,u,g,m) ({ 0; })
+#define audit_notify_watch(i,m) ({ 0; })
+#endif
+
+#ifdef CONFIG_AUDITFILESYSTEM
+extern int audit_list_watches(int pid, int seq);
+extern int audit_receive_watch(int type, int pid, int uid, int seq,
+ struct watch_transport *req);
+extern int audit_filesystem_init(void);
+extern int audit_inode_alloc(struct inode *inode);
+extern void audit_inode_free(struct inode *inode);
+extern void audit_update_watch(struct dentry *dentry, int remove);
+extern void audit_wentry_put(struct audit_wentry *wentry);
+extern struct audit_wentry *audit_wentry_get(struct audit_wentry *wentry);
+#else
+#define audit_list_watches(p,s) ({ 0; })
+#define audit_receive_watch(t,p,u,s,r) ({ -EOPNOTSUPP; })
+#define audit_filesystem_init() ({ 0; })
+#define audit_inode_alloc(i) ({ 0; })
+#define audit_inode_free(i) do { ; } while(0)
+#define audit_update_watch(d,r) do { ; } while (0)
+#define audit_wentry_put(w) do { ; } while(0)
+#define audit_wentry_get(w) ({ 0; })
#endif

#ifdef CONFIG_AUDIT
diff -Nurp linux-2.6.12-rc3-mm3~orig/include/linux/fs.h linux-2.6.12-rc3-mm3~audit/include/linux/fs.h
--- linux-2.6.12-rc3-mm3~orig/include/linux/fs.h 2005-05-06 13:05:14.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/include/linux/fs.h 2005-05-06 14:29:04.000000000 -0500
@@ -226,6 +226,7 @@ struct poll_table_struct;
struct kstatfs;
struct vm_area_struct;
struct vfsmount;
+struct audit_inode_data;

/* Used to be a macro which just called the function, now just a function */
extern void update_atime (struct inode *);
@@ -477,6 +478,7 @@ struct inode {
struct semaphore inotify_sem; /* protects the watches list */
#endif

+ struct audit_inode_data *i_audit;
unsigned long i_state;
unsigned long dirtied_when; /* jiffies of first dirtying */

diff -Nurp linux-2.6.12-rc3-mm3~orig/init/Kconfig linux-2.6.12-rc3-mm3~audit/init/Kconfig
--- linux-2.6.12-rc3-mm3~orig/init/Kconfig 2005-05-06 13:05:15.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/init/Kconfig 2005-05-06 14:29:04.000000000 -0500
@@ -180,6 +180,16 @@ config AUDITSYSCALL
can be used independently or with another kernel subsystem,
such as SELinux.

+config AUDITFILESYSTEM
+ bool "Enable file system auditing support"
+ depends on AUDITSYSCALL
+ default n
+ help
+ Enable file system auditing for regular files and directories.
+ When a targeted file or directory is accessed, an audit record
+ is generated describing the inode accessed, how it was accessed,
+ and by whom (ie: pid and system call).
+
config HOTPLUG
bool "Support for hot-pluggable devices" if !ARCH_S390
default ARCH_S390
diff -Nurp linux-2.6.12-rc3-mm3~orig/kernel/Makefile linux-2.6.12-rc3-mm3~audit/kernel/Makefile
--- linux-2.6.12-rc3-mm3~orig/kernel/Makefile 2005-05-06 13:05:15.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/kernel/Makefile 2005-05-06 14:29:04.000000000 -0500
@@ -25,6 +25,7 @@ obj-$(CONFIG_IKCONFIG_PROC) += configs.o
obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
obj-$(CONFIG_AUDIT) += audit.o
obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
+obj-$(CONFIG_AUDITFILESYSTEM) += auditfs.o
obj-$(CONFIG_KPROBES) += kprobes.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-$(CONFIG_DETECT_SOFTLOCKUP) += softlockup.o
diff -Nurp linux-2.6.12-rc3-mm3~orig/kernel/audit.c linux-2.6.12-rc3-mm3~audit/kernel/audit.c
--- linux-2.6.12-rc3-mm3~orig/kernel/audit.c 2005-05-06 13:05:15.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/kernel/audit.c 2005-05-06 14:29:04.000000000 -0500
@@ -321,6 +321,9 @@ static int audit_netlink_ok(kernel_cap_t
case AUDIT_SET:
case AUDIT_ADD:
case AUDIT_DEL:
+ case AUDIT_WATCH_LIST:
+ case AUDIT_WATCH_INS:
+ case AUDIT_WATCH_REM:
if (!cap_raised(eff_cap, CAP_AUDIT_CONTROL))
err = -EPERM;
break;
@@ -419,6 +422,17 @@ static int audit_receive_msg(struct sk_b
err = -EOPNOTSUPP;
#endif
break;
+ case AUDIT_WATCH_LIST:
+ err = audit_list_watches(pid, seq);
+ break;
+ case AUDIT_WATCH_INS:
+ case AUDIT_WATCH_REM:
+ if (nlh->nlmsg_len < sizeof(struct watch_transport))
+ return -EINVAL;
+ err = audit_receive_watch(nlh->nlmsg_type,
+ NETLINK_CB(skb).pid,
+ uid, seq, data);
+ break;
default:
err = -EINVAL;
break;
@@ -560,6 +574,7 @@ static int __init audit_init(void)

audit_initialized = 1;
audit_enabled = audit_default;
+ audit_filesystem_init();
audit_log(NULL, "initialized");
return 0;
}
diff -Nurp linux-2.6.12-rc3-mm3~orig/kernel/auditfs.c linux-2.6.12-rc3-mm3~audit/kernel/auditfs.c
--- linux-2.6.12-rc3-mm3~orig/kernel/auditfs.c 1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc3-mm3~audit/kernel/auditfs.c 2005-05-06 14:29:04.000000000 -0500
@@ -0,0 +1,608 @@
+/* auditfs.c -- Filesystem auditing support -*- linux-c -*-
+ * Implements filesystem auditing support, depends on kernel/auditsc.c
+ *
+ * Copyright 2005 International Business Machines Corp. (IBM)
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ * 02111-1307 USA
+ *
+ * Written by Timothy R. Chavez <***@us.ibm.com>
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/fs.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/namei.h>
+#include <linux/mount.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/audit.h>
+#include <asm/uaccess.h>
+
+extern int audit_enabled;
+
+kmem_cache_t *audit_watch_cache;
+kmem_cache_t *audit_wentry_cache;
+
+DEFINE_SPINLOCK(audit_master_watchlist_lock);
+HLIST_HEAD(audit_master_watchlist);
+
+
+/* Private Interface */
+
+static inline struct audit_wentry *audit_wentry_fetch(const char *name,
+ struct audit_inode_data *data)
+{
+ struct audit_wentry *wentry, *ret = NULL;
+ struct hlist_node *pos;
+
+ hlist_for_each_entry(wentry, pos, &data->watchlist, w_node)
+ if(!strcmp(wentry->w_watch->name, name)) {
+ ret = audit_wentry_get(wentry);
+ break;
+ }
+
+ return ret;
+}
+
+static inline struct audit_wentry *audit_wentry_fetch_lock(const char *name,
+ struct audit_inode_data *data)
+{
+ struct audit_wentry *ret = NULL;
+
+ if (name && data) {
+ read_lock(&data->lock);
+ ret = audit_wentry_fetch(name, data);
+ read_unlock(&data->lock);
+ }
+
+ return ret;
+}
+
+static inline struct audit_watch *audit_watch_alloc(void)
+{
+ struct audit_watch *watch;
+
+ watch = kmem_cache_alloc(audit_watch_cache, GFP_KERNEL);
+ if (watch) {
+ watch->name = NULL;
+ watch->filterkey = NULL;
+ watch->perms = 0;
+ }
+
+ return watch;
+}
+
+static inline void audit_watch_free(struct audit_watch *watch)
+{
+ if (watch) {
+ kfree(watch->name);
+ kfree(watch->path);
+ kfree(watch->filterkey);
+ kmem_cache_free(audit_watch_cache, watch);
+ }
+}
+
+struct audit_watch *audit_to_watch(void *memblk)
+{
+ unsigned int offset;
+ struct watch_transport *t;
+ struct audit_watch *watch = NULL;
+
+ watch = audit_watch_alloc();
+ if (!watch)
+ goto audit_to_watch_exit;
+
+ t = (struct watch_transport *)memblk;
+
+ watch->perms = t->perms;
+
+ offset = sizeof(struct watch_transport);
+ watch->filterkey = kmalloc(t->fklen, GFP_KERNEL);
+ if (!watch->filterkey)
+ goto audit_to_watch_fail;
+ strncpy(watch->filterkey, memblk + offset, t->fklen);
+
+ offset += t->fklen;
+ watch->path = kmalloc(t->pathlen, GFP_KERNEL);
+ if (!watch->path)
+ goto audit_to_watch_fail;
+ strncpy(watch->path, memblk + offset, t->pathlen);
+
+ goto audit_to_watch_exit;
+
+audit_to_watch_fail:
+ audit_watch_free(watch);
+audit_to_watch_exit:
+ return watch;
+}
+
+
+static void *audit_to_transport(struct audit_watch *watch,
+ unsigned int size)
+{
+ int ret;
+ unsigned int offset;
+ struct watch_transport t;
+ struct audit_wentry *wentry;
+ struct audit_inode_data *data;
+ struct nameidata nd;
+ void *memblk;
+
+ memblk = kmalloc(size, GFP_KERNEL);
+ if (!memblk)
+ goto audit_to_transport_exit;
+
+ memset(&t, 0, sizeof(t));
+
+ t.dev_major = MAJOR(watch->dev);
+ t.dev_minor = MINOR(watch->dev);
+ t.perms = watch->perms;
+ t.pathlen = strlen(watch->path) + 1;
+ if (watch->filterkey)
+ t.fklen = strlen(watch->filterkey) + 1;
+
+ ret = path_lookup(watch->path, LOOKUP_PARENT, &nd);
+ if (!ret) {
+ data = nd.dentry->d_inode->i_audit;
+ wentry = audit_wentry_fetch_lock(watch->name, data);
+ if (wentry && nd.dentry->d_inode->i_sb->s_dev == watch->dev)
+ t.valid = 1;
+ audit_wentry_put(wentry);
+ path_release(&nd);
+ }
+
+ printk("filterkey=%s\n", watch->filterkey);
+
+ memcpy(memblk, &t, sizeof(t));
+ offset = sizeof(t);
+ memcpy(memblk + offset, watch->filterkey, t.fklen);
+ offset += t.fklen;
+ memcpy(memblk + offset, watch->path, t.pathlen);
+
+audit_to_transport_exit:
+ return memblk;
+}
+
+
+static inline struct audit_watch *audit_create_watch(const char *path,
+ const char *name,
+ const char *filterkey,
+ __u32 perms, dev_t dev)
+{
+ struct audit_watch *err = NULL;
+ struct audit_watch *watch = NULL;
+
+ err = ERR_PTR(-ENOMEM);
+ watch = audit_watch_alloc();
+ if (watch) {
+ watch->path = kmalloc(strlen(path)+1, GFP_KERNEL);
+ if (!watch->path)
+ goto audit_create_watch_fail;
+ strcpy(watch->path, path);
+ watch->name = kmalloc(strlen(name)+1, GFP_KERNEL);
+ if (!watch->name)
+ goto audit_create_watch_fail;
+ strcpy(watch->name, name);
+
+ if (filterkey) {
+ watch->filterkey = kmalloc(strlen(filterkey)+1,
+ GFP_KERNEL);
+ if (!watch->filterkey)
+ goto audit_create_watch_fail;
+ strcpy(watch->filterkey, filterkey);
+ }
+
+ watch->dev = dev;
+ watch->perms = perms;
+
+ goto audit_create_watch_exit;
+ }
+
+
+audit_create_watch_fail:
+ audit_watch_free(watch);
+ watch = err;
+audit_create_watch_exit:
+ return watch;
+}
+
+static inline struct audit_wentry *audit_wentry_alloc(void)
+{
+ struct audit_wentry *wentry;
+
+ wentry = kmem_cache_alloc(audit_wentry_cache, GFP_KERNEL);
+ if (wentry) {
+ atomic_set(&wentry->w_count, 1);
+ wentry->w_watch = NULL;
+ }
+
+ return wentry;
+}
+
+static inline void audit_wentry_free(struct audit_wentry *wentry)
+{
+ if (wentry) {
+ audit_watch_free(wentry->w_watch);
+ kmem_cache_free(audit_wentry_cache, wentry);
+ }
+}
+
+/* The only time the new wentry gets updated is when it is inaccessible
+ * (out of the list).
+ */
+static inline int audit_create_wentry(const char *path,
+ const char *name,
+ const char *filterkey,
+ __u32 perms, dev_t dev,
+ struct audit_inode_data *data)
+{
+ int ret;
+ struct audit_wentry *wentry = NULL;
+ struct audit_wentry *new = NULL;
+
+ ret = -ENOMEM;
+ new = audit_wentry_alloc();
+ if (!new)
+ goto audit_create_wentry_fail;
+
+ new->w_watch = audit_create_watch(path, name, filterkey, perms, dev);
+ if (IS_ERR(new->w_watch)) {
+ ret = PTR_ERR(new->w_watch);
+ new->w_watch = NULL;
+ goto audit_create_wentry_fail;
+ }
+
+ ret = 0;
+ write_lock(&data->lock);
+ wentry = audit_wentry_fetch(name, data);
+ if (!wentry) {
+ spin_lock(&audit_master_watchlist_lock);
+ new = audit_wentry_get(new);
+ hlist_add_head(&new->w_master, &audit_master_watchlist);
+ spin_unlock(&audit_master_watchlist_lock);
+ hlist_add_head(&new->w_node, &data->watchlist);
+ write_unlock(&data->lock);
+ goto audit_create_wentry_exit;
+ }
+ audit_wentry_put(wentry);
+ write_unlock(&data->lock);
+
+ ret = -EEXIST;
+
+audit_create_wentry_fail:
+ audit_wentry_put(new);
+audit_create_wentry_exit:
+ return ret;
+}
+
+/* Caller must hold watchlist_lock */
+
+static inline void audit_destroy_wentry(struct audit_wentry *wentry)
+{
+ if (wentry) {
+ spin_lock(&audit_master_watchlist_lock);
+ hlist_del_init(&wentry->w_master);
+ audit_wentry_put(wentry);
+ spin_unlock(&audit_master_watchlist_lock);
+ hlist_del_init(&wentry->w_node);
+ audit_wentry_put(wentry);
+ }
+}
+
+
+/* Caller must hold data->lock */
+
+static inline void audit_drain_watchlist(struct audit_inode_data *data)
+{
+ struct audit_wentry *wentry;
+ struct hlist_node *pos, *buf;
+
+ hlist_for_each_entry_safe(wentry, pos, buf, &data->watchlist, w_node)
+ audit_destroy_wentry(wentry);
+}
+
+static inline struct audit_inode_data *audit_data_alloc(void)
+{
+ struct audit_inode_data *data;
+
+ data = kmalloc(sizeof(struct audit_inode_data), GFP_KERNEL);
+ if (data) {
+ data->wentry = NULL;
+ INIT_HLIST_HEAD(&data->watchlist);
+ data->lock = RW_LOCK_UNLOCKED;
+ }
+
+ return data;
+}
+
+static inline void audit_data_free(struct audit_inode_data *data)
+{
+ if (data) {
+ write_lock(&data->lock);
+ audit_drain_watchlist(data);
+ audit_wentry_put(data->wentry);
+ write_unlock(&data->lock);
+ kfree(data);
+ }
+}
+
+static inline int audit_insert_watch(struct audit_watch *watch)
+{
+ int ret;
+ struct nameidata nd;
+
+ ret = path_lookup(watch->path, LOOKUP_PARENT, &nd);
+ if (ret < 0)
+ goto audit_insert_watch_exit;
+
+ ret = -EPERM;
+ if (nd.last_type != LAST_NORM || !nd.last.name)
+ goto audit_insert_watch_release;
+
+ ret = audit_create_wentry(watch->path,
+ nd.last.name,
+ watch->filterkey,
+ watch->perms,
+ nd.dentry->d_inode->i_sb->s_dev,
+ nd.dentry->d_inode->i_audit);
+
+ /* __d_lookup will attach the audit data, if nd.last exists. */
+ dput(d_lookup(nd.dentry, &nd.last));
+
+audit_insert_watch_release:
+ path_release(&nd);
+audit_insert_watch_exit:
+ return ret;
+}
+
+static inline int audit_remove_watch(struct audit_watch *watch)
+{
+ int ret;
+ struct nameidata nd;
+ struct audit_inode_data *data;
+ struct audit_wentry *wentry;
+
+ ret = path_lookup(watch->path, LOOKUP_PARENT, &nd);
+ if (ret < 0)
+ goto audit_remove_watch_exit;
+
+ ret = -EPERM;
+ if (nd.last_type != LAST_NORM || !nd.last.name)
+ goto audit_remove_watch_release;
+
+ data = nd.dentry->d_inode->i_audit;
+
+ write_lock(&data->lock);
+ wentry = audit_wentry_fetch(nd.last.name, data);
+ if (!wentry) {
+ write_unlock(&data->lock);
+ goto audit_remove_watch_release;
+ }
+ audit_destroy_wentry(wentry);
+ audit_wentry_put(wentry);
+ write_unlock(&data->lock);
+
+ ret = 0;
+
+audit_remove_watch_release:
+ path_release(&nd);
+audit_remove_watch_exit:
+ return ret;
+}
+
+struct audit_wentry *audit_wentry_get(struct audit_wentry *wentry)
+{
+ if (wentry) {
+ BUG_ON(!atomic_read(&wentry->w_count));
+ atomic_inc(&wentry->w_count);
+ }
+
+ return wentry;
+}
+
+void audit_wentry_put(struct audit_wentry *wentry)
+{
+ if (wentry && atomic_dec_and_test(&wentry->w_count))
+ audit_wentry_free(wentry);
+}
+
+/*
+ * The update hook is responsible for watching and unwatching d_inodes during
+ * their lifetimes in dcache. Each d_inode being watched is pinned in memory.
+ * As soon as a d_inode becomes unwatched (when !d_inode->i_audit->wentry), it
+ * is unpinned.
+ *
+ * Hook appears in fs/dcache.c:
+ * d_move(),
+ * d_delete(),
+ * d_instantiate(),
+ * d_splice_alias()
+ * __d_lookup()
+ */
+void audit_update_watch(struct dentry *dentry, int remove)
+{
+ struct audit_wentry *wentry;
+ struct audit_inode_data *data, *parent;
+
+ if (likely(!audit_enabled))
+ return;
+
+ if (!dentry || !dentry->d_inode)
+ return;
+
+ if (!dentry->d_parent || !dentry->d_parent->d_inode)
+ return;
+
+ data = dentry->d_inode->i_audit;
+ parent = dentry->d_parent->d_inode->i_audit;
+
+ wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent);
+
+ write_lock(&data->lock);
+ /* FIXME: long watchlist == too much spinning? */
+ if (remove) {
+ audit_drain_watchlist(data);
+ if (wentry && data->wentry) {
+ /* UNTESTED */
+ if (wentry == data->wentry) {
+ /*
+ if (!strcmp(wentry->w_watch->name,
+ data->wentry->w_watch->name)) {
+ */
+ dput(dentry);
+ audit_wentry_put(data->wentry);
+ data->wentry = NULL;
+ }
+ }
+ } else if (!data->wentry) {
+ data->wentry = audit_wentry_get(wentry);
+ if (data->wentry)
+ dget(dentry);
+ } else if (hlist_unhashed(&data->wentry->w_node)) {
+ dput(dentry);
+ audit_wentry_put(data->wentry);
+ data->wentry = audit_wentry_get(wentry);
+ if (data->wentry)
+ dget(dentry);
+ }
+
+ audit_wentry_put(wentry);
+ write_unlock(&data->lock);
+}
+
+int audit_send_watch(int pid, int seq, struct audit_watch *watch)
+{
+ int ret = 0;
+ size_t size;
+ void *memblk;
+
+ size = sizeof(struct watch_transport) +
+ strlen(watch->path) + strlen(watch->filterkey) + 2;
+
+ memblk = audit_to_transport(watch, size);
+ if (!memblk)
+ return -ENOMEM;
+
+ audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 0, 1,
+ memblk, size);
+ kfree(memblk);
+
+ return ret;
+}
+
+int audit_list_watches(int pid, int seq)
+{
+ int ret = 0;
+ struct audit_wentry *wentry;
+ struct hlist_node *pos;
+
+ spin_lock(&audit_master_watchlist_lock);
+ hlist_for_each_entry(wentry, pos, &audit_master_watchlist, w_master) {
+ ret = audit_send_watch(pid, seq, wentry->w_watch);
+ if (ret < 0)
+ break;
+ }
+ spin_unlock(&audit_master_watchlist_lock);
+
+ audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 1, 1,
+ NULL, 0);
+
+ return ret;
+}
+
+int audit_receive_watch(int type, int pid, int uid, int seq,
+ struct watch_transport *req)
+{
+ int ret = 0;
+ struct audit_watch *watch = NULL;
+
+ ret = -EINVAL;
+ if (req->pathlen > PATH_MAX)
+ goto audit_receive_watch_exit;
+
+ if (req->fklen > AUDIT_FILTERKEY_MAX)
+ goto audit_receive_watch_exit;
+
+ if (req->perms > (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND))
+ goto audit_receive_watch_exit;
+
+ ret = -ENOMEM;
+ watch = audit_to_watch(req);
+ if (!watch)
+ goto audit_receive_watch_exit;
+
+ switch (type) {
+ case AUDIT_WATCH_INS:
+ ret = audit_insert_watch(watch);
+ break;
+ case AUDIT_WATCH_REM:
+ ret = audit_remove_watch(watch);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+audit_receive_watch_exit:
+ audit_watch_free(watch);
+ return ret;
+}
+
+int audit_inode_alloc(struct inode *inode)
+{
+ if (inode) {
+ inode->i_audit = audit_data_alloc();
+ if (!inode->i_audit)
+ return ENOMEM;
+ }
+
+ return 0;
+}
+
+void audit_inode_free(struct inode *inode)
+{
+ if (inode)
+ audit_data_free(inode->i_audit);
+}
+
+int audit_filesystem_init()
+{
+ int ret = 0;
+
+ audit_watch_cache =
+ kmem_cache_create("audit_watch_cache",
+ sizeof(struct audit_watch), 0, 0, NULL, NULL);
+ if (!audit_watch_cache)
+ goto audit_filesystem_init_fail;
+
+ audit_wentry_cache =
+ kmem_cache_create("audit_wentry_cache",
+ sizeof(struct audit_wentry), 0, 0, NULL, NULL);
+ if (!audit_wentry_cache)
+ goto audit_filesystem_init_fail;
+
+ goto audit_filesystem_init_exit;
+
+audit_filesystem_init_fail:
+ ret = -ENOMEM;
+ kmem_cache_destroy(audit_watch_cache);
+ kmem_cache_destroy(audit_wentry_cache);
+audit_filesystem_init_exit:
+ return ret;
+
+}
diff -Nurp linux-2.6.12-rc3-mm3~orig/kernel/auditsc.c linux-2.6.12-rc3-mm3~audit/kernel/auditsc.c
--- linux-2.6.12-rc3-mm3~orig/kernel/auditsc.c 2005-05-06 13:05:15.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/kernel/auditsc.c 2005-05-06 14:29:04.000000000 -0500
@@ -102,6 +102,7 @@ struct audit_aux_data {
};

#define AUDIT_AUX_IPCPERM 0
+#define AUDIT_AUX_WATCH 1

struct audit_aux_data_ipcctl {
struct audit_aux_data d;
@@ -112,6 +113,16 @@ struct audit_aux_data_ipcctl {
mode_t mode;
};

+struct audit_aux_data_watched {
+ struct audit_aux_data link;
+ struct audit_wentry *wentry;
+ unsigned long ino;
+ int mask;
+ uid_t uid;
+ gid_t gid;
+ dev_t dev;
+ dev_t rdev;
+};

/* The per-task audit context. */
struct audit_context {
@@ -696,6 +707,24 @@ static void audit_log_exit(struct audit_
audit_log_format(ab,
" qbytes=%lx uid=%d gid=%d mode=%x",
axi->qbytes, axi->uid, axi->gid, axi->mode);
+ break;
+ }
+
+ case AUDIT_AUX_WATCH: {
+ struct audit_aux_data_watched *axi = (void *)aux;
+ audit_log_format(ab, " watch=");
+ audit_log_untrustedstring(ab, axi->wentry->w_watch->name);
+ audit_log_format(ab,
+ " filterkey=%s perm=%u perm_mask=%d"
+ " inode=%lu inode_uid=%d inode_gid=%d"
+ " inode_dev=%02x:%02x inode_rdev=%02x:%02x",
+ axi->wentry->w_watch->filterkey,
+ axi->wentry->w_watch->perms,
+ axi->mask, axi->ino, axi->uid, axi->gid,
+ MAJOR(axi->dev), MINOR(axi->dev),
+ MAJOR(axi->rdev), MINOR(axi->rdev));
+ audit_wentry_put(axi->wentry);
+ break;
}
}
audit_log_end(ab);
@@ -1056,3 +1085,52 @@ int audit_ipc_perms(unsigned long qbytes
context->aux = (void *)ax;
return 0;
}
+
+int audit_notify_watch(struct inode *inode, int mask)
+{
+ int ret = 0;
+ struct audit_context *context = current->audit_context;
+ struct audit_aux_data_watched *ax;
+ struct audit_wentry *wentry = NULL;
+
+ if (likely(!audit_enabled))
+ goto audit_notify_watch_exit;
+
+ if (!inode)
+ goto audit_notify_watch_exit;
+
+ wentry = audit_wentry_get(inode->i_audit->wentry);
+ if (!wentry)
+ goto audit_notify_watch_exit;
+
+ if (mask && (wentry->w_watch->perms && !(wentry->w_watch->perms&mask)))
+ goto audit_notify_watch_fail;
+
+ ret = -ENOMEM;
+ ax = kmalloc(sizeof(*ax), GFP_KERNEL);
+ if (!ax)
+ goto audit_notify_watch_fail;
+
+ ret = 0;
+ if (context->in_syscall && !context->auditable)
+ context->auditable = 1;
+
+ ax->wentry = wentry;
+ ax->mask = mask;
+ ax->ino = inode->i_ino;
+ ax->uid = inode->i_uid;
+ ax->gid = inode->i_gid;
+ ax->dev = inode->i_sb->s_dev;
+ ax->rdev = inode->i_rdev;
+
+ ax->link.type = AUDIT_AUX_WATCH;
+ ax->link.next = context->aux;
+ context->aux = (void *)ax;
+
+ goto audit_notify_watch_exit;
+
+audit_notify_watch_fail:
+ audit_wentry_put(wentry);
+audit_notify_watch_exit:
+ return ret;
+}
diff -Nurp linux-2.6.12-rc3-mm3~orig/security/selinux/nlmsgtab.c linux-2.6.12-rc3-mm3~audit/security/selinux/nlmsgtab.c
--- linux-2.6.12-rc3-mm3~orig/security/selinux/nlmsgtab.c 2005-05-06 13:05:16.000000000 -0500
+++ linux-2.6.12-rc3-mm3~audit/security/selinux/nlmsgtab.c 2005-05-06 14:29:04.000000000 -0500
@@ -97,6 +97,9 @@ static struct nlmsg_perm nlmsg_audit_per
{ AUDIT_ADD, NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
{ AUDIT_DEL, NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
{ AUDIT_USER, NETLINK_AUDIT_SOCKET__NLMSG_RELAY },
+ { AUDIT_WATCH_INS, NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
+ { AUDIT_WATCH_REM, NETLINK_AUDIT_SOCKET__NLMSG_WRITE },
+ { AUDIT_WATCH_LIST, NETLINK_AUDIT_SOCKET__NLMSG_READPRIV },
};
Timothy R. Chavez
2005-05-06 21:46:03 UTC
Permalink
Hello,

Steve had suggested the patch be cleft in two -- seperate the interface
from the mechanics. Once we can determine that the interface is stable,
I think this is probably a good idea. It'll give the person looking at
the patch a little less distraction and a bit more focus methinks.

And to piggyback on this message, I am working on the design document.
It is in XML DocBook format and will try my hardest to get something
released to this list by next week.

Thanks!

-tim
David Woodhouse
2005-05-07 00:31:22 UTC
Permalink
Post by Timothy R. Chavez
This patch is against 12-rc3-mm3 and incorporates feedback I received on
#7U3 and does not include fixes based on Viro feedback (I'll be working
on this over the weekend).
In audit.31 kernel uploading to zeniv now...
--
dwmw2
Steve Grubb
2005-05-07 13:44:04 UTC
Permalink
Post by Timothy R. Chavez
This patch is against 12-rc3-mm3 and incorporates feedback I received on
#7U3 and does not include fixes based on Viro feedback (I'll be working
on this over the weekend).
I installed the .31 kernel. I then ran:

auditctl -w /etc/passwd -k fk_passwd -p rwea

I got this:

May 7 09:35:02 localhost kernel: CPU: 0
May 7 09:35:02 localhost kernel: EIP: 0060:[<c014baf7>] Not tainted VLI
May 7 09:35:02 localhost kernel: EFLAGS: 00210046 (2.6.9-5.0.3.EL.audit.31)
May 7 09:35:02 localhost kernel: EIP is at kmem_cache_alloc+0x25/0x4c
May 7 09:35:02 localhost kernel: eax: 00000080 ebx: 000000d0 ecx:
00000000
edx: e273a000
May 7 09:35:02 localhost kernel: esi: 00200246 edi: 00000000 ebp:
ed99e410
esp: e273ac04
May 7 09:35:02 localhost kernel: ds: 007b es: 007b ss: 0068
May 7 09:35:02 localhost kernel: Process auditctl (pid: 9301,
threadinfo=e273a000 task=e8aec880)
May 7 09:35:02 localhost kernel: Stack: ffffffea 000003ef ea4f6e00 c0141a69
ffffffea 000003ef ea4f6e00 000010e5
May 7 09:35:02 localhost kernel: c01421a5 00000007 00000000 f084b332
ef08d764 00000001 d14995cc 000001bb
May 7 09:35:02 localhost kernel: eef3355c eef3355c eef3355c eef3355c
00800000 c01c00f2 00000000 000001bb
May 7 09:35:02 localhost kernel: Call Trace:
May 7 09:35:02 localhost kernel: [<c0141a69>] audit_to_watch+0x15/0xc0
May 7 09:35:02 localhost kernel: [<c01421a5>] audit_receive_watch+0x3e/0x309
May 7 09:35:02 localhost kernel: [<f084b332>]
__journal_file_buffer+0x10f/0x1e8 [jbd]
May 7 09:35:02 localhost kernel: [<c01c00f2>] avc_has_perm_noaudit+0x8d/0xda
May 7 09:35:02 localhost kernel: [<f084b332>]
__journal_file_buffer+0x10f/0x1e8 [jbd]
May 7 09:35:02 localhost kernel: [<c01c017a>] avc_has_perm+0x3b/0x45
May 7 09:35:02 localhost kernel: [<c013f66f>] audit_receive_msg+0x238/0x25c
May 7 09:35:02 localhost kernel: [<c013f6c5>] audit_receive_skb+0x32/0x70
May 7 09:35:02 localhost kernel: [<c013f72b>] audit_receive+0x28/0x7d
May 7 09:35:02 localhost kernel: [<c02bc636>] netlink_data_ready+0x14/0x43
May 7 09:35:02 localhost kernel: [<c02bbd35>] netlink_sendskb+0x52/0x6b
May 7 09:35:02 localhost kernel: [<c02bc452>] netlink_sendmsg+0x267/0x276
May 7 09:35:02 localhost kernel: [<c029eb3b>] sock_sendmsg+0xdb/0xf7
May 7 09:35:02 localhost kernel: [<c0147179>] buffered_rmqueue+0x1c4/0x1e7
May 7 09:35:02 localhost kernel: [<c0147250>] __alloc_pages+0xb4/0x298
May 7 09:35:02 localhost kernel: [<c015321d>] do_anonymous_page+0x215/0x27f
May 7 09:35:02 localhost kernel: [<c011d04b>]
autoremove_wake_function+0x0/0x2d
May 7 09:35:02 localhost kernel: [<c029fe4b>] sys_sendto+0xc7/0xe2
May 7 09:35:02 localhost kernel: [<c01193e9>] do_page_fault+0x1ac/0x4dc
May 7 09:35:02 localhost kernel: [<c029e8b6>] sock_map_file+0x98/0x106
May 7 09:35:03 localhost kernel: [<c01547e3>] __vma_link+0x59/0x66
May 7 09:35:03 localhost kernel: [<c01548d1>] vma_link+0xe1/0x1dd
May 7 09:35:03 localhost kernel: [<c02a0644>] sys_socketcall+0x14c/0x1dd
May 7 09:35:03 localhost kernel: [<c030336f>] syscall_call+0x7/0xb
May 7 09:35:03 localhost kernel: Code: 5d e9 6e eb 08 00 57 f6 c2 10 89 c7 56
53 89 d3 74 16 31 c9 ba 0f 08 00 00 b8 f4 3f 31 c0 e8 cc 0e fd ff e8 36 60 1b
00
9c 5e fa <8b> 17 8b 02 85 c0 74 10 c7 42 0c 01 00 00 00 48 89 02 8b 44 82


At this point, I would recommend that we fix this patch without any updates to
re-mediate problems Al Viro found. I was planning to release 0.7.4 as soon as
I can successfully test that we can insert, remove, and list watches.

-Steve
David Woodhouse
2005-05-07 17:09:13 UTC
Permalink
Post by Steve Grubb
May 7 09:35:02 localhost kernel: CPU: 0
May 7 09:35:02 localhost kernel: EIP: 0060:[<c014baf7>] Not tainted VLI
May 7 09:35:02 localhost kernel: EFLAGS: 00210046 (2.6.9-5.0.3.EL.audit.31)
May 7 09:35:02 localhost kernel: EIP is at kmem_cache_alloc+0x25/0x4c
00000000
edx: e273a000
ed99e410
esp: e273ac04
Your mailer word-wrapped it; please don't allow it to do that. Also, we
seem to be missing the first few lines. Was this a BUG()?
--
dwmw2
Steve Grubb
2005-05-07 17:22:35 UTC
Permalink
Also, we seem to be missing the first few lines. Was this a BUG()?
It was:

May 7 11:58:45 localhost kernel: Unable to handle kernel NULL pointer
dereference at virtual address 00000000
May 7 11:58:45 localhost kernel: printing eip:
May 7 11:58:45 localhost kernel: c014bb37
May 7 11:58:45 localhost kernel: *pde = 00000000
May 7 11:58:45 localhost kernel: Oops: 0000 [#1]

My current theory is that audit_watch_cache is NULL. I'm currently building a
new kernel to check it out. I'm attaching my current patch against the file
system audit code that fixes numerous little issues. It is not final...its
just so that no one duplicates the effort while debugging. Try as much or as
little of it as you want.

-Steve
Steve Grubb
2005-05-07 18:08:44 UTC
Permalink
Post by Steve Grubb
My current theory is that audit_watch_cache is NULL.
I've confirmed this is the problem. The patch I sent earlier will output a
message if watch_alloc is called and the cache pointer is NULL. I just
checked my logs and the message is there.

The audit system get started very early in the boot sequence. I think its the
first subsystem to get initted. So the question is when should this get
allocated? Should we do a lazy allocation? If so do we have any races to
worry about?

-Steve
Steve Grubb
2005-05-07 22:40:21 UTC
Permalink
Post by Steve Grubb
Should we do a lazy allocation? If so do we have any races to
worry about?
OK, I decided to defer allocating the cache until its needed. (I make no
warranties about races allocating the cache.) The attached patch is what I'm
testing. Here's how it looks:

[***@endeavor ~]# auditctl -w /etc/passwd -k fk_passwd -p rwea
No rules
[***@endeavor ~]# auditctl -l
No rules
AUDIT_WATCH_LIST: dev=3:2, path=/etc/passwd, filterkey=fk_passwd, perms=15,
valid=1
[***@endeavor ~]# auditctl -W /etc/passwd -k fk_passwd -p rwea
No rules
[***@endeavor ~]# auditctl -l
No rules
No watches

Everything looks fine so far. I need to fix auditctl so that it lists watches
that get entered and then I can release the new audit package.

-Steve
David Woodhouse
2005-05-07 22:52:31 UTC
Permalink
+#ifdef CONFIG_AUDITSYSCALL
err = audit_list_watches(pid, seq);
+#else
+ err = -EOPNOTSUPP;
+#endif
Don't do this kind of thing. Just make audit_list_watches() return
-EOPNOTSUPP in the !CONFIG_AUDITSYSCALL case.
--
dwmw2
Steve Grubb
2005-05-08 13:51:05 UTC
Permalink
Post by David Woodhouse
Don't do this kind of thing. Just make audit_list_watches() return
-EOPNOTSUPP in the !CONFIG_AUDITSYSCALL case.
I was just being consistent with other code in audit.c. Attached is another
patch that fixes this + adds 2 messages saying a watch was added and deleted
- this is required for CAPP,

I'm also attaching another patch that makes the other code conform to your
request.

-Steve
Timothy R. Chavez
2005-05-09 14:35:41 UTC
Permalink
static spinlock_t audit_master_watchlist_lock = SPIN_LOCK_UNLOCKED;
HLIST_HEAD(audit_master_watchlist);
@@ -78,9 +78,19 @@
{
struct audit_watch *watch;
+ if (audit_watch_cache == NULL) {
+ /* FIXME: not sure if this is racy */
+ audit_watch_cache =
kmem_cache_create("audit_watch_cache",
+ sizeof(struct audit_watch), 0, 0,
NULL, NULL);
+ if (audit_watch_cache == NULL) {
+ printk(KERN_ERR "Could not allocate
audit_watch_cache");
+ return NULL;
+ }
+ }
I think we're cool here. There are two ways we could arrive here:

audit_watch_alloc()

1) audit_receive_watch->audit_to_watch
2) audit_insert_watch->audit_create_wentry->audit_create_watch

And we'll always hit #1 first (which is protected against contention
with audit_netlink_sem)

audit_wentry_alloc()

1) audit_insert_watch->audit_create_wentry

Because we're holding audit_netlink_sem we'll there will never be a
chance where two "inserts" race here either and because this data is not
accessible via the hooks yet, we won't have to worry about an internal
racing either.

Is that a fair analysis?

Admittedly, my approach was sloppier, and it appeared it wasn't working
for you (but I'm trying to figure out why still... in theory, the init
function should have always been hit prior to being able to add any kind
of watch right). This approach is better, but I'm still not liking this
conditional (that's only ever going to execute its contents once).

Maybe we should be using unlikely() here?

-tim
Steve Grubb
2005-05-09 14:45:21 UTC
Permalink
Post by Timothy R. Chavez
Is that a fair analysis?
I guess. Is this good for you David?
Post by Timothy R. Chavez
Admittedly, my approach was sloppier, and it appeared it wasn't
working for you
Did it work for you?
Post by Timothy R. Chavez
Maybe we should be using unlikely() here?
Sure. The only other benefit that I could think of is that by deferring the
allocation, it only occurs IFF the filesystem auditing is used. Anyone doing
syscall only (or no auditing - just SE Linux avc denials) has a little bit of
memory saved.

-Steve
Timothy R. Chavez
2005-05-09 14:57:35 UTC
Permalink
Post by Steve Grubb
Post by Timothy R. Chavez
Is that a fair analysis?
I guess. Is this good for you David?
Post by Timothy R. Chavez
Admittedly, my approach was sloppier, and it appeared it wasn't
working for you
Did it work for you?
Yes it did. That's what weird to me... I briefly looked at the audit.31
kernel though and see the filesystem_init() function being called. But,
regardless, I think I read somewhere people generally do not like those
types of "init" functions any way.
Post by Steve Grubb
Post by Timothy R. Chavez
Maybe we should be using unlikely() here?
Sure. The only other benefit that I could think of is that by deferring the
allocation, it only occurs IFF the filesystem auditing is used. Anyone doing
syscall only (or no auditing - just SE Linux avc denials) has a little bit of
memory saved.
Yeah, well the init function could have been macro'ed out. I think its
water under the bridge now, unless there are in fact racey conditions.
Post by Steve Grubb
-Steve
--
Linux-audit mailing list
http://www.redhat.com/mailman/listinfo/linux-audit
David Woodhouse
2005-05-10 12:36:33 UTC
Permalink
Post by Timothy R. Chavez
Admittedly, my approach was sloppier, and it appeared it wasn't working
for you (but I'm trying to figure out why still... in theory, the init
function should have always been hit prior to being able to add any kind
of watch right).
There are _two_ instances of audit_init(). You only added
audit_filesystem_init() to one of them.

--- audit.c~ 2005-05-10 13:25:19.000000000 +0100
+++ audit.c 2005-05-10 13:34:06.000000000 +0100
@@ -542,6 +542,7 @@ int __init audit_init(void)

audit_initialized = 1;
audit_enabled = audit_default;
+ audit_filesystem_init();
audit_log(NULL, "initialized");
return 0;
}
--
dwmw2
Stephen Smalley
2005-05-09 14:24:47 UTC
Permalink
Post by Timothy R. Chavez
+/*
+ * The update hook is responsible for watching and unwatching d_inodes during
+ * their lifetimes in dcache. Each d_inode being watched is pinned in memory.
+ * As soon as a d_inode becomes unwatched (when !d_inode->i_audit->wentry), it
+ * is unpinned.
Wouldn't it be simpler to hold a reference to the dentry upon
audit_insert_watch(), saving the dentry in the audit_wentry, and then
release it upon audit_wentry_free()? This would keep it pinned from the
time of insertion until the wentry is freed.
--
Stephen Smalley
National Security Agency
Timothy R. Chavez
2005-05-09 14:51:09 UTC
Permalink
Post by Stephen Smalley
Post by Timothy R. Chavez
+/*
+ * The update hook is responsible for watching and unwatching d_inodes during
+ * their lifetimes in dcache. Each d_inode being watched is pinned in memory.
+ * As soon as a d_inode becomes unwatched (when !d_inode->i_audit->wentry), it
+ * is unpinned.
Wouldn't it be simpler to hold a reference to the dentry upon
audit_insert_watch(), saving the dentry in the audit_wentry, and then
release it upon audit_wentry_free()? This would keep it pinned from the
time of insertion until the wentry is freed.
Hi Stephen,

I like the suggestion, but I don't think it'll necessarily be simpler.
Here's why:

When I grab a reference to a dentry and store it with the wentry and
then I unlink (via rm) the dentry, because the refcount > 0, it'll be
kept in memory until I release the reference, right?

The upshot is that this will require a conditional and a possible swap
only on insertions.

-tim
Stephen Smalley
2005-05-11 16:15:14 UTC
Permalink
Post by Timothy R. Chavez
I like the suggestion, but I don't think it'll necessarily be simpler.
When I grab a reference to a dentry and store it with the wentry and
then I unlink (via rm) the dentry, because the refcount > 0, it'll be
kept in memory until I release the reference, right?
The upshot is that this will require a conditional and a possible swap
only on insertions.
Not sure I follow your last statement. With regard to the first point,
yes, I think you are correct - d_delete would just unhash the dentry,
not turn it into a negative dentry. An option might be to hook d_delete
again to release the saved reference before d_delete performs the check
of the reference count.

My concern is that the current code seems very fragile; getting
references without saving them anywhere, putting references that were
not acquired in the same call nor explicitly saved earlier (even though
in theory you've "saved" them by getting them earlier), putting a
reference and then re-getting one shortly thereafter without any
revalidation of the dentry.
--
Stephen Smalley
National Security Agency
Timothy R. Chavez
2005-05-11 17:00:50 UTC
Permalink
Post by Stephen Smalley
My concern is that the current code seems very fragile; getting
references without saving them anywhere, putting references that were
not acquired in the same call nor explicitly saved earlier (even though
in theory you've "saved" them by getting them earlier), putting a
reference and then re-getting one shortly thereafter without any
revalidation of the dentry.
I mucked around with it a bit and combined your idea and my idea of how it
should be done. I "_think_" it should be A-okay now. I'm going to release a
patch momentarily and I'll try to explain fully what I'm doing with pinning
(and I've even thrown in some comments to the code to ease the pain a
little). I was encountering deadlock with the way I was doing it previously
(because I wasn't dropping the reference in d_delete and never getting to
dentry_iput).

-tim

Loading...