Re: deadlock fixes

[Alexander Viro <aviro@redhat.com>]

On Thu, Apr 27, 2006 at 07:27:25AM -0400, Alexander Viro wrote:
> handling, as discussed on IRC yesterday.

Patch in question (thanks to Steve for reminding to send it to list):

diff --git a/kernel/audit.c b/kernel/audit.c
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -368,6 +368,46 @@ static int kauditd_thread(void *dummy)
 	return 0;
 }
 
+int audit_send_list(void *_dest)
+{
+	struct audit_netlink_list *dest = _dest;
+	int pid = dest->pid;
+	struct sk_buff *skb;
+
+	while ((skb = __skb_dequeue(&dest->q)) != NULL)
+		netlink_unicast(audit_sock, skb, pid, 0);
+
+	kfree(dest);
+
+	return 0;
+}
+
+struct sk_buff *audit_make_reply(int pid, int seq, int type, int done,
+				 int multi, void *payload, int size)
+{
+	struct sk_buff	*skb;
+	struct nlmsghdr	*nlh;
+	int		len = NLMSG_SPACE(size);
+	void		*data;
+	int		flags = multi ? NLM_F_MULTI : 0;
+	int		t     = done  ? NLMSG_DONE  : type;
+
+	skb = alloc_skb(len, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+
+	nlh		 = NLMSG_PUT(skb, pid, seq, t, size);
+	nlh->nlmsg_flags = flags;
+	data		 = NLMSG_DATA(nlh);
+	memcpy(data, payload, size);
+	return skb;
+
+nlmsg_failure:			/* Used by NLMSG_PUT */
+	if (skb)
+		kfree_skb(skb);
+	return NULL;
+}
+
 /**
  * audit_send_reply - send an audit reply message via netlink
  * @pid: process id to send reply to
@@ -385,29 +425,13 @@ void audit_send_reply(int pid, int seq, 
 		      void *payload, int size)
 {
 	struct sk_buff	*skb;
-	struct nlmsghdr	*nlh;
-	int		len = NLMSG_SPACE(size);
-	void		*data;
-	int		flags = multi ? NLM_F_MULTI : 0;
-	int		t     = done  ? NLMSG_DONE  : type;
-
-	skb = alloc_skb(len, GFP_KERNEL);
+	skb = audit_make_reply(pid, seq, type, done, multi, payload, size);
 	if (!skb)
 		return;
-
-	nlh		 = NLMSG_PUT(skb, pid, seq, t, size);
-	nlh->nlmsg_flags = flags;
-	data		 = NLMSG_DATA(nlh);
-	memcpy(data, payload, size);
-
 	/* Ignore failure. It'll only happen if the sender goes away,
 	   because our timeout is set to infinite. */
 	netlink_unicast(audit_sock, skb, pid, 0);
 	return;
-
-nlmsg_failure:			/* Used by NLMSG_PUT */
-	if (skb)
-		kfree_skb(skb);
 }
 
 /*
diff --git a/kernel/audit.h b/kernel/audit.h
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -95,12 +95,22 @@ struct audit_entry {
 extern int audit_pid;
 extern int audit_comparator(const u32 left, const u32 op, const u32 right);
 extern int audit_compare_dname_path(const char *dname, const char *path);
+extern struct sk_buff *	    audit_make_reply(int pid, int seq, int type,
+					     int done, int multi,
+					     void *payload, int size);
 extern void		    audit_send_reply(int pid, int seq, int type,
 					     int done, int multi,
 					     void *payload, int size);
 extern void		    audit_log_lost(const char *message);
 extern void		    audit_panic(const char *message);
 
+struct audit_netlink_list {
+	int pid;
+	struct sk_buff_head q;
+};
+
+int audit_send_list(void *);
+
 struct inotify_watch;
 extern void audit_handle_ievent(struct inotify_watch *, u32, u32, u32,
 				const char *, struct inode *);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1065,19 +1065,12 @@ static inline int audit_del_rule(struct 
 
 /* List rules using struct audit_rule.  Exists for backward
  * compatibility with userspace. */
