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

[PATCH] bug fixes + cleanups against audit.58



Hello,

This patch introduces some more bug fixes which hopefully address Rob's 
Oopses.  

I've made sure to call hlist_del_init instead of hlist_del where necessary 
(ie: audit_update_watch).  

I've also scrapped the audit_is_watched function in favor of !hlist_unhashed.  
One other change was putting the lock around the entire list traversal in 
audit_drain_watchlist, rather then in it.  This was just to make the locking 
more uniform (ie: if auditfs_lock is held, whatever is holding it has 
complete control).

I also rearranged auditfs_wdata_attach a bit, such that if there is no 
context, we return out of the function.  I've also made it so the context is 
only made auditable if it makes it past the memory allocation.  No sense in 
making it auditable if we were able to collect audit info.

I added audit_notify_watch hooks to fs/open.c and fs/attr.c

Some other little odds and ends are cleanup related.  I removed the item= 
field from the PATH record component, I turned audit_notify_watch and 
auditfs_attach_wdata into void functions as we're unable to handle failed 
memory allocations.  Reduced my goto abuse.

-tim

diff -Nurp linux-2.6.9~58~orig/fs/attr.c linux-2.6.9~58~tc1/fs/attr.c
--- linux-2.6.9~58~orig/fs/attr.c	2004-10-18 16:53:21.000000000 -0500
+++ linux-2.6.9~58~tc1/fs/attr.c	2005-06-17 18:18:04.000000000 -0500
@@ -14,6 +14,7 @@
 #include <linux/fcntl.h>
 #include <linux/quotaops.h>
 #include <linux/security.h>
+#include <linux/audit.h>
 
 /* Taken over from the old code... */
 
