* [PATCH] audit: fix memleak in auditd_send_unicast_skb.
@ 2017-07-18 6:37 shuwang
2017-07-18 12:21 ` Paul Moore
0 siblings, 1 reply; 4+ messages in thread
From: shuwang @ 2017-07-18 6:37 UTC (permalink / raw)
To: paul, eparis; +Cc: linux-audit, linux-kernel, liwang, chuhu, shuwang
From: Shu Wang <shuwang@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@redhat.com>
---
kernel/audit.c | 1 +
1 file changed, 1 insertion(+)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] audit: fix memleak in auditd_send_unicast_skb. 2017-07-18 6:37 [PATCH] audit: fix memleak in auditd_send_unicast_skb shuwang @ 2017-07-18 12:21 ` Paul Moore 2017-07-19 0:10 ` Shu Wang 0 siblings, 1 reply; 4+ messages in thread From: Paul Moore @ 2017-07-18 12:21 UTC (permalink / raw) To: shuwang; +Cc: Eric Paris, linux-audit, linux-kernel, liwang, chuhu On Tue, Jul 18, 2017 at 2:37 AM, <shuwang@redhat.com> wrote: > From: Shu Wang <shuwang@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@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()). * 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] audit: fix memleak in auditd_send_unicast_skb. 2017-07-18 12:21 ` Paul Moore @ 2017-07-19 0:10 ` Shu Wang 2017-07-19 14:35 ` Paul Moore 0 siblings, 1 reply; 4+ messages in thread From: Shu Wang @ 2017-07-19 0:10 UTC (permalink / raw) To: Paul Moore; +Cc: Eric Paris, linux-audit, linux-kernel, liwang, chuhu ----- Original Message ----- > From: "Paul Moore" <paul@paul-moore.com> > To: shuwang@redhat.com > Cc: "Eric Paris" <eparis@redhat.com>, 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, <shuwang@redhat.com> wrote: > > From: Shu Wang <shuwang@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@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. 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] audit: fix memleak in auditd_send_unicast_skb. 2017-07-19 0:10 ` Shu Wang @ 2017-07-19 14:35 ` Paul Moore 0 siblings, 0 replies; 4+ messages in thread From: Paul Moore @ 2017-07-19 14:35 UTC (permalink / raw) To: Shu Wang; +Cc: liwang, linux-audit, chuhu, linux-kernel On Tue, Jul 18, 2017 at 8:10 PM, Shu Wang <shuwang@redhat.com> wrote: > ----- Original Message ----- >> From: "Paul Moore" <paul@paul-moore.com> >> To: shuwang@redhat.com >> Cc: "Eric Paris" <eparis@redhat.com>, 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, <shuwang@redhat.com> wrote: >> > From: Shu Wang <shuwang@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@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-19 14:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-18 6:37 [PATCH] audit: fix memleak in auditd_send_unicast_skb shuwang 2017-07-18 12:21 ` Paul Moore 2017-07-19 0:10 ` Shu Wang 2017-07-19 14:35 ` Paul Moore
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox