[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

fsaudit patch against audit.53 RPM



Hello,

I already see two little bugs in my code in audit_remove_watch.  David,
before you build can you add in the spin_unlock() before the goto?

kernel/auditfs.c: audit_remove_watch()

+
+       spin_lock(&auditfs_lock);
+       real = audit_fetch_watch(nd.last.name, data);
+       if (!real)
->		spin_unlock(&auditfs_lock);
                goto audit_remove_watch_release;
-       }
        audit_destroy_watch(real);
+       spin_unlock(&auditfs_lock);

And, place before the kfree an audit_watch_put()

kernel/auditsc.c: audit_attach_wdata()
+
+auditfs_attach_wdata_fail:
+       hlist_for_each_entry_safe(this, pos, tmp, &ax->watches, node) {
+               hlist_del(&this->node);
->		 audit_watch_put(this);
+               kfree(this);
+       }
+
+       return -ENOMEM


This patch includes David's Implicit unpinning patch.

Changelog:

* Removed a fundamental mathematical operation embedded in
a debugging statement ;)

* Removed the local lock on inode audit data in favor of of a global
auditfs_lock spinlock

* Added a list of watches that could be associated with any given
inode in favor of a pointer to just one

* Attempt to remove watches first via the master watchlist and then
by path lookup

* Remove watch from inode, master watchlist, and local watchlist 
when removing watch via administrator action

* Altered record in kernel/auditsc.c to support watches per inode per
record

-tim


diff -Nurp linux-2.6.9-prep/fs/dcache.c linux-2.6.9~53+tim/fs/dcache.c
--- linux-2.6.9-prep/fs/dcache.c	2011-06-05 20:45:41.000000000 -0500
+++ linux-2.6.9~53+tim/fs/dcache.c	2011-06-02 22:56:34.000000000 -0500
@@ -1112,7 +1112,6 @@ void d_delete(struct dentry * dentry)
 	/*
 	 * Are we the only user?
 	 */
-	audit_dentry_unpin(dentry);
 	spin_lock(&dcache_lock);
 	spin_lock(&dentry->d_lock);
 	if (atomic_read(&dentry->d_count) == 1) {
diff -Nurp linux-2.6.9-prep/fs/inode.c linux-2.6.9~53+tim/fs/inode.c
--- linux-2.6.9-prep/fs/inode.c	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/fs/inode.c	2011-06-02 22:56:34.000000000 -0500
@@ -262,7 +262,7 @@ void clear_inode(struct inode *inode)
 		bd_forget(inode);
 	if (inode->i_cdev)
 		cd_forget(inode);
-	inode->i_state = I_CLEAR;
+	inode->i_state = I_CLEAR | (inode->i_state & I_AUDIT);
 }
 
 EXPORT_SYMBOL(clear_inode);
@@ -1019,7 +1019,7 @@ void generic_delete_inode(struct inode *
 	hlist_del_init(&inode->i_hash);
 	spin_unlock(&inode_lock);
 	wake_up_inode(inode);
-	if (inode->i_state != I_CLEAR)
+	if ((inode->i_state & ~I_AUDIT) != I_CLEAR)
 		BUG();
 	destroy_inode(inode);
 }
diff -Nurp linux-2.6.9-prep/fs/namei.c linux-2.6.9~53+tim/fs/namei.c
--- linux-2.6.9-prep/fs/namei.c	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/fs/namei.c	2005-06-02 01:54:49.000000000 -0500
@@ -2116,8 +2116,8 @@ int vfs_rename_other(struct inode *old_d
 
 	error = audit_notify_watch(old_dentry->d_inode, MAY_WRITE);
 	if (!error)
-		security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry);
-	
+		security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry);
+
 	if (target)
 		up(&target->i_sem);
 	dput(new_dentry);
