From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752497AbdGSAKh (ORCPT ); Tue, 18 Jul 2017 20:10:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58384 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751560AbdGSAKf (ORCPT ); Tue, 18 Jul 2017 20:10:35 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2AC407DCE9 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=shuwang@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 2AC407DCE9 Date: Tue, 18 Jul 2017 20:10:31 -0400 (EDT) From: Shu Wang To: Paul Moore Cc: Eric Paris , linux-audit@redhat.com, linux-kernel@vger.kernel.org, liwang@redhat.com, chuhu@redhat.com Message-ID: <1092391733.28508271.1500423031067.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1500359844-27882-1-git-send-email-shuwang@redhat.com> Subject: Re: [PATCH] audit: fix memleak in auditd_send_unicast_skb. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.66.12.174, 10.4.195.23] Thread-Topic: audit: fix memleak in auditd_send_unicast_skb. Thread-Index: mQANc8hiN/vgtbaTTcymFw7E+Z6w1Q== X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 19 Jul 2017 00:10:35 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- Original Message ----- > From: "Paul Moore" > To: shuwang@redhat.com > Cc: "Eric Paris" , linux-audit@redhat.com, linux-kernel@vger.kernel.org, liwang@redhat.com, > chuhu@redhat.com > Sent: Tuesday, July 18, 2017 8:21:05 PM > Subject: Re: [PATCH] audit: fix memleak in auditd_send_unicast_skb. > > On Tue, Jul 18, 2017 at 2:37 AM, wrote: > > From: Shu Wang > > > > Found this issue by kmemleak report, auditd_send_unicast_skb > > did not free skb if rcu_dereference(auditd_conn) returns null. > > > > unreferenced object 0xffff88082568ce00 (size 256): > > comm "auditd", pid 1119, jiffies 4294708499 > > backtrace: > > [] kmemleak_alloc+0x4a/0xa0 > > [] kmem_cache_alloc_node+0xcc/0x210 > > [] __alloc_skb+0x5d/0x290 > > [] audit_make_reply+0x54/0xd0 > > [] audit_receive_msg+0x967/0xd70 > > ---------------- > > (gdb) list *audit_receive_msg+0x967 > > 0xffffffff8113dff7 is in audit_receive_msg (kernel/audit.c:1133). > > 1132 skb = audit_make_reply(0, AUDIT_REPLACE, 0, > > 0, &pvnr, sizeof(pvnr)); > > --------------- > > [] audit_receive+0x52/0xa0 > > [] netlink_unicast+0x181/0x240 > > [] netlink_sendmsg+0x2c2/0x3b0 > > [] sock_sendmsg+0x38/0x50 > > [] SYSC_sendto+0x102/0x190 > > [] SyS_sendto+0xe/0x10 > > [] entry_SYSCALL_64_fastpath+0x1a/0xa5 > > [] 0xffffffffffffffff > > > > Signed-off-by: Shu Wang > > --- > > kernel/audit.c | 1 + > > 1 file changed, 1 insertion(+) > > Hello and thank you for the problem report, it is appreciated. This > was also reported by Masami Ichikawa who provided a patch with the > correct fix (your patch does not catch any error conditions from > netlink_unicast()). netlink_unicast has it's error handling, and is responsible for releasing skb. so the Masami Ichikawa's fix may cause double free problems. 1274 int netlink_unicast(struct sock *ssk, struct sk_buff *skb, 1275 >--->--- u32 portid, int nonblock) 1276 { ...... 1285 >---sk = netlink_getsockbyportid(ssk, portid); 1286 >---if (IS_ERR(sk)) { 1287 >--->---kfree_skb(skb); 1288 >--->---return PTR_ERR(sk); 1289 >---} ...... 1293 >---if (sk_filter(sk, skb)) { 1294 >--->---err = skb->len; 1295 >--->---kfree_skb(skb); 1296 >--->---sock_put(sk); 1297 >--->---return err; 1298 >---} > > * https://www.redhat.com/archives/linux-audit/2017-July/msg00073.html > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index 833267b..6dd5569 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -641,6 +641,7 @@ static int auditd_send_unicast_skb(struct sk_buff *skb) > > ac = rcu_dereference(auditd_conn); > > if (!ac) { > > rcu_read_unlock(); > > + kfree_skb(skb); > > rc = -ECONNREFUSED; > > goto err; > > } > > -- > > 2.5.0 > > -- > paul moore > www.paul-moore.com >