@@ -68,6 +69,8 @@ int inode_setattr(struct inode * inode, 
 	unsigned int ia_valid = attr->ia_valid;
 	int error = 0;
 
+	audit_notify_watch(inode, MAY_WRITE);	
+
 	if (ia_valid & ATTR_SIZE) {
 		if (attr->ia_size != i_size_read(inode)) {
 			error = vmtruncate(inode, attr->ia_size);
diff -Nurp linux-2.6.9~58~orig/fs/open.c linux-2.6.9~58~tc1/fs/open.c
--- linux-2.6.9~58~orig/fs/open.c	2005-06-17 16:13:58.000000000 -0500
+++ linux-2.6.9~58~tc1/fs/open.c	2005-06-17 19:02:09.628924928 -0500
@@ -22,6 +22,7 @@
 #include <asm/uaccess.h>
 #include <linux/fs.h>
 #include <linux/pagemap.h>
+#include <linux/audit.h>
 
 #include <asm/unistd.h>
 
@@ -610,6 +611,8 @@ asmlinkage long sys_fchmod(unsigned int 
 
 	dentry = file->f_dentry;
 	inode = dentry->d_inode;
+	
+	audit_notify_watch(inode, MAY_WRITE);
 
 	err = -EROFS;
 	if (IS_RDONLY(inode))
@@ -642,6 +645,8 @@ asmlinkage long sys_chmod(const char __u
 	if (error)
 		goto out;
 	inode = nd.dentry->d_inode;
+	
+	audit_notify_watch(inode, MAY_WRITE);
 
 	error = -EROFS;
 	if (IS_RDONLY(inode))
@@ -676,6 +681,7 @@ static int chown_common(struct dentry * 
 		printk(KERN_ERR "chown_common: NULL inode\n");
 		goto out;
 	}
+	audit_notify_watch(inode, MAY_WRITE);
 	error = -EROFS;
 	if (IS_RDONLY(inode))
 		goto out;
diff -Nurp linux-2.6.9~58~orig/include/linux/audit.h linux-2.6.9~58~tc1/include/linux/audit.h
--- linux-2.6.9~58~orig/include/linux/audit.h	2005-06-17 16:13:58.000000000 -0500
+++ linux-2.6.9~58~tc1/include/linux/audit.h	2005-06-17 18:18:04.000000000 -0500
@@ -308,9 +308,9 @@ extern void audit_inode_free(struct inod
 extern void audit_update_watch(struct dentry *dentry, int remove);
 extern void audit_watch_put(struct audit_watch *watch);
 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 hlist_head *watches,
-				int mask);
+extern void audit_notify_watch(struct inode *inode, int mask);
+extern void auditfs_attach_wdata(struct inode *inode, struct hlist_head *watches,
+				 int mask);
 #else
 #define audit_filesystem_init() ({ 0; })
 #define audit_list_watches(p,s) ({ -EOPNOTSUPP; })
@@ -319,7 +319,7 @@ extern int auditfs_attach_wdata(struct i
 #define audit_update_watch(d,r) do { ; } while (0)
 #define audit_watch_put(w) do { ; } while(0)
 #define audit_watch_get(w) ({ 0; })
-#define audit_notify_watch(i,m) ({ 0; })
+#define audit_notify_watch(i,m) do { ; } while(0)
 #endif
 
 #ifdef CONFIG_AUDIT
diff -Nurp linux-2.6.9~58~orig/kernel/auditfs.c linux-2.6.9~58~tc1/kernel/auditfs.c
--- linux-2.6.9~58~orig/kernel/auditfs.c	2005-06-17 16:13:58.000000000 -0500
+++ linux-2.6.9~58~tc1/kernel/auditfs.c	2005-06-17 19:03:44.316530224 -0500
@@ -230,7 +230,7 @@ static inline struct audit_watch *audit_
 
 	watch = audit_watch_alloc();
 	if (!watch)
-		goto audit_to_watch_exit;
+		goto audit_to_watch_fail;
 
 	t = memblk;
 
@@ -254,7 +254,6 @@ static inline struct audit_watch *audit_
 
 audit_to_watch_fail:
 	audit_watch_free(watch);
-audit_to_watch_exit:
 	return NULL;
 }
 
@@ -316,13 +315,13 @@ static inline void audit_drain_watchlist
 	struct audit_watch *watch;
 	struct hlist_node *pos, *tmp;
 
+	spin_lock(&auditfs_lock);
 	hlist_for_each_entry_safe(watch, pos, tmp, &data->watchlist, w_node) {
-		spin_lock(&auditfs_lock);
 		audit_destroy_watch(watch);
-		spin_unlock(&auditfs_lock);
 		audit_data_pool_shrink();
 		audit_log(NULL, AUDIT_CONFIG_CHANGE, "auid=%u removed watch implicitly", -1);
 	}
+	spin_unlock(&auditfs_lock);
 }
 
 static void audit_data_unhash(struct audit_inode_data *data)
@@ -498,23 +497,6 @@ audit_remove_watch_exit:
 	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;
@@ -557,7 +539,7 @@ void audit_watch_put(struct audit_watch 
  */
 void audit_update_watch(struct dentry *dentry, int remove)
 {
-	struct audit_watch *me, *this, *watch;
+	struct audit_watch *this, *watch;
 	struct audit_inode_data *data, *parent;
 	struct hlist_node *pos, *tmp;
 
@@ -590,24 +572,20 @@ void audit_update_watch(struct dentry *d
 
 	spin_lock(&auditfs_lock);
 	if (remove) {
-		me = audit_is_watched(watch, data);
-		if (me) {
-			hlist_del(&watch->w_watched);
+		if (watch && !hlist_unhashed(&watch->w_watched)) {
+			hlist_del_init(&watch->w_watched);
 			audit_watch_put(watch);
-			audit_watch_put(me);
 		}
 	} else {
 		hlist_for_each_entry_safe(this, pos, tmp, &data->watches, w_watched)
 			if (hlist_unhashed(&this->w_node)) {
-				hlist_del(&this->w_watched);
+				hlist_del_init(&this->w_watched);
 				audit_watch_put(this);
 			}
-		me = audit_is_watched(watch, data);
-		if (!me && watch) {
+		if (watch && hlist_unhashed(&watch->w_watched)) {
 			audit_watch_get(watch);
 			hlist_add_head(&watch->w_watched, &data->watches);
 		}
-		audit_watch_put(me);
 	}
 	spin_unlock(&auditfs_lock);
 	audit_data_put(data);
@@ -675,8 +653,10 @@ int audit_list_watches(int pid, int seq)
 		entry = audit_to_skb(watch);
 		if (IS_ERR(entry)) {
 			ret = PTR_ERR(entry);
+			audit_watch_put(watch);
 			goto audit_list_watches_fail;
 		}
+
 		hlist_add_head(&entry->list, &skb_list);
 		spin_lock(&auditfs_lock);
 		if (hlist_unhashed(&watch->w_master)) {
@@ -693,6 +673,7 @@ int audit_list_watches(int pid, int seq)
 				kfree(entry->memblk);
 				kfree(entry);
 			}
+			spin_unlock(&auditfs_lock);
 			goto restart;
 		}
 		audit_watch_put(watch);
@@ -772,9 +753,6 @@ void audit_inode_free(struct inode *inod
 	struct hlist_node *pos, *tmp;
 	struct audit_inode_data *data = audit_data_get(inode, 0);
 
-	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);
-
 	if (data) {
 		spin_lock(&auditfs_hash_lock);
 		audit_data_unhash(data);
@@ -828,28 +806,26 @@ audit_filesystem_init_fail:
 }
 
 
-int audit_notify_watch(struct inode *inode, int mask)
+void audit_notify_watch(struct inode *inode, int mask)
 {
-	int ret = 0;
 	struct audit_inode_data *data;
 
 	if (likely(!audit_enabled))
-		return 0;
+		return;
 
 	if (!inode || !current->audit_context)
-		return 0;
+		return;
 
 	data = audit_data_get(inode, 0);
 	if (!data)
-		return 0;
+		return;
 
 	if (hlist_empty(&data->watches))
 		goto out;
 
-	ret = auditfs_attach_wdata(inode, &data->watches, mask);
+	auditfs_attach_wdata(inode, &data->watches, mask);
 
 out:
 	audit_data_put(data);
-	return ret;
 }
 
diff -Nurp linux-2.6.9~58~orig/kernel/auditsc.c linux-2.6.9~58~tc1/kernel/auditsc.c
--- linux-2.6.9~58~orig/kernel/auditsc.c	2005-06-17 16:13:58.000000000 -0500
+++ linux-2.6.9~58~tc1/kernel/auditsc.c	2005-06-17 18:18:04.000000000 -0500
@@ -813,9 +813,8 @@ static void audit_log_exit(struct audit_
 		if (!ab)
 			continue; /* audit_panic has been called */
 
-		audit_log_format(ab, "item=%d", i);
 		if (context->names[i].name) {
-			audit_log_format(ab, " name=");
+			audit_log_format(ab, "name=");
 			audit_log_untrustedstring(ab, context->names[i].name);
 		}
 		if (context->names[i].ino != (unsigned long)-1)
@@ -1215,7 +1214,7 @@ 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 hlist_head *watches,
+void auditfs_attach_wdata(struct inode *inode, struct hlist_head *watches,
 			 int mask)
 {
 	struct audit_context *context = current->audit_context;
@@ -1224,12 +1223,12 @@ int auditfs_attach_wdata(struct inode *i
 	struct audit_watch_info *this, *winfo;
 	struct hlist_node *pos, *tmp;
 
+	if (!context)
+		return;
+
 	ax = kmalloc(sizeof(*ax), GFP_KERNEL);
 	if (!ax)
-		return -ENOMEM;
-
-	if (context->in_syscall && !context->auditable)
-		context->auditable = 1;
+		return;
 
 	INIT_HLIST_HEAD(&ax->watches);
 
@@ -1243,6 +1242,9 @@ int auditfs_attach_wdata(struct inode *i
 		hlist_add_head(&winfo->node, &ax->watches);
 	}
 
+	if (context->in_syscall && !context->auditable)
+		context->auditable = 1;
+	
 	ax->mask = mask;
 	ax->ino = inode->i_ino;
 	ax->uid = inode->i_uid;
@@ -1253,8 +1255,7 @@ int auditfs_attach_wdata(struct inode *i
 	ax->link.type = AUDIT_FS_INODE;
 	ax->link.next = context->aux;
 	context->aux = (void *)ax;
-
-	return 0;
+	return;
 
 auditfs_attach_wdata_fail:
 	hlist_for_each_entry_safe(this, pos, tmp, &ax->watches, node) {
@@ -1263,7 +1264,6 @@ auditfs_attach_wdata_fail:
 		kfree(this);
 	}
 	kfree(ax);
-
-	return -ENOMEM;
 }
+
 #endif /* CONFIG_AUDITFILESYSTEM */

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