Re: [PATCH] audit: fix memleak in auditd_send_unicast_skb.
On Tue, Jul 18, 2017 at 8:10 PM, Shu Wang <shuwang(a)redhat.com> wrote:
----- Original Message -----
> From: "Paul Moore" <paul(a)paul-moore.com>
> To: shuwang(a)redhat.com
> Cc: "Eric Paris" <eparis(a)redhat.com>, linux-audit(a)redhat.com,
linux-kernel(a)vger.kernel.org, liwang(a)redhat.com,
> chuhu(a)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, <shuwang(a)redhat.com> wrote:
> > From: Shu Wang <shuwang(a)redhat.com>
> >
> > 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:
> > [<ffffffff8176166a>] kmemleak_alloc+0x4a/0xa0
> > [<ffffffff8121820c>] kmem_cache_alloc_node+0xcc/0x210
> > [<ffffffff8161b99d>] __alloc_skb+0x5d/0x290
> > [<ffffffff8113c614>] audit_make_reply+0x54/0xd0
> > [<ffffffff8113dfa7>] 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));
> > ---------------
> > [<ffffffff8113e402>] audit_receive+0x52/0xa0
> > [<ffffffff8166c561>] netlink_unicast+0x181/0x240
> > [<ffffffff8166c8e2>] netlink_sendmsg+0x2c2/0x3b0
> > [<ffffffff816112e8>] sock_sendmsg+0x38/0x50
> > [<ffffffff816117a2>] SYSC_sendto+0x102/0x190
> > [<ffffffff81612f4e>] SyS_sendto+0xe/0x10
> > [<ffffffff8176d337>] entry_SYSCALL_64_fastpath+0x1a/0xa5
> > [<ffffffffffffffff>] 0xffffffffffffffff
> >
> > Signed-off-by: Shu Wang <shuwang(a)redhat.com>
> > ---
> > 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.
Yes, my apologies, you are correct. I've backed out the other patch
and applied your fix to audit/stable-4.13.
--
paul moore
www.paul-moore.com