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

Re: audit.53 kernel



On Thu, 2005-06-02 at 17:46 -0400, Steve Grubb wrote:
> The other problem is an oops. It only occurs if auditing is enabled
> during shutdown. When auditing is disabled, you get the inodes busy
> message.

They're both manifestations of the same problem -- I don't think we were
unpinning dentries at all on unmount. I think we can ditch the dentry
pinning scheme, and rely on the implicit pinning which we get from using
a bit in inode->i_state to note the presence of audit data in the hash
table for the inode.

This will mean that prune_icache() will leave the inode alone, because
it merely checks whether i_state is zero or not. 

I'm playing with something like this... note the conditional debugging
in audit_inode_free(), because it's too noisy if it does it for all file
systems. You might want to change the block device it prints messages
for, if you aren't testing on /dev/hda6 as I am.

--- kernel-2.6.9-running//linux-2.6.9/fs/inode.c	2005-06-03 14:14:18.000000000 +0100
+++ kernel-2.6.9/linux-2.6.9/fs/inode.c	2005-06-03 11:50:24.000000000 +0100
@@ -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_AUDIT);
+	inode->i_state = I_CLEAR;
 }
 
 EXPORT_SYMBOL(clear_inode);
--- kernel-2.6.9-running//linux-2.6.9/include/linux/fs.h	2005-06-03 12:36:12.000000000 +0100
+++ kernel-2.6.9/linux-2.6.9/include/linux/fs.h	2005-06-03 11:50:24.000000000 +0100
@@ -986,7 +986,6 @@ 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)
 
--- kernel-2.6.9-running//linux-2.6.9/kernel/auditfs.c	2005-06-03 14:03:50.000000000 +0100
+++ kernel-2.6.9/linux-2.6.9/kernel/auditfs.c	2005-06-03 11:50:24.000000000 +0100
@@ -36,7 +36,7 @@
 #include <linux/module.h>
 #include <asm/uaccess.h>
 
-#if 0
+#if 1
 #define dprintk(...) do { } while(0)
 #define __print_symbol(x, y) do { } while(0)
 #else
@@ -58,7 +58,7 @@ struct audit_skb_list {
 	void *memblk;
 	size_t size;
 };
-extern spinlock_t inode_lock;
+
 
 static int audit_nr_watches;
 static int audit_pool_size;
@@ -106,9 +106,7 @@ static struct audit_inode_data *audit_da
 
 	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) */
-			
+
 	while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) {
 		dprintk("list %p -> %p\n", list, *list);
 		list = &(*list)->next_hash;
@@ -116,12 +114,6 @@ static struct audit_inode_data *audit_da
 	if (*list && (*list)->inode == inode)
 		ret = *list;
 
-	if (ret && !(inode->i_state & I_AUDIT)) {
-		printk("Eep. Inode #%lu has data %p but not I_AUDIT\n",
-		       inode->i_ino, ret);
-	} else if (!ret && (inode->i_state & I_AUDIT)) {
-		printk("Eep. Inode #%lu has I_AUDIT but not data\n", inode->i_ino);
-	}
 	if (ret) {
 		ret->count++;
 	} else if (allocate) {
@@ -134,9 +126,6 @@ static struct audit_inode_data *audit_da
 		ret->watch = NULL;
 		ret->lock = RW_LOCK_UNLOCKED;
 		ret->inode = inode;
-		spin_lock(&inode_lock);
-		inode->i_state |= I_AUDIT;
-		spin_unlock(&inode_lock);
 		ret->next_hash = *list;
 		ret->count = 2;
 		*list = ret;
@@ -146,7 +135,6 @@ static struct audit_inode_data *audit_da
 		       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;
@@ -157,26 +145,18 @@ static struct audit_inode_data *audit_da
 /* Unpin the dentry stored at watch->w_dentry. */
 static inline void audit_unpin(struct audit_watch *watch)
 {
-#if 0
 	if (watch && watch->w_dentry) {
-		printk("unpin dentry %s\n", watch->w_dentry->d_name.name);
-		BUG_ON(atomic_read(&watch->w_dentry->d_count) == 1);
 		dput(watch->w_dentry);
 		watch->w_dentry = NULL;
 	}
-#endif
 }
 
 /* Pin the dentry and store it at watch->w_dentry. */
 static inline void audit_pin(struct audit_watch *watch,
 			     struct dentry *dentry)
 {
-#if 0
-	if (watch && !watch->w_dentry) {
+	if (watch && !watch->w_dentry)
 		watch->w_dentry = dget(dentry);
-		printk("pin dentry %s\n", dentry->d_name.name);
-	}
-#endif
 }
 
 static inline struct audit_watch *audit_watch_fetch(const char *name,
@@ -414,24 +394,6 @@ static inline void audit_drain_watchlist
 	}
 }
 
-static void audit_data_unhash(struct audit_inode_data *data)
-{
-	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;
-
-	spin_lock(&inode_lock);
-	data->inode->i_state &= ~I_AUDIT;
-	spin_unlock(&inode_lock);
-	data->inode = NULL;
-}
-
-
 static void audit_data_put(struct audit_inode_data *data)
 {
 	if (!data)
@@ -452,13 +414,22 @@ 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 */
-		if (data->inode)
-			audit_data_unhash(data);
+		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;
+
 		spin_unlock(&auditfs_hash_lock);
 
 		audit_drain_watchlist(data);
-
 		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);
@@ -470,6 +441,7 @@ static void audit_data_put(struct audit_
 			kfree(data);
 		}
 	}
+
 	spin_unlock(&auditfs_hash_lock);
 }
 
@@ -564,8 +536,7 @@ struct audit_watch *audit_watch_get(stru
 {
 	if (watch) {
 		BUG_ON(!atomic_read(&watch->w_count));
-		printk("Increase count on watch %p to %d\n",
-		       watch, atomic_inc_return(&watch->w_count));
+		atomic_inc(&watch->w_count);
 	}
 
 	return watch;
@@ -573,15 +544,8 @@ struct audit_watch *audit_watch_get(stru
 
 void audit_watch_put(struct audit_watch *watch)
 {
-	int new;
-
-	if (watch) {
-		new = atomic_dec_return(&watch->w_count);
-		if (!new)
-			audit_watch_free(watch);
-		printk("Reduce count on watch %p to %d\n",
-		       watch, new);
-	}
+	if (watch && atomic_dec_and_test(&watch->w_count))
+		audit_watch_free(watch);
 }
 
 /*
@@ -806,14 +770,7 @@ void audit_inode_free(struct inode *inod
 {
 	struct audit_inode_data *data = audit_data_get(inode, 0);
 
-	if (inode->i_sb->s_dev == MKDEV(3,6))
-		printk("Put inode #%lu, data %p\n", inode->i_ino, data);
-
 	if (data) {
-		spin_lock(&auditfs_hash_lock);
-		audit_data_unhash(data);
-		spin_unlock(&auditfs_hash_lock);
-		
 		audit_drain_watchlist(data);
 		audit_watch_put(data->watch);
 		data->watch = NULL;
@@ -830,7 +787,6 @@ void audit_inode_free(struct inode *inod
  */
 void audit_dentry_unpin(struct dentry *dentry)
 {
-#if 0
 	struct audit_inode_data *parent;
 	struct audit_watch *watch;
 
@@ -849,7 +805,6 @@ void audit_dentry_unpin(struct dentry *d
 		audit_watch_put(watch);
 	}
 	audit_data_put(parent);
-#endif
 }
 
 int audit_filesystem_init(void)


-- 
dwmw2


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