From mboxrd@z Thu Jan 1 00:00:00 1970 From: "George C. Wilson" Subject: Subject: [PATCH] Fix audit_list{,_rules} deadlock Date: Tue, 25 Apr 2006 16:16:44 -0500 Message-ID: <20060425211644.GA18366@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mx3.redhat.com (mx3.redhat.com [172.16.48.32]) by int-mx1.corp.redhat.com (8.12.11.20060308/8.11.6) with ESMTP id k3PLGxAP023430 for ; Tue, 25 Apr 2006 17:16:59 -0400 Received: from e34.co.us.ibm.com (e34.co.us.ibm.com [32.97.110.152]) by mx3.redhat.com (8.13.1/8.13.1) with ESMTP id k3PLGqSi011168 for ; Tue, 25 Apr 2006 17:16:52 -0400 Received: from westrelay02.boulder.ibm.com (westrelay02.boulder.ibm.com [9.17.195.11]) by e34.co.us.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id k3PLGlh5001846 for ; Tue, 25 Apr 2006 17:16:47 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by westrelay02.boulder.ibm.com (8.12.10/NCO/VER6.8) with ESMTP id k3PLCxs4266450 for ; Tue, 25 Apr 2006 15:12:59 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.12.11/8.13.3) with ESMTP id k3PLGl0f028646 for ; Tue, 25 Apr 2006 15:16:47 -0600 Received: from us.ibm.com (ea.austin.ibm.com [9.53.40.26]) by d03av02.boulder.ibm.com (8.12.11/8.12.11) with ESMTP id k3PLGkX7028531 for ; Tue, 25 Apr 2006 15:16:47 -0600 Received: (from gcwilson@localhost) by us.ibm.com (8.13.4/8.13.4/Submit) id k3PLGiBD018397 for linux-audit@redhat.com; Tue, 25 Apr 2006 16:16:44 -0500 Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com List-Id: linux-audit@redhat.com Following is Serge's patch: From: Serge Hallyn Subject: [PATCH] Fix audit_list{,_rules} deadlock The audit_list and audit_list_rules functions send all stored audit rules to userspace over netlink from a kthread. They do this entirely from under the audit_netlink_mutex. If the netlink buffer gets to being too large, then netlink will wait for the receiver to grab the contents. But the receiver is in audit_receive, which also grabs the audit_netlink_mutex. Deadlock. This patch makes audit_list{,_rules} form a list of rules under the mutex, then drop the mutex, then send the list. Signed-off-by: Serge Hallyn --- kernel/auditfilter.c | 54 +++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 46 insertions(+), 8 deletions(-) diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 63aa039..b2e656f 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -510,12 +510,22 @@ static inline int audit_del_rule(struct /* List rules using struct audit_rule. Exists for backward * compatibility with userspace. */ +struct au_rule_list { + union { + struct audit_rule *rule; + struct audit_rule_data *data; + } u; + struct list_head next; +}; + static int audit_list(void *_dest) { int pid, seq; int *dest = _dest; struct audit_entry *entry; int i; + LIST_HEAD(sendlist); + struct au_rule_list *r, *r1; pid = dest[0]; seq = dest[1]; @@ -532,14 +542,27 @@ 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, - rule, sizeof(*rule)); - kfree(rule); + r = kmalloc(sizeof(*r), GFP_KERNEL); + if (unlikely(!r)) { + kfree(rule); + break; + } + r->u.rule = rule; + list_add(&r->next, &sendlist); } } - audit_send_reply(pid, seq, AUDIT_LIST, 1, 1, NULL, 0); mutex_unlock(&audit_netlink_mutex); + + list_for_each_entry_safe(r, r1, &sendlist, next) { + audit_send_reply(pid, seq, AUDIT_LIST, 0, 1, + r->u.rule, sizeof(*r->u.rule)); + list_del(&r->next); + kfree(r->u.rule); + kfree(r); + } + + audit_send_reply(pid, seq, AUDIT_LIST, 1, 1, NULL, 0); return 0; } @@ -549,6 +572,8 @@ static int audit_list_rules(void *_dest) int pid, seq; int *dest = _dest; struct audit_entry *e; + LIST_HEAD(sendlist); + struct au_rule_list *r, *r1; int i; pid = dest[0]; @@ -566,14 +591,27 @@ 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, - data, sizeof(*data)); - kfree(data); + r = kmalloc(sizeof(*r), GFP_KERNEL); + if (unlikely(!r)) { + kfree(data); + break; + } + r->u.data = data; + list_add(&r->next, &sendlist); } } + mutex_unlock(&audit_netlink_mutex); + + list_for_each_entry_safe(r, r1, &sendlist, next) { + audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1, + r->u.data, sizeof(*r->u.data)); + list_del(&r->next); + kfree(r->u.data); + kfree(r); + } + audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0); - mutex_unlock(&audit_netlink_mutex); return 0; } -- George Wilson IBM Linux Technology Center