-static int audit_list(void *_dest)
+static void audit_list(int pid, int seq, struct sk_buff_head *q)
 {
-	int pid, seq;
-	int *dest = _dest;
+	struct sk_buff *skb;
 	struct audit_entry *entry;
 	int i;
 
-	pid = dest[0];
-	seq = dest[1];
-	kfree(dest);
-
-	mutex_lock(&audit_filter_mutex);
-
 	/* This is a blocking read, so use audit_filter_mutex instead of rcu
 	 * iterator to sync with list writers. */
 	for (i=0; i<AUDIT_NR_FILTERS; i++) {
@@ -1087,31 +1080,25 @@ static int audit_list(void *_dest)
 			rule = audit_krule_to_rule(&entry->rule);
 			if (unlikely(!rule))
 				break;
-			audit_send_reply(pid, seq, AUDIT_LIST, 0, 1,
+			skb = audit_make_reply(pid, seq, AUDIT_LIST, 0, 1,
 					 rule, sizeof(*rule));
+			if (skb)
+				skb_queue_tail(q, skb);
 			kfree(rule);
 		}
 	}
-	audit_send_reply(pid, seq, AUDIT_LIST, 1, 1, NULL, 0);
-	
-	mutex_unlock(&audit_filter_mutex);
-	return 0;
+	skb = audit_make_reply(pid, seq, AUDIT_LIST, 1, 1, NULL, 0);
+	if (skb)
+		skb_queue_tail(q, skb);
 }
 
 /* List rules using struct audit_rule_data. */
-static int audit_list_rules(void *_dest)
+static void audit_list_rules(int pid, int seq, struct sk_buff_head *q)
 {
-	int pid, seq;
-	int *dest = _dest;
+	struct sk_buff *skb;
 	struct audit_entry *e;
 	int i;
 
-	pid = dest[0];
-	seq = dest[1];
-	kfree(dest);
-
-	mutex_lock(&audit_filter_mutex);
-
 	/* This is a blocking read, so use audit_filter_mutex instead of rcu
 	 * iterator to sync with list writers. */
 	for (i=0; i<AUDIT_NR_FILTERS; i++) {
@@ -1121,15 +1108,16 @@ static int audit_list_rules(void *_dest)
 			data = audit_krule_to_data(&e->rule);
 			if (unlikely(!data))
 				break;
-			audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
+			skb = audit_make_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
 					 data, sizeof(*data) + data->buflen);
+			if (skb)
+				skb_queue_tail(q, skb);
 			kfree(data);
 		}
 	}
-	audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
-
-	mutex_unlock(&audit_filter_mutex);
-	return 0;
+	skb = audit_make_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
+	if (skb)
+		skb_queue_tail(q, skb);
 }
 
 /**
@@ -1147,7 +1135,7 @@ int audit_receive_filter(int type, int p
 			 size_t datasz, uid_t loginuid, u32 sid)
 {
 	struct task_struct *tsk;
-	int *dest;
+	struct audit_netlink_list *dest;
 	int err = 0;
 	struct audit_entry *entry;
 
@@ -1160,18 +1148,22 @@ int audit_receive_filter(int type, int p
 		 * happen if we're actually running in the context of auditctl
 		 * trying to _send_ the stuff */
 		 
-		dest = kmalloc(2 * sizeof(int), GFP_KERNEL);
+		dest = kmalloc(sizeof(struct audit_netlink_list), GFP_KERNEL);
 		if (!dest)
 			return -ENOMEM;
-		dest[0] = pid;
-		dest[1] = seq;
+		dest->pid = pid;
+		skb_queue_head_init(&dest->q);
 
+		mutex_lock(&audit_filter_mutex);
 		if (type == AUDIT_LIST)
-			tsk = kthread_run(audit_list, dest, "audit_list");
+			audit_list(pid, seq, &dest->q);
 		else
-			tsk = kthread_run(audit_list_rules, dest,
-					  "audit_list_rules");
+			audit_list_rules(pid, seq, &dest->q);
+		mutex_unlock(&audit_filter_mutex);
+
+		tsk = kthread_run(audit_send_list, dest, "audit_send_list");
 		if (IS_ERR(tsk)) {
+			skb_queue_purge(&dest->q);
 			kfree(dest);
 			err = PTR_ERR(tsk);
 		}
-- 
0.99.9.GIT