[PATCH] fix deadlocks in AUDIT_LIST/AUDIT_LIST_RULES

We should not send a pile of replies while holding audit_netlink_mutex
since we hold the same mutex when we receive commands.  As the result,
we can get blocked while sending and sit there holding the mutex while
auditctl is unable to send the next command and get around to receiving
what we'd sent.

Solution: create skb and put them into a queue instead of sending;
once we are done, send what we've got on the list.  The former can
be done synchronously while we are handling AUDIT_LIST or AUDIT_LIST_RULES;
we are holding audit_netlink_mutex at that point.  The latter is done
asynchronously and without messing with audit_netlink_mutex.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/kernel/audit.c b/kernel/audit.c
index df57b493e1cb2c35a8d6fd7f4602751f2ac7fd6d..bf74bf02aa4be262d40f9737aff28b996a66ca47 100644 (file)
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -366,6 +366,50 @@ 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;
+
+       /* wait for parent to finish and send an ACK */
+       mutex_lock(&audit_netlink_mutex);
+       mutex_unlock(&audit_netlink_mutex);
+
+       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
@@ -383,29 +427,13 @@ void audit_send_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);
+       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
index 6f733920fd32e1cc88fe6840620720ec8cb62978..8948fc1e9e5442deea3c04e77132bcf44a58d8e7 100644 (file)
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -22,6 +22,7 @@
 #include <linux/mutex.h>
 #include <linux/fs.h>
 #include <linux/audit.h>
+#include <linux/skbuff.h>
 
 /* 0 = no checking
    1 = put_count checking
@@ -82,6 +83,9 @@ struct audit_entry {
 extern int audit_pid;
 extern int audit_comparator(const u32 left, const u32 op, const u32 right);
 
+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);
@@ -89,4 +93,11 @@ extern void              audit_log_lost(const char *message);
 extern void                audit_panic(const char *message);
 extern struct mutex audit_netlink_mutex;
 
+struct audit_netlink_list {
+       int pid;
+       struct sk_buff_head q;
+};
+
+int audit_send_list(void *);
+
 extern int selinux_audit_rule_update(void);

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 7c134906d689c8af18397351089dc01fbbf00461..ccfea6d82cc37049c1f15a3d71e48c75da7e5ac9 100644 (file)
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -510,19 +510,12 @@ static inline int audit_del_rule(struct audit_entry *entry,
 
 /* 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_netlink_mutex);
-
        /* The *_rcu iterators not needed here because we are
           always called with audit_netlink_mutex held. */
        for (i=0; i<AUDIT_NR_FILTERS; i++) {
@@ -532,31 +525,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_netlink_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_netlink_mutex);
-
        /* The *_rcu iterators not needed here because we are
           always called with audit_netlink_mutex held. */
        for (i=0; i<AUDIT_NR_FILTERS; i++) {
@@ -566,15 +553,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));
+                       if (skb)
+                               skb_queue_tail(q, skb);
                        kfree(data);
                }
        }
-       audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
-
-       mutex_unlock(&audit_netlink_mutex);
-       return 0;
+       skb = audit_make_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
+       if (skb)
+               skb_queue_tail(q, skb);
 }
 
 /**
@@ -592,7 +580,7 @@ int audit_receive_filter(int type, int pid, int uid, int seq, void *data,
                         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;
 
@@ -605,18 +593,20 @@ int audit_receive_filter(int type, int pid, int uid, int seq, void *data,
                 * 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);
 
                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);
+
+               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);
                }