audit.60 kernel (Re: [PATCH] bug fixes + cleanups against audit.58)

[David Woodhouse]

On Sun, 2005-06-19 at 19:29 +0100, David Woodhouse wrote:
> Tim, what's the locking strategy here? Why is auditfs_attach_wdata()
> allowed to walk the watch list as it does, allocating memory and
> potentially sleeping between each item?

To answer the rhetorical question...

--- linux-2.6.9/kernel/auditsc.c~	2005-06-19 20:28:08.000000000 +0100
+++ linux-2.6.9/kernel/auditsc.c	2005-06-19 23:13:33.000000000 +0100
@@ -1232,6 +1232,8 @@ void audit_signal_info(int sig, struct t
 }
 
 #ifdef CONFIG_AUDITFILESYSTEM
+extern spinlock_t auditfs_lock;
+
 /* This has to be here instead of in auditfs.c, because it needs to
    see the audit context */
 void auditfs_attach_wdata(struct inode *inode, struct hlist_head *watches,
@@ -1252,7 +1254,11 @@ void auditfs_attach_wdata(struct inode *
 
 	INIT_HLIST_HEAD(&ax->watches);
 
+	spin_lock(&auditfs_lock);
 	hlist_for_each_entry(watch, pos, watches, w_watched) {
+	restart:
+		audit_watch_get(watch);
+		spin_unlock(&auditfs_lock);
 		winfo = kmalloc(sizeof(struct audit_watch_info), GFP_KERNEL);
 		if (!winfo)
 			goto auditfs_attach_wdata_fail;
@@ -1260,7 +1266,25 @@ void auditfs_attach_wdata(struct inode *
 			continue;
 		winfo->watch = audit_watch_get(watch);
 		hlist_add_head(&winfo->node, &ax->watches);
+		spin_lock(&auditfs_lock);
+		if (hlist_unhashed(&watch->w_watched)) {
+			audit_watch_put(watch);
+			/* Someone took it off the list while we didn't have it locked.
+			   Go through the list of watches again until we find one which 
+			   we haven't already dealt with... */
+			hlist_for_each_entry(watch, pos, watches, w_watched) {
+				hlist_for_each_entry(winfo, tmp, &ax->watches, node) {
+					if (winfo->watch == watch)
+						continue;
+				}
+				/* This watch wasn't found on ax's list, so
+				   pick up where we left off. */
+				goto restart;
+			}
+		}
+		audit_watch_put(watch);
 	}
+	spin_unlock(&auditfs_lock);
 
 	if (context->in_syscall && !context->auditable &&
 		 AUDIT_DISABLED != audit_filter_syscall(current, context,
--- linux-2.6.9/kernel/auditfs.c~	2005-06-19 18:53:58.000000000 +0100
+++ linux-2.6.9/kernel/auditfs.c	2005-06-19 22:27:41.000000000 +0100
@@ -52,7 +52,7 @@ extern int audit_enabled;
 static kmem_cache_t *audit_watch_cache;
 
 static HLIST_HEAD(master_watchlist);
-static spinlock_t auditfs_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t auditfs_lock = SPIN_LOCK_UNLOCKED;
 
 struct audit_skb_list {
 	struct hlist_node list;


-- 
dwmw2