* [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