diff -Nurp linux-2.6.9-prep/include/linux/audit.h linux-2.6.9~53+tim/include/linux/audit.h
--- linux-2.6.9-prep/include/linux/audit.h	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/include/linux/audit.h	2011-06-05 21:03:27.000000000 -0500
@@ -219,7 +219,7 @@ struct audit_watch {
 	atomic_t		w_count;
 	struct hlist_node 	w_node;		/* per-directory list	      */
 	struct hlist_node	w_master;	/* Master watch list	      */
-	struct dentry		*w_dentry;	/* Watched inode	      */
+	struct hlist_node	w_watched;	/* Watches on inode	      */
 	dev_t			w_dev;		/* Superblock device	      */
 	__u32			w_perms;	/* Permissions filtering      */
 	char			*w_name;	/* Watch point beneath parent */
@@ -231,9 +231,8 @@ struct audit_inode_data {
 	int			count;
 	struct audit_inode_data *next_hash;	/* Watch data hash table      */
 	struct inode		*inode;		/* Inode to which it belongs  */
-	struct audit_watch	*watch;		/* Watch for this inode	      */
+	struct hlist_head	watches;	/* List of watches on inode   */
 	struct hlist_head 	watchlist;	/* Watches for children       */
-	rwlock_t		lock;
 };
 
 
@@ -242,6 +241,11 @@ struct audit_sig_info {
 	pid_t		pid;
 };
 
+struct audit_watch_info {
+	struct hlist_node node;
+	struct audit_watch *watch;
+};
+
 struct audit_buffer;
 struct audit_context;
 struct inode;
@@ -302,10 +306,9 @@ extern int audit_receive_watch(int type,
 extern void audit_inode_free(struct inode *inode);
 extern void audit_update_watch(struct dentry *dentry, int remove);
 extern void audit_watch_put(struct audit_watch *watch);
-extern void audit_dentry_unpin(struct dentry *dentry);
 extern struct audit_watch *audit_watch_get(struct audit_watch *watch);
 extern int audit_notify_watch(struct inode *inode, int mask);
-extern int auditfs_attach_wdata(struct inode *inode, struct audit_watch *watch,
+extern int auditfs_attach_wdata(struct inode *inode, struct hlist_head *watches,
 				int mask);
 #else
 #define audit_filesystem_init() ({ 0; })
diff -Nurp linux-2.6.9-prep/include/linux/fs.h linux-2.6.9~53+tim/include/linux/fs.h
--- linux-2.6.9-prep/include/linux/fs.h	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/include/linux/fs.h	2011-06-02 22:56:34.000000000 -0500
@@ -986,6 +986,7 @@ struct super_operations {
 #define I_FREEING		16
 #define I_CLEAR			32
 #define I_NEW			64
+#define I_AUDIT			128
 
 #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
 
diff -Nurp linux-2.6.9-prep/kernel/audit.c linux-2.6.9~53+tim/kernel/audit.c
--- linux-2.6.9-prep/kernel/audit.c	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/kernel/audit.c	2005-06-02 02:12:39.000000000 -0500
@@ -581,6 +581,7 @@ static int __init audit_enable(char *str
 	printk(KERN_INFO "audit: %s%s\n",
 	       audit_default ? "enabled" : "disabled",
 	       audit_initialized ? "" : " (after initialization)");
+	audit_filesystem_init();
 	if (audit_initialized)
 		audit_enabled = audit_default;
 	return 0;
diff -Nurp linux-2.6.9-prep/kernel/auditfs.c linux-2.6.9~53+tim/kernel/auditfs.c
--- linux-2.6.9-prep/kernel/auditfs.c	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/kernel/auditfs.c	2011-06-04 00:37:04.000000000 -0500
@@ -19,7 +19,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
  * 02111-1307  USA
  *
- * Written by Timothy R. Chavez <chavezt us ibm com>
+ * Written by:
+ * Timothy R. Chavez <chavezt us ibm com>
  *
  */
 
@@ -49,9 +50,8 @@ extern int audit_enabled;
 
 static kmem_cache_t *audit_watch_cache;
 
-/* Read-heavy list */
 static HLIST_HEAD(master_watchlist);
-static spinlock_t master_watchlist_lock = SPIN_LOCK_UNLOCKED;
+static spinlock_t auditfs_lock = SPIN_LOCK_UNLOCKED;
 
 struct audit_skb_list {
 	struct hlist_node list;
@@ -59,6 +59,7 @@ struct audit_skb_list {
 	size_t size;
 };
 
+extern spinlock_t inode_lock;
 
 static int audit_nr_watches;
 static int audit_pool_size;
@@ -70,6 +71,8 @@ static int auditfs_cache_buckets = 16384
 module_param(auditfs_cache_buckets, int, 0);
 MODULE_PARM_DESC(auditfs_cache_buckets, "Number of auditfs cache entries to allocate (default 16384)\n");
 
+static void audit_data_put(struct audit_inode_data *data);
+
 static int audit_data_pool_grow(void)
 {
 	struct audit_inode_data *new;
@@ -95,6 +98,13 @@ static void audit_data_pool_shrink(void)
 {
 	spin_lock(&auditfs_hash_lock);
 	audit_nr_watches--;
+
+	while (audit_pool_size > audit_nr_watches + 1) {
+		struct audit_inode_data *old = audit_data_pool;
+		audit_data_pool = old->next_hash;
+		audit_pool_size--;
+		kfree(old);
+	}
 	spin_unlock(&auditfs_hash_lock);
 }
 
@@ -102,11 +112,18 @@ static struct audit_inode_data *audit_da
 {
 	struct audit_inode_data **list;
 	struct audit_inode_data *ret = NULL;
-	int h = hash_ptr(inode, auditfs_hash_bits);
+	int h;
 
-	list = &auditfs_hash_table[h];
 	spin_lock(&auditfs_hash_lock);
 
+	/* I_AUDIT bit can only be changed under auditfs_hash_lock, so no need
+	   to lock inode_lock (on all known hardware) */
+	if (!allocate && !(inode->i_state & I_AUDIT))
+		goto out;
+
+	h = hash_ptr(inode, auditfs_hash_bits);
+	list = &auditfs_hash_table[h];
+
 	while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) {
 		dprintk("list %p -> %p\n", list, *list);
 		list = &(*list)->next_hash;
@@ -123,18 +140,22 @@ static struct audit_inode_data *audit_da
 		dprintk("allocate from pool. %d left\n", audit_pool_size);
 
 		INIT_HLIST_HEAD(&ret->watchlist);
-		ret->watch = NULL;
-		ret->lock = RW_LOCK_UNLOCKED;
+		INIT_HLIST_HEAD(&ret->watches);
 		ret->inode = inode;
 		ret->next_hash = *list;
 		ret->count = 2;
 		*list = ret;
-	}
+
+		spin_lock(&inode_lock);
+		inode->i_state |= I_AUDIT;
+		spin_unlock(&inode_lock);
+        }
 	if (ret) {
 		dprintk("Got audit data %p for inode %p (%lu), count++ now %d. From %p: ", 
-		       ret, ret->inode, ret->inode->i_ino, ret->count, __builtin_return_address(0));
+			ret, ret->inode, ret->inode->i_ino, ret->count, __builtin_return_address(0));
 		__print_symbol("%s\n", __builtin_return_address(0));
 	}
+ out:
 	spin_unlock(&auditfs_hash_lock);
 
 	return ret;
@@ -142,47 +163,31 @@ static struct audit_inode_data *audit_da
 
 /* Private Interface */
 
-/* Unpin the dentry stored at watch->w_dentry. */
-static inline void audit_unpin(struct audit_watch *watch)
-{
-	if (watch && watch->w_dentry) {
-		dput(watch->w_dentry);
-		watch->w_dentry = NULL;
-	}
-}
-
-/* Pin the dentry and store it at watch->w_dentry. */
-static inline void audit_pin(struct audit_watch *watch,
-			     struct dentry *dentry)
-{
-	if (watch && !watch->w_dentry)
-		watch->w_dentry = dget(dentry);
-}
-
-static inline struct audit_watch *audit_watch_fetch(const char *name,
+/* Caller should be holding auditfs_lock */
+static inline struct audit_watch *audit_fetch_watch(const char *name,
 						    struct audit_inode_data *data)
 {
 	struct audit_watch *watch, *ret = NULL;
 	struct hlist_node *pos;
 
 	hlist_for_each_entry(watch, pos, &data->watchlist, w_node)
-		if(!strcmp(watch->w_name, name)) {
+		if (!strcmp(watch->w_name, name)) {
 			ret = audit_watch_get(watch);
 			break;
 		}
-
+
 	return ret;
 }
 
-static inline struct audit_watch *audit_watch_fetch_lock(const char *name,
+static inline struct audit_watch *audit_fetch_watch_lock(const char *name,
 							 struct audit_inode_data *data)
 {
 	struct audit_watch *ret = NULL;
 
 	if (name && data) {
-		read_lock(&data->lock);
-		ret = audit_watch_fetch(name, data);
-		read_unlock(&data->lock);
+		spin_lock(&auditfs_lock);
+		ret = audit_fetch_watch(name, data);
+		spin_unlock(&auditfs_lock);
 	}
 
 	return ret;
@@ -201,18 +206,12 @@ static inline struct audit_watch *audit_
 	return watch;
 }
 
-/*
- * If a watch has been removed from a watchlist, we promptly unpin the
- * dentry that was being watched, as it is no longer important to us.
- */
 static inline void audit_watch_free(struct audit_watch *watch)
 {
 	if (watch) {
-		audit_unpin(watch);
 		kfree(watch->w_name);
 		kfree(watch->w_path);
 		kfree(watch->w_filterkey);
-		BUG_ON(watch->w_dentry);
 		BUG_ON(!hlist_unhashed(&watch->w_node));
 		BUG_ON(!hlist_unhashed(&watch->w_master));
 		kmem_cache_free(audit_watch_cache, watch);
@@ -291,95 +290,33 @@ audit_to_transport_exit:
 	return t;
 }
 
-/*
- * Because we're protected by audit_netlink_sem, there will never be a race on
- * current insertions of the same watch.  However, we do lock both the master
- * watchlist and the local watchlist to ensure that insertions and removals of
- * watches are serialized.
- */
-static inline int audit_create_watch(const char *path,
-				     const char *name,
-				     const char *filterkey,
-				     __u32 perms, dev_t dev,
-				     struct audit_inode_data *data)
+static void audit_data_unhash(struct audit_inode_data *data)
 {
-	int ret;
-	struct audit_watch *watch;
-
-	ret = -EEXIST;
-	watch = audit_watch_fetch_lock(name, data);
-	if (watch) {
-		audit_watch_put(watch);
-		goto audit_create_watch_exit;
-	}
-
-	ret = -ENOMEM;
-	watch = audit_watch_alloc();
-	if (!watch)
-		goto audit_create_watch_exit;
-
-	watch->w_path = kmalloc(strlen(path)+1, GFP_KERNEL);
-	if (!watch->w_path)
-		goto audit_create_watch_fail;
-	strcpy(watch->w_path, path);
-
-	watch->w_name = kmalloc(strlen(name)+1, GFP_KERNEL);
-	if (!watch->w_name)
-		goto audit_create_watch_fail;
-	strcpy(watch->w_name, name);
+	int h = hash_ptr(data->inode, auditfs_hash_bits);
+	struct audit_inode_data **list = &auditfs_hash_table[h];
 
-	if (filterkey) {
-		watch->w_filterkey = kmalloc(strlen(filterkey)+1, GFP_KERNEL);
-		if (!watch->w_filterkey)
-			goto audit_create_watch_fail;
-		strcpy(watch->w_filterkey, filterkey);
-	}
-
-	watch->w_dev = dev;
-	watch->w_perms = perms;
-
-	audit_watch_get(watch);
-
-	write_lock(&data->lock);
-	hlist_add_head(&watch->w_node, &data->watchlist);
-	spin_lock(&master_watchlist_lock);
-	hlist_add_head(&watch->w_master, &master_watchlist);
-	spin_unlock(&master_watchlist_lock);
-	write_unlock(&data->lock);
-	return 0;
+	while (*list && (unsigned long)((*list)->inode) < (unsigned long)data->inode)
+		list = &(*list)->next_hash;
 
- audit_create_watch_fail:
-	audit_watch_put(watch);
+	BUG_ON(*list != data);
+	*list = data->next_hash;
 
- audit_create_watch_exit:
-	return ret;
+	spin_lock(&inode_lock);
+	data->inode->i_state &= ~I_AUDIT;
+	spin_unlock(&inode_lock);
+	data->inode = NULL;
 }
 
-/*
- * There's three ways we can arrive at audit_destroy_watch.  Two of the ways
- * may cause contention with one another, while the third may not.  They are:
- *
- * 1)  An administrator has explicitly requested a watch removal
- * 2)  A directory with an "active" watchlist is rename()'ed
- * 3)  The inode is in the process of being destroyed
- *
- * In ways 1) and 2) the data->lock must be held to safely enter.  We use
- * the audit_master_watchlist spinlock to protect against removal of two
- * consecutive entries concurrently in the master watchlist.
- *
- * If way 3) occurs there will no longer be any reachable references to the
- * inode, thus there is only one way to remove this watch, and no need to
- * hold the data->lock.
- */
 static inline void audit_destroy_watch(struct audit_watch *watch)
 {
 	if (watch) {
-		hlist_del_init(&watch->w_node);
-		spin_lock(&master_watchlist_lock);
+		hlist_del_init(&watch->w_watched);
 		hlist_del_init(&watch->w_master);
-		spin_unlock(&master_watchlist_lock);
+		hlist_del_init(&watch->w_node);
+		audit_watch_put(watch);
 		audit_watch_put(watch);
 		audit_watch_put(watch);
+
 	}
 }
 
@@ -406,7 +343,7 @@ static void audit_data_put(struct audit_
 	__print_symbol("%s\n", __builtin_return_address(0));
 
 	if (data->count == 1 && data->inode && 
-	    !data->watch && hlist_empty(&data->watchlist)) {
+	    hlist_empty(&data->watches) && hlist_empty(&data->watchlist)) {
 		dprintk("Last put.\n");
 		data->count--;
 	}
@@ -414,22 +351,13 @@ static void audit_data_put(struct audit_
 	if (!data->count) {
 		/* We are last user. Remove it from the hash table to
 		   disassociate it from its inode */
-		int h = hash_ptr(data->inode, auditfs_hash_bits);
-
-		struct audit_inode_data **list = &auditfs_hash_table[h];
-
-		while (*list && (unsigned long)((*list)->inode) < (unsigned long)data->inode)
-			list = &(*list)->next_hash;
-
-		BUG_ON(*list != data);
-		*list = data->next_hash;
-		data->inode = NULL;
-
+		if (data->inode)
+			audit_data_unhash(data);
 		spin_unlock(&auditfs_hash_lock);
 
 		audit_drain_watchlist(data);
-		spin_lock(&auditfs_hash_lock);
 
+		spin_lock(&auditfs_hash_lock);
 		/* Check whether to free it or return it to the pool */
 		if (audit_nr_watches > audit_pool_size) {
 			dprintk("Back to pool. %d watches, %d in pool\n", audit_nr_watches, audit_pool_size);
@@ -441,21 +369,21 @@ static void audit_data_put(struct audit_
 			kfree(data);
 		}
 	}
-
 	spin_unlock(&auditfs_hash_lock);
 }
 
 static inline int audit_insert_watch(struct audit_watch *watch, uid_t loginuid)
 {
-	int ret;
-	struct nameidata nd;
-	struct audit_inode_data *pdata;
-
+        int ret;
+        struct nameidata nd;
+        struct audit_inode_data *pdata;
+        struct audit_watch *lookup;
+
 	/* Grow the pool by two -- one for the watch itself, and
 	   one for the parent directory */
 	if (audit_data_pool_grow())
 		return -ENOMEM;
-
+
 	ret = path_lookup(watch->w_path, LOOKUP_PARENT, &nd);
 	if (ret < 0)
 		goto out;
@@ -463,22 +391,35 @@ static inline int audit_insert_watch(str
 	ret = -EPERM;
 	if (nd.last_type != LAST_NORM || !nd.last.name)
 		goto release;
-
-	ret = -ENOMEM;
+
 	pdata = audit_data_get(nd.dentry->d_inode, 1);
 	if (!pdata)
 		goto put_pdata;
 
-	ret = audit_create_watch(watch->w_path,
-				 nd.last.name,
-				 watch->w_filterkey,
-				 watch->w_perms,
-				 nd.dentry->d_inode->i_sb->s_dev,
-				 pdata);
-	if (ret < 0)
+	ret = -EEXIST;
+	lookup = audit_fetch_watch_lock(nd.last.name, pdata);
+	if (lookup) {
+		audit_watch_put(lookup);
 		goto put_pdata;
+	}
+
+	ret = -ENOMEM;
+	watch->w_name = kmalloc(strlen(nd.last.name)+1, GFP_KERNEL);
+	if (!watch->w_name)
+		goto put_pdata;
+	strcpy(watch->w_name, nd.last.name);
+
+	watch->w_dev = nd.dentry->d_inode->i_sb->s_dev;
+
+	ret = 0;
+	audit_watch_get(watch);
+	spin_lock(&auditfs_lock);
+	hlist_add_head(&watch->w_node, &pdata->watchlist);
+	hlist_add_head(&watch->w_master, &master_watchlist);
+	spin_unlock(&auditfs_lock);
 
 	audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u inserted watch\n", loginuid);
+
 	/* __d_lookup will attach the audit data, if nd.last exists. */
 	dput(d_lookup(nd.dentry, &nd.last));
 
@@ -489,16 +430,27 @@ static inline int audit_insert_watch(str
  out:
 	if (ret)
 		audit_data_pool_shrink();
-
+
 	return ret;
 }
 
 static inline int audit_remove_watch(struct audit_watch *watch, uid_t loginuid)
 {
-	int ret;
+	int ret = 0;
 	struct nameidata nd;
-	struct audit_inode_data *data;
-	struct audit_watch *real;
+	struct audit_inode_data *data = NULL;
+	struct audit_watch *real, *this;
+	struct hlist_node *pos, *tmp;
+
+	/* Let's try removing via the master watchlist first */
+	spin_lock(&auditfs_lock);
+	hlist_for_each_entry_safe(this, pos, tmp, &master_watchlist, w_master)
+		if (!strcmp(this->w_path, watch->w_path)) {
+			audit_destroy_watch(this);
+			spin_unlock(&auditfs_lock);
+			goto audit_remove_watch_exit;
+		}
+	spin_unlock(&auditfs_lock);
 
 	ret = path_lookup(watch->w_path, LOOKUP_PARENT, &nd);
 	if (ret < 0)
@@ -511,32 +463,55 @@ static inline int audit_remove_watch(str
 	data = audit_data_get(nd.dentry->d_inode, 0);
 	if (!data)
 		goto audit_remove_watch_release;
-
-	write_lock(&data->lock);
-	real = audit_watch_fetch(nd.last.name, data);
-	if (!real) {
-		write_unlock(&data->lock);
+
+	spin_lock(&auditfs_lock);
+	real = audit_fetch_watch(nd.last.name, data);
+	if (!real)
 		goto audit_remove_watch_release;
-	}
 	audit_destroy_watch(real);
+	spin_unlock(&auditfs_lock);
 	audit_watch_put(real);
-	audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u removed watch\n", loginuid);
-	write_unlock(&data->lock);
-	audit_data_put(data);
-	audit_data_pool_shrink();
+
 	ret = 0;
 
 audit_remove_watch_release:
 	path_release(&nd);
 audit_remove_watch_exit:
+	audit_data_put(data);
+	if (!ret) {
+		audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u removed watch\n", loginuid);
+		audit_data_pool_shrink();
+	}
+
 	return ret;
 }
 
+
+/* Caller should be holding auditfs_lock */
+struct audit_watch *audit_is_watched(struct audit_watch *watch,
+				     struct audit_inode_data *data)
+{
+	struct audit_watch *watched;
+	struct hlist_node *pos;
+
+	if (watch && data) {
+		hlist_for_each_entry(watched, pos, &data->watches, w_watched)
+			if (!strcmp(watch->w_name, watched->w_name))
+				return audit_watch_get(watched);
+	}
+
+	return NULL;
+}
+
 struct audit_watch *audit_watch_get(struct audit_watch *watch)
 {
+	int new;
+
 	if (watch) {
 		BUG_ON(!atomic_read(&watch->w_count));
-		atomic_inc(&watch->w_count);
+		new = atomic_inc_return(&watch->w_count);
+		dprintk("Increase count on watch %p to %d\n",
+		       watch, new);
 	}
 
 	return watch;
@@ -544,8 +519,15 @@ struct audit_watch *audit_watch_get(stru
 
 void audit_watch_put(struct audit_watch *watch)
 {
-	if (watch && atomic_dec_and_test(&watch->w_count))
-		audit_watch_free(watch);
+	int new;
+
+	if (watch) {
+		new = atomic_dec_return(&watch->w_count);
+		if (!new)
+			audit_watch_free(watch);
+		dprintk("Reduce count on watch %p to %d\n",
+		       watch, new);
+	}
 }
 
 /*
@@ -562,9 +544,10 @@ void audit_watch_put(struct audit_watch 
  *	__d_lookup()
  */
 void audit_update_watch(struct dentry *dentry, int remove)
-{
-	struct audit_watch *watch = NULL;
+{
+	struct audit_watch *me, *this, *watch;
 	struct audit_inode_data *data, *parent;
+	struct hlist_node *pos, *tmp;
 
 	if (likely(!audit_enabled))
 		return;
@@ -581,7 +564,7 @@ void audit_update_watch(struct dentry *d
 	if (!parent)
 		return;
 
-	watch = audit_watch_fetch_lock(dentry->d_name.name, parent);
+	watch = audit_fetch_watch_lock(dentry->d_name.name, parent);
 
 	/* Fetch audit data, using the preallocated one from the watch if
 	   there is actually a relevant watch and the inode didn't already
@@ -593,29 +576,29 @@ void audit_update_watch(struct dentry *d
 	if (!data)
 		goto put_watch;
 
-	write_lock(&data->lock);
+	spin_lock(&auditfs_lock);
 	if (remove) {
-		/* If the inode was being watched for _this_ pathname, clear the watch */
-		/* FIXME: If there was _another_ path to the same inode which was 
-		   supposed to be watched, we ought to continue watching. */
-		if (watch && data->watch && 
-		    !strcmp(watch->w_name, data->watch->w_name)) {
-			audit_watch_put(data->watch);
-			data->watch = NULL;
+		me = audit_is_watched(watch, data);
+		if (me) {
+			hlist_del_init(&me->w_watched);
+			audit_watch_put(me);
 		}
-	} else if (!data->watch) {
-		/* Inode wasn't watched before. Maybe it is now */
-		data->watch = audit_watch_get(watch);
-		audit_pin(data->watch, dentry);
-	} else if (hlist_unhashed(&data->watch->w_node)) {
-		/* Old watch is dead now. Drop it and add new one .*/
-		audit_watch_put(data->watch);
-		data->watch = audit_watch_get(watch);
-		audit_pin(data->watch, dentry);
+	} else {
+		hlist_for_each_entry_safe(this, pos, tmp, &data->watches, w_watched)
+			if (hlist_unhashed(&this->w_node)) {
+				hlist_del(&this->w_watched);
+				audit_watch_put(this);
+			}
+		me = audit_is_watched(watch, data);
+		if (!me && watch) {
+			audit_watch_get(watch);
+			hlist_add_head(&watch->w_watched, &data->watches);
+		}
+		audit_watch_put(me);
 	}
-	/* FIXME: If inode's i_audit is no longer used, clear it. */
-	write_unlock(&data->lock);
+	spin_unlock(&auditfs_lock);
 	audit_data_put(data);
+
  put_watch:
 	audit_watch_put(watch);
 	audit_data_put(parent);
@@ -671,19 +654,18 @@ int audit_list_watches(int pid, int seq)
 
  restart:
 	INIT_HLIST_HEAD(&skb_list);
-	spin_lock(&master_watchlist_lock);
+	spin_lock(&auditfs_lock);
 
 	hlist_for_each_entry(watch, pos, &master_watchlist, w_master) {
 		audit_watch_get(watch);
-		spin_unlock(&master_watchlist_lock);
+		spin_unlock(&auditfs_lock);
 		entry = audit_to_skb(watch);
 		if (IS_ERR(entry)) {
 			ret = PTR_ERR(entry);
-			spin_unlock(&master_watchlist_lock);
 			goto audit_list_watches_fail;
 		}
 		hlist_add_head(&entry->list, &skb_list);
-		spin_lock(&master_watchlist_lock);
+		spin_lock(&auditfs_lock);
 		if (hlist_unhashed(&watch->w_master)) {
 			/* This watch was removed from the list while we 
 			   pondered it. We could play tricks to find how far
@@ -702,7 +684,7 @@ int audit_list_watches(int pid, int seq)
 		}
 		audit_watch_put(watch);
 	}
-	spin_unlock(&master_watchlist_lock);
+	spin_unlock(&auditfs_lock);
 
 	hlist_for_each_entry_safe(entry, pos, tmp, &skb_list, list) {
 		audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 0, 1, 
@@ -739,7 +721,7 @@ int audit_receive_watch(int type, int pi
 	if (req->fklen > AUDIT_FILTERKEY_MAX)
 		goto audit_receive_watch_exit;
 
-	if ( payload[req->fklen] != '/')
+	if (payload[req->fklen] != '/')
 		goto audit_receive_watch_exit;
 
 	if (req->perms > (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND))
@@ -761,50 +743,35 @@ int audit_receive_watch(int type, int pi
 		ret = -EINVAL;
 	}
 
+	if (ret < 0 || type == AUDIT_WATCH_REM)
+		audit_watch_put(watch);
+
 audit_receive_watch_exit:
-	audit_watch_free(watch);
 	return ret;
 }
 
 void audit_inode_free(struct inode *inode)
 {
+        struct audit_watch *watch;
+        struct hlist_node *pos, *tmp;
 	struct audit_inode_data *data = audit_data_get(inode, 0);
 
-	if (data) {
-		audit_drain_watchlist(data);
-		audit_watch_put(data->watch);
-		data->watch = NULL;
-		audit_data_put(data);
-	}
-}
-
-/*
- * When we delete a dentry we check to see whether or not we're being
- * watched.  If we are watched, we have to put back our reference to
- * the dentry to unpin it from memory or bad things happen.
- *
- * Hook appears in fs/dcache.c:d_delete()
- */
-void audit_dentry_unpin(struct dentry *dentry)
-{
-	struct audit_inode_data *parent;
-	struct audit_watch *watch;
-
-	parent = audit_data_get(dentry->d_parent->d_inode, 0);
-	if (!parent)
-		return;
+	if (inode->i_sb->s_dev == MKDEV(3,6) || inode->i_sb->s_dev == MKDEV(3,9))
+		dprintk("Put inode #%lu, data %p\n", inode->i_ino, data);
 
-	watch = audit_watch_fetch_lock(dentry->d_name.name, parent);
-	if (watch) {
-		struct audit_inode_data *data = audit_data_get(dentry->d_inode, 0);
+        if (data) {
+		spin_lock(&auditfs_hash_lock);
+		audit_data_unhash(data);
+		spin_unlock(&auditfs_hash_lock);
 
-		if (data) {
-			audit_unpin(data->watch);
-			audit_data_put(data);
-		}
-		audit_watch_put(watch);
+		audit_drain_watchlist(data);
+		/* Release all our references to any watches we may have on us */
+        	hlist_for_each_entry_safe(watch, pos, tmp, &data->watches, w_watched) {
+			hlist_del(&watch->w_watched);
+                	audit_watch_put(watch);
+        	}
+		audit_data_put(data);
 	}
-	audit_data_put(parent);
 }
 
 int audit_filesystem_init(void)
@@ -849,40 +816,26 @@ audit_filesystem_init_exit:
 
 int audit_notify_watch(struct inode *inode, int mask)
 {
-	int ret = 0;
-	struct audit_inode_data *data;
-	struct audit_watch *watch = NULL;
-
+        int ret = 0;
+        struct audit_inode_data *data;
+
 	if (likely(!audit_enabled))
-		return 0;
-
+                return 0;
+
 	if (!inode || !current->audit_context)
-		return 0;
-
+                return 0;
+
 	data = audit_data_get(inode, 0);
 	if (!data)
 		return 0;
 
-	/* 
-	 * Won't be able to drop i_audit->watch reference
-	 * before we obtain our own reference
-	 */
-	read_lock(&data->lock);
-	watch = audit_watch_get(data->watch);
-	read_unlock(&data->lock);
-	if (!watch)
+	if (hlist_empty(&data->watches))
 		goto out;
+
+	ret = auditfs_attach_wdata(inode, &data->watches, mask);
 
-	if (mask && (watch->w_perms && !(watch->w_perms & mask)))
-		goto fail;
-
-	ret = auditfs_attach_wdata(inode, watch, mask);
-
-	if (ret) {
-	fail:
-		audit_watch_put(watch);
-	}
- out:
+out:
 	audit_data_put(data);
 	return ret;
 }
+
diff -Nurp linux-2.6.9-prep/kernel/auditsc.c linux-2.6.9~53+tim/kernel/auditsc.c
--- linux-2.6.9-prep/kernel/auditsc.c	2011-06-05 20:45:42.000000000 -0500
+++ linux-2.6.9~53+tim/kernel/auditsc.c	2011-06-04 02:04:22.000000000 -0500
@@ -131,7 +131,7 @@ struct audit_aux_data_path {
 
 struct audit_aux_data_watched {
 	struct audit_aux_data	link;
-	struct audit_watch	*watch;
+	struct hlist_head	watches;
 	unsigned long		ino;
 	int			mask;
 	uid_t			uid;
@@ -576,6 +576,8 @@ static inline void audit_free_names(stru
 static inline void audit_free_aux(struct audit_context *context)
 {
 	struct audit_aux_data *aux;
+	struct audit_watch_info *winfo;
+	struct hlist_node *pos, *tmp;
 
 	while ((aux = context->aux)) {
 		switch(aux->type) {
@@ -586,7 +588,11 @@ static inline void audit_free_aux(struct
 			break; }
 		case AUDIT_FS_WATCH: {
 			struct audit_aux_data_watched *axi = (void *)aux;
-			audit_watch_put(axi->watch);
+			hlist_for_each_entry_safe(winfo, pos, tmp, &axi->watches, node) {
+				audit_watch_put(winfo->watch);
+				hlist_del(&winfo->node);
+				kfree(winfo);
+                        }
 			break; }
 		}
 		
@@ -702,6 +708,8 @@ static void audit_log_exit(struct audit_
 	int i;
 	struct audit_buffer *ab;
 	struct audit_aux_data *aux;
+	struct audit_watch_info *winfo;
+	struct hlist_node *pos;
 
 	ab = audit_log_start(context, AUDIT_SYSCALL);
 	if (!ab)
@@ -768,17 +776,26 @@ static void audit_log_exit(struct audit_
 
 		case AUDIT_FS_WATCH: {
 			struct audit_aux_data_watched *axi = (void *)aux;
-			audit_log_format(ab, " watch=");
-			audit_log_untrustedstring(ab, axi->watch->w_name);
+			struct audit_buffer *sub_ab;
 			audit_log_format(ab,
-					 " filterkey=%s perm=%u perm_mask=%d"
-					 " inode=%lu inode_uid=%u inode_gid=%u"
+					 "inode=%lu inode_uid=%u inode_gid=%u"
 					 " inode_dev=%02x:%02x inode_rdev=%02x:%02x",
-					 axi->watch->w_filterkey,
-					 axi->watch->w_perms,
-					 axi->mask, axi->ino, axi->uid, axi->gid,
+					 axi->ino, axi->uid, axi->gid,
 					 MAJOR(axi->dev), MINOR(axi->dev),
 					 MAJOR(axi->rdev), MINOR(axi->rdev));
+			hlist_for_each_entry(winfo, pos, &axi->watches, node) {
+				sub_ab = audit_log_start(context, AUDIT_FS_WATCH);
+				if (!sub_ab)
+					return;		/* audit_panic has been called */
+				audit_log_format(sub_ab, "watch_inode=%lu", axi->ino);
+				audit_log_format(sub_ab, " watch=");
+				audit_log_untrustedstring(sub_ab, winfo->watch->w_name);
+				audit_log_format(sub_ab,
+						 " filterkey=%s perm=%u perm_mask=%u",
+						 winfo->watch->w_filterkey,
+						 winfo->watch->w_perms, axi->mask);
+				audit_log_end(sub_ab);
+			}
 			break; }
 		}
 		audit_log_end(ab);
@@ -1198,11 +1215,14 @@ void audit_signal_info(int sig, struct t
 #ifdef CONFIG_AUDITFILESYSTEM
 /* This has to be here instead of in auditfs.c, because it needs to
    see the audit context */
-int auditfs_attach_wdata(struct inode *inode, struct audit_watch *watch,
+int auditfs_attach_wdata(struct inode *inode, struct hlist_head *watches,
 			 int mask)
 {
 	struct audit_context *context = current->audit_context;
 	struct audit_aux_data_watched *ax;
+	struct audit_watch *watch;
+	struct audit_watch_info *this, *winfo;
+	struct hlist_node *pos, *tmp;
 
 	ax = kmalloc(sizeof(*ax), GFP_KERNEL);
 	if (!ax)
@@ -1211,7 +1231,18 @@ int auditfs_attach_wdata(struct inode *i
 	if (context->in_syscall && !context->auditable)
 		context->auditable = 1;
 
-	ax->watch = watch;
+	INIT_HLIST_HEAD(&ax->watches);
+
+	hlist_for_each_entry(watch, pos, watches, w_watched) {
+		winfo = kmalloc(sizeof(struct audit_watch_info), GFP_KERNEL);
+		if (!winfo)
+			goto auditfs_attach_wdata_fail;
+ 		if (mask && (watch->w_perms && !(watch->w_perms&mask)))
+			continue;
+		winfo->watch = audit_watch_get(watch);
+		hlist_add_head(&winfo->node, &ax->watches);
+	}
+
 	ax->mask = mask;
 	ax->ino = inode->i_ino;
 	ax->uid = inode->i_uid;
@@ -1224,5 +1255,13 @@ int auditfs_attach_wdata(struct inode *i
 	context->aux = (void *)ax;
 
 	return 0;
+
+auditfs_attach_wdata_fail:
+	hlist_for_each_entry_safe(this, pos, tmp, &ax->watches, node) {
+		hlist_del(&this->node);
+		kfree(this);
+	}
+
+	return -ENOMEM;
 }
 #endif /* CONFIG_AUDITFILESYSTEM */

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]