1:37 a.m.
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(+)
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
7:21 a.m.
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()).
* 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
7:10 p.m.
----- 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.
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
9:35 a.m.
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
2621
days inactive
2622
days old
linux-audit@lists.linux-audit.osci.io
3 comments
3 participants
participants (3)
-
Paul Moore -
Shu Wang -
shuwang＠redhat.com