* 4.1+ use after free in netlink_broadcast_filtered
@ 2015-06-26 4:44 Dave Jones
2015-06-26 5:17 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2015-06-26 4:44 UTC (permalink / raw)
To: netdev
I taught Trinity about NETLINK_LISTEN_ALL_NSID and NETLINK_LIST_MEMBERSHIPS
yesterday, and this evening, this fell out..
general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
CPU: 1 PID: 9130 Comm: kworker/1:1 Not tainted 4.1.0-gelk-debug+ #1
Workqueue: sock_diag_events sock_diag_broadcast_destroy_work
task: ffff8800b94e4c40 ti: ffff8800352ec000 task.ti: ffff8800352ec000
RIP: 0010:[<ffffffff845c82e4>] [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
RSP: 0000:ffff8800352efd08 EFLAGS: 00010292
RAX: ffff8800ab903d80 RBX: 0000000000000003 RCX: 0000000000000003
RDX: 0000000000000000 RSI: 00000000000000d0 RDI: ffff8800b9c586c0
RBP: ffff8800352efd78 R08: 00000000000000d0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000220 R12: 0000000000000000
R13: 6b6b6b6b6b6b6b6b R14: 0000000000000003 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8800bf700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000002121ff8 CR3: 0000000030169000 CR4: 00000000000007e0
DR0: 00007fe1f0454000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
Stack:
ffff8800b9c586c0 ffff8800b9c586c0 ffff8800ac4692c0 ffff8800936d4a90
ffff8800352efd38 ffffffff8469a93e ffff8800352efd98 ffffffffc09b9b90
ffff8800352efd78 ffff8800ac4692c0 ffff8800b9c586c0 ffff8800831b6ab8
Call Trace:
[<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
[<ffffffffc09b9b90>] ? inet_diag_handler_get_info+0x110/0x1fb [inet_diag]
[<ffffffff845c868d>] netlink_broadcast+0x1d/0x20
[<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
[<ffffffff845b2bf5>] sock_diag_broadcast_destroy_work+0xd5/0x160
[<ffffffff8408ea97>] process_one_work+0x147/0x420
[<ffffffff8408f0f9>] worker_thread+0x69/0x470
[<ffffffff8409fda3>] ? preempt_count_sub+0xa3/0xf0
[<ffffffff8408f090>] ? rescuer_thread+0x320/0x320
[<ffffffff84093cd7>] kthread+0x107/0x120
[<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
[<ffffffff8469d31f>] ret_from_fork+0x3f/0x70
[<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 e5 41 57 41 56 41 55 49 89 fd 48 89 f7 44 89 c6 41 54 41 89 d4 53 89 cb 48 83 ec 48 <49> 8b 45 30 44 89 45 a4 4c 89 4d 98 48 89 45 c0 e8 07 f6 ff ff
RIP [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
RSP <ffff8800352efd08>
---[ end trace e2d8a07893775a9e ]---
r13 looks like slab poison, and the decoded instruction shows..
int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid,
u32 group, gfp_t allocation,
int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
void *filter_data)
{
1b70: e8 00 00 00 00 callq 1b75 <netlink_broadcast_filtered+0x5>
1b75: 55 push %rbp
1b76: 48 89 e5 mov %rsp,%rbp
1b79: 41 57 push %r15
1b7b: 41 56 push %r14
1b7d: 41 55 push %r13
1b7f: 49 89 fd mov %rdi,%r13
1b82: 48 89 f7 mov %rsi,%rdi
1b85: 44 89 c6 mov %r8d,%esi
1b88: 41 54 push %r12
1b8a: 41 89 d4 mov %edx,%r12d
1b8d: 53 push %rbx
1b8e: 89 cb mov %ecx,%ebx
1b90: 48 83 ec 48 sub $0x48,%rsp
1b94: 49 8b 45 30 mov 0x30(%r13),%rax <-- trapping instruction
1b98: 44 89 45 a4 mov %r8d,-0x5c(%rbp)
1b9c: 4c 89 4d 98 mov %r9,-0x68(%rbp)
1ba0: 48 89 45 c0 mov %rax,-0x40(%rbp)
struct net *net = sock_net(ssk);
So it looks like the ssk we passed in was already freed.
I'll dig into this some more next week, and try to find a better
reproducer.
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 4.1+ use after free in netlink_broadcast_filtered
2015-06-26 4:44 4.1+ use after free in netlink_broadcast_filtered Dave Jones
@ 2015-06-26 5:17 ` Eric Dumazet
2015-06-26 14:33 ` Craig Gallek
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-06-26 5:17 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev, Craig Gallek
On Fri, 2015-06-26 at 00:44 -0400, Dave Jones wrote:
> I taught Trinity about NETLINK_LISTEN_ALL_NSID and NETLINK_LIST_MEMBERSHIPS
> yesterday, and this evening, this fell out..
>
> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> CPU: 1 PID: 9130 Comm: kworker/1:1 Not tainted 4.1.0-gelk-debug+ #1
> Workqueue: sock_diag_events sock_diag_broadcast_destroy_work
> task: ffff8800b94e4c40 ti: ffff8800352ec000 task.ti: ffff8800352ec000
> RIP: 0010:[<ffffffff845c82e4>] [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
> RSP: 0000:ffff8800352efd08 EFLAGS: 00010292
> RAX: ffff8800ab903d80 RBX: 0000000000000003 RCX: 0000000000000003
> RDX: 0000000000000000 RSI: 00000000000000d0 RDI: ffff8800b9c586c0
> RBP: ffff8800352efd78 R08: 00000000000000d0 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000220 R12: 0000000000000000
> R13: 6b6b6b6b6b6b6b6b R14: 0000000000000003 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff8800bf700000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000002121ff8 CR3: 0000000030169000 CR4: 00000000000007e0
> DR0: 00007fe1f0454000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> Stack:
> ffff8800b9c586c0 ffff8800b9c586c0 ffff8800ac4692c0 ffff8800936d4a90
> ffff8800352efd38 ffffffff8469a93e ffff8800352efd98 ffffffffc09b9b90
> ffff8800352efd78 ffff8800ac4692c0 ffff8800b9c586c0 ffff8800831b6ab8
> Call Trace:
> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
> [<ffffffffc09b9b90>] ? inet_diag_handler_get_info+0x110/0x1fb [inet_diag]
> [<ffffffff845c868d>] netlink_broadcast+0x1d/0x20
> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
> [<ffffffff845b2bf5>] sock_diag_broadcast_destroy_work+0xd5/0x160
> [<ffffffff8408ea97>] process_one_work+0x147/0x420
> [<ffffffff8408f0f9>] worker_thread+0x69/0x470
> [<ffffffff8409fda3>] ? preempt_count_sub+0xa3/0xf0
> [<ffffffff8408f090>] ? rescuer_thread+0x320/0x320
> [<ffffffff84093cd7>] kthread+0x107/0x120
> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
> [<ffffffff8469d31f>] ret_from_fork+0x3f/0x70
> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
> Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 e5 41 57 41 56 41 55 49 89 fd 48 89 f7 44 89 c6 41 54 41 89 d4 53 89 cb 48 83 ec 48 <49> 8b 45 30 44 89 45 a4 4c 89 4d 98 48 89 45 c0 e8 07 f6 ff ff
> RIP [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
> RSP <ffff8800352efd08>
> ---[ end trace e2d8a07893775a9e ]---
>
>
> r13 looks like slab poison, and the decoded instruction shows..
>
>
> int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid,
> u32 group, gfp_t allocation,
> int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
> void *filter_data)
> {
> 1b70: e8 00 00 00 00 callq 1b75 <netlink_broadcast_filtered+0x5>
> 1b75: 55 push %rbp
> 1b76: 48 89 e5 mov %rsp,%rbp
> 1b79: 41 57 push %r15
> 1b7b: 41 56 push %r14
> 1b7d: 41 55 push %r13
> 1b7f: 49 89 fd mov %rdi,%r13
> 1b82: 48 89 f7 mov %rsi,%rdi
> 1b85: 44 89 c6 mov %r8d,%esi
> 1b88: 41 54 push %r12
> 1b8a: 41 89 d4 mov %edx,%r12d
> 1b8d: 53 push %rbx
> 1b8e: 89 cb mov %ecx,%ebx
> 1b90: 48 83 ec 48 sub $0x48,%rsp
> 1b94: 49 8b 45 30 mov 0x30(%r13),%rax <-- trapping instruction
> 1b98: 44 89 45 a4 mov %r8d,-0x5c(%rbp)
> 1b9c: 4c 89 4d 98 mov %r9,-0x68(%rbp)
> 1ba0: 48 89 45 c0 mov %rax,-0x40(%rbp)
> struct net *net = sock_net(ssk);
>
>
> So it looks like the ssk we passed in was already freed.
> I'll dig into this some more next week, and try to find a better
> reproducer.
CC Craig
Thanks for the report Dave.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 4.1+ use after free in netlink_broadcast_filtered
2015-06-26 5:17 ` Eric Dumazet
@ 2015-06-26 14:33 ` Craig Gallek
2015-06-26 20:26 ` Craig Gallek
0 siblings, 1 reply; 6+ messages in thread
From: Craig Gallek @ 2015-06-26 14:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dave Jones, netdev
On Fri, Jun 26, 2015 at 1:17 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2015-06-26 at 00:44 -0400, Dave Jones wrote:
>> I taught Trinity about NETLINK_LISTEN_ALL_NSID and NETLINK_LIST_MEMBERSHIPS
>> yesterday, and this evening, this fell out..
>>
>> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>> CPU: 1 PID: 9130 Comm: kworker/1:1 Not tainted 4.1.0-gelk-debug+ #1
>> Workqueue: sock_diag_events sock_diag_broadcast_destroy_work
>> task: ffff8800b94e4c40 ti: ffff8800352ec000 task.ti: ffff8800352ec000
>> RIP: 0010:[<ffffffff845c82e4>] [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
>> RSP: 0000:ffff8800352efd08 EFLAGS: 00010292
>> RAX: ffff8800ab903d80 RBX: 0000000000000003 RCX: 0000000000000003
>> RDX: 0000000000000000 RSI: 00000000000000d0 RDI: ffff8800b9c586c0
>> RBP: ffff8800352efd78 R08: 00000000000000d0 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000220 R12: 0000000000000000
>> R13: 6b6b6b6b6b6b6b6b R14: 0000000000000003 R15: 0000000000000000
>> FS: 0000000000000000(0000) GS:ffff8800bf700000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> CR2: 0000000002121ff8 CR3: 0000000030169000 CR4: 00000000000007e0
>> DR0: 00007fe1f0454000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>> Stack:
>> ffff8800b9c586c0 ffff8800b9c586c0 ffff8800ac4692c0 ffff8800936d4a90
>> ffff8800352efd38 ffffffff8469a93e ffff8800352efd98 ffffffffc09b9b90
>> ffff8800352efd78 ffff8800ac4692c0 ffff8800b9c586c0 ffff8800831b6ab8
>> Call Trace:
>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
>> [<ffffffffc09b9b90>] ? inet_diag_handler_get_info+0x110/0x1fb [inet_diag]
>> [<ffffffff845c868d>] netlink_broadcast+0x1d/0x20
>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
>> [<ffffffff845b2bf5>] sock_diag_broadcast_destroy_work+0xd5/0x160
>> [<ffffffff8408ea97>] process_one_work+0x147/0x420
>> [<ffffffff8408f0f9>] worker_thread+0x69/0x470
>> [<ffffffff8409fda3>] ? preempt_count_sub+0xa3/0xf0
>> [<ffffffff8408f090>] ? rescuer_thread+0x320/0x320
>> [<ffffffff84093cd7>] kthread+0x107/0x120
>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
>> [<ffffffff8469d31f>] ret_from_fork+0x3f/0x70
>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
>> Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 e5 41 57 41 56 41 55 49 89 fd 48 89 f7 44 89 c6 41 54 41 89 d4 53 89 cb 48 83 ec 48 <49> 8b 45 30 44 89 45 a4 4c 89 4d 98 48 89 45 c0 e8 07 f6 ff ff
>> RIP [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
>> RSP <ffff8800352efd08>
>> ---[ end trace e2d8a07893775a9e ]---
>>
>>
>> r13 looks like slab poison, and the decoded instruction shows..
>>
>>
>> int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid,
>> u32 group, gfp_t allocation,
>> int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
>> void *filter_data)
>> {
>> 1b70: e8 00 00 00 00 callq 1b75 <netlink_broadcast_filtered+0x5>
>> 1b75: 55 push %rbp
>> 1b76: 48 89 e5 mov %rsp,%rbp
>> 1b79: 41 57 push %r15
>> 1b7b: 41 56 push %r14
>> 1b7d: 41 55 push %r13
>> 1b7f: 49 89 fd mov %rdi,%r13
>> 1b82: 48 89 f7 mov %rsi,%rdi
>> 1b85: 44 89 c6 mov %r8d,%esi
>> 1b88: 41 54 push %r12
>> 1b8a: 41 89 d4 mov %edx,%r12d
>> 1b8d: 53 push %rbx
>> 1b8e: 89 cb mov %ecx,%ebx
>> 1b90: 48 83 ec 48 sub $0x48,%rsp
>> 1b94: 49 8b 45 30 mov 0x30(%r13),%rax <-- trapping instruction
>> 1b98: 44 89 45 a4 mov %r8d,-0x5c(%rbp)
>> 1b9c: 4c 89 4d 98 mov %r9,-0x68(%rbp)
>> 1ba0: 48 89 45 c0 mov %rax,-0x40(%rbp)
>> struct net *net = sock_net(ssk);
>>
>>
>> So it looks like the ssk we passed in was already freed.
>> I'll dig into this some more next week, and try to find a better
>> reproducer.
Thanks for the pointer. In this stack, I believe ssk should always be
diag_nlsk from the struct net associated with a sock that is being
destroyed. Given that diag_nlsk is created/destroyed via __net_init
and __net_exit and that this broadcast work happens out of band in a
work queue, it seems possible that the destruction of a given
diag_nlsk can race with a socked destruction event.
I'll try to reproduce it and send a fix as soon as I confirm. I think
a simple fix may be to change the nlmsg_multicast line in
sock_diag_broadcast_destroy_work to use init_net instead of the per
socket namespace.
>
> CC Craig
>
> Thanks for the report Dave.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 4.1+ use after free in netlink_broadcast_filtered
2015-06-26 14:33 ` Craig Gallek
@ 2015-06-26 20:26 ` Craig Gallek
2015-06-30 15:28 ` Craig Gallek
0 siblings, 1 reply; 6+ messages in thread
From: Craig Gallek @ 2015-06-26 20:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dave Jones, netdev
On Fri, Jun 26, 2015 at 10:33 AM, Craig Gallek <kraig@google.com> wrote:
> On Fri, Jun 26, 2015 at 1:17 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Fri, 2015-06-26 at 00:44 -0400, Dave Jones wrote:
>>> I taught Trinity about NETLINK_LISTEN_ALL_NSID and NETLINK_LIST_MEMBERSHIPS
>>> yesterday, and this evening, this fell out..
>>>
>>> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>>> CPU: 1 PID: 9130 Comm: kworker/1:1 Not tainted 4.1.0-gelk-debug+ #1
>>> Workqueue: sock_diag_events sock_diag_broadcast_destroy_work
>>> task: ffff8800b94e4c40 ti: ffff8800352ec000 task.ti: ffff8800352ec000
>>> RIP: 0010:[<ffffffff845c82e4>] [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
>>> RSP: 0000:ffff8800352efd08 EFLAGS: 00010292
>>> RAX: ffff8800ab903d80 RBX: 0000000000000003 RCX: 0000000000000003
>>> RDX: 0000000000000000 RSI: 00000000000000d0 RDI: ffff8800b9c586c0
>>> RBP: ffff8800352efd78 R08: 00000000000000d0 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000220 R12: 0000000000000000
>>> R13: 6b6b6b6b6b6b6b6b R14: 0000000000000003 R15: 0000000000000000
>>> FS: 0000000000000000(0000) GS:ffff8800bf700000(0000) knlGS:0000000000000000
>>> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>>> CR2: 0000000002121ff8 CR3: 0000000030169000 CR4: 00000000000007e0
>>> DR0: 00007fe1f0454000 DR1: 0000000000000000 DR2: 0000000000000000
>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>>> Stack:
>>> ffff8800b9c586c0 ffff8800b9c586c0 ffff8800ac4692c0 ffff8800936d4a90
>>> ffff8800352efd38 ffffffff8469a93e ffff8800352efd98 ffffffffc09b9b90
>>> ffff8800352efd78 ffff8800ac4692c0 ffff8800b9c586c0 ffff8800831b6ab8
>>> Call Trace:
>>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
>>> [<ffffffffc09b9b90>] ? inet_diag_handler_get_info+0x110/0x1fb [inet_diag]
>>> [<ffffffff845c868d>] netlink_broadcast+0x1d/0x20
>>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
>>> [<ffffffff845b2bf5>] sock_diag_broadcast_destroy_work+0xd5/0x160
>>> [<ffffffff8408ea97>] process_one_work+0x147/0x420
>>> [<ffffffff8408f0f9>] worker_thread+0x69/0x470
>>> [<ffffffff8409fda3>] ? preempt_count_sub+0xa3/0xf0
>>> [<ffffffff8408f090>] ? rescuer_thread+0x320/0x320
>>> [<ffffffff84093cd7>] kthread+0x107/0x120
>>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
>>> [<ffffffff8469d31f>] ret_from_fork+0x3f/0x70
>>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
>>> Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 e5 41 57 41 56 41 55 49 89 fd 48 89 f7 44 89 c6 41 54 41 89 d4 53 89 cb 48 83 ec 48 <49> 8b 45 30 44 89 45 a4 4c 89 4d 98 48 89 45 c0 e8 07 f6 ff ff
>>> RIP [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
>>> RSP <ffff8800352efd08>
>>> ---[ end trace e2d8a07893775a9e ]---
>>>
>>>
>>> r13 looks like slab poison, and the decoded instruction shows..
>>>
>>>
>>> int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid,
>>> u32 group, gfp_t allocation,
>>> int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
>>> void *filter_data)
>>> {
>>> 1b70: e8 00 00 00 00 callq 1b75 <netlink_broadcast_filtered+0x5>
>>> 1b75: 55 push %rbp
>>> 1b76: 48 89 e5 mov %rsp,%rbp
>>> 1b79: 41 57 push %r15
>>> 1b7b: 41 56 push %r14
>>> 1b7d: 41 55 push %r13
>>> 1b7f: 49 89 fd mov %rdi,%r13
>>> 1b82: 48 89 f7 mov %rsi,%rdi
>>> 1b85: 44 89 c6 mov %r8d,%esi
>>> 1b88: 41 54 push %r12
>>> 1b8a: 41 89 d4 mov %edx,%r12d
>>> 1b8d: 53 push %rbx
>>> 1b8e: 89 cb mov %ecx,%ebx
>>> 1b90: 48 83 ec 48 sub $0x48,%rsp
>>> 1b94: 49 8b 45 30 mov 0x30(%r13),%rax <-- trapping instruction
>>> 1b98: 44 89 45 a4 mov %r8d,-0x5c(%rbp)
>>> 1b9c: 4c 89 4d 98 mov %r9,-0x68(%rbp)
>>> 1ba0: 48 89 45 c0 mov %rax,-0x40(%rbp)
>>> struct net *net = sock_net(ssk);
>>>
>>>
>>> So it looks like the ssk we passed in was already freed.
>>> I'll dig into this some more next week, and try to find a better
>>> reproducer.
> Thanks for the pointer. In this stack, I believe ssk should always be
> diag_nlsk from the struct net associated with a sock that is being
> destroyed. Given that diag_nlsk is created/destroyed via __net_init
> and __net_exit and that this broadcast work happens out of band in a
> work queue, it seems possible that the destruction of a given
> diag_nlsk can race with a socked destruction event.
>
> I'll try to reproduce it and send a fix as soon as I confirm. I think
> a simple fix may be to change the nlmsg_multicast line in
> sock_diag_broadcast_destroy_work to use init_net instead of the per
> socket namespace.
I haven't been able to reproduce this failure yet. Further, I think
I've convinced myself that the network namespace reference counting is
correct in the sock_diag_broadcast_destroy_work path (the socket being
destroyed should hold a reference to the net structure at least until
it calls sk_destruct).
My new theory is that there was a pre-existing extraneous call to
put_net that prematurely destroys the structure. My change to add the
broadcast (which relies on the net structure) may have simply exposed
it. An additional sanity check in put_net could confirm this theory
(with a reliable test case). I'll keep digging...
>>
>> CC Craig
>>
>> Thanks for the report Dave.
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 4.1+ use after free in netlink_broadcast_filtered
2015-06-26 20:26 ` Craig Gallek
@ 2015-06-30 15:28 ` Craig Gallek
2015-06-30 16:20 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Craig Gallek @ 2015-06-30 15:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dave Jones, netdev
On Fri, Jun 26, 2015 at 4:26 PM, Craig Gallek <kraig@google.com> wrote:
> On Fri, Jun 26, 2015 at 10:33 AM, Craig Gallek <kraig@google.com> wrote:
>> On Fri, Jun 26, 2015 at 1:17 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> On Fri, 2015-06-26 at 00:44 -0400, Dave Jones wrote:
>>>> I taught Trinity about NETLINK_LISTEN_ALL_NSID and NETLINK_LIST_MEMBERSHIPS
>>>> yesterday, and this evening, this fell out..
>>>>
>>>> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>>>> CPU: 1 PID: 9130 Comm: kworker/1:1 Not tainted 4.1.0-gelk-debug+ #1
>>>> Workqueue: sock_diag_events sock_diag_broadcast_destroy_work
>>>> task: ffff8800b94e4c40 ti: ffff8800352ec000 task.ti: ffff8800352ec000
>>>> RIP: 0010:[<ffffffff845c82e4>] [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
>>>> RSP: 0000:ffff8800352efd08 EFLAGS: 00010292
>>>> RAX: ffff8800ab903d80 RBX: 0000000000000003 RCX: 0000000000000003
>>>> RDX: 0000000000000000 RSI: 00000000000000d0 RDI: ffff8800b9c586c0
>>>> RBP: ffff8800352efd78 R08: 00000000000000d0 R09: 0000000000000000
>>>> R10: 0000000000000000 R11: 0000000000000220 R12: 0000000000000000
>>>> R13: 6b6b6b6b6b6b6b6b R14: 0000000000000003 R15: 0000000000000000
>>>> FS: 0000000000000000(0000) GS:ffff8800bf700000(0000) knlGS:0000000000000000
>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>>>> CR2: 0000000002121ff8 CR3: 0000000030169000 CR4: 00000000000007e0
>>>> DR0: 00007fe1f0454000 DR1: 0000000000000000 DR2: 0000000000000000
>>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>>>> Stack:
>>>> ffff8800b9c586c0 ffff8800b9c586c0 ffff8800ac4692c0 ffff8800936d4a90
>>>> ffff8800352efd38 ffffffff8469a93e ffff8800352efd98 ffffffffc09b9b90
>>>> ffff8800352efd78 ffff8800ac4692c0 ffff8800b9c586c0 ffff8800831b6ab8
>>>> Call Trace:
>>>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
>>>> [<ffffffffc09b9b90>] ? inet_diag_handler_get_info+0x110/0x1fb [inet_diag]
>>>> [<ffffffff845c868d>] netlink_broadcast+0x1d/0x20
>>>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
>>>> [<ffffffff845b2bf5>] sock_diag_broadcast_destroy_work+0xd5/0x160
>>>> [<ffffffff8408ea97>] process_one_work+0x147/0x420
>>>> [<ffffffff8408f0f9>] worker_thread+0x69/0x470
>>>> [<ffffffff8409fda3>] ? preempt_count_sub+0xa3/0xf0
>>>> [<ffffffff8408f090>] ? rescuer_thread+0x320/0x320
>>>> [<ffffffff84093cd7>] kthread+0x107/0x120
>>>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
>>>> [<ffffffff8469d31f>] ret_from_fork+0x3f/0x70
>>>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
>>>> Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 e5 41 57 41 56 41 55 49 89 fd 48 89 f7 44 89 c6 41 54 41 89 d4 53 89 cb 48 83 ec 48 <49> 8b 45 30 44 89 45 a4 4c 89 4d 98 48 89 45 c0 e8 07 f6 ff ff
>>>> RIP [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
>>>> RSP <ffff8800352efd08>
>>>> ---[ end trace e2d8a07893775a9e ]---
>>>>
>>>>
>>>> r13 looks like slab poison, and the decoded instruction shows..
>>>>
>>>>
>>>> int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid,
>>>> u32 group, gfp_t allocation,
>>>> int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
>>>> void *filter_data)
>>>> {
>>>> 1b70: e8 00 00 00 00 callq 1b75 <netlink_broadcast_filtered+0x5>
>>>> 1b75: 55 push %rbp
>>>> 1b76: 48 89 e5 mov %rsp,%rbp
>>>> 1b79: 41 57 push %r15
>>>> 1b7b: 41 56 push %r14
>>>> 1b7d: 41 55 push %r13
>>>> 1b7f: 49 89 fd mov %rdi,%r13
>>>> 1b82: 48 89 f7 mov %rsi,%rdi
>>>> 1b85: 44 89 c6 mov %r8d,%esi
>>>> 1b88: 41 54 push %r12
>>>> 1b8a: 41 89 d4 mov %edx,%r12d
>>>> 1b8d: 53 push %rbx
>>>> 1b8e: 89 cb mov %ecx,%ebx
>>>> 1b90: 48 83 ec 48 sub $0x48,%rsp
>>>> 1b94: 49 8b 45 30 mov 0x30(%r13),%rax <-- trapping instruction
>>>> 1b98: 44 89 45 a4 mov %r8d,-0x5c(%rbp)
>>>> 1b9c: 4c 89 4d 98 mov %r9,-0x68(%rbp)
>>>> 1ba0: 48 89 45 c0 mov %rax,-0x40(%rbp)
>>>> struct net *net = sock_net(ssk);
>>>>
>>>>
>>>> So it looks like the ssk we passed in was already freed.
>>>> I'll dig into this some more next week, and try to find a better
>>>> reproducer.
>> Thanks for the pointer. In this stack, I believe ssk should always be
>> diag_nlsk from the struct net associated with a sock that is being
>> destroyed. Given that diag_nlsk is created/destroyed via __net_init
>> and __net_exit and that this broadcast work happens out of band in a
>> work queue, it seems possible that the destruction of a given
>> diag_nlsk can race with a socked destruction event.
>>
>> I'll try to reproduce it and send a fix as soon as I confirm. I think
>> a simple fix may be to change the nlmsg_multicast line in
>> sock_diag_broadcast_destroy_work to use init_net instead of the per
>> socket namespace.
>
> I haven't been able to reproduce this failure yet. Further, I think
> I've convinced myself that the network namespace reference counting is
> correct in the sock_diag_broadcast_destroy_work path (the socket being
> destroyed should hold a reference to the net structure at least until
> it calls sk_destruct).
>
> My new theory is that there was a pre-existing extraneous call to
> put_net that prematurely destroys the structure. My change to add the
> broadcast (which relies on the net structure) may have simply exposed
> it. An additional sanity check in put_net could confirm this theory
> (with a reliable test case). I'll keep digging...
I still haven't been able to produce this exact crash, but I think I
understand what can cause it. The patch below shows a reference count
of zero when creating/destroying a network namespace.
~# ip netns add test-ns
~# ip netns delete test-ns
[ 342.351708] broadcast kernel socket ffff880662f1f2c0 count: 0
The reference counting behavior of network namespaces seems to have
changed recently in
https://patchwork.ozlabs.org/patch/470239/
through
https://patchwork.ozlabs.org/patch/470244/
I'm not exactly sure if this is a coincidence or actually related to
this issue. Either way, I don't think we care about broadcasting the
destruction of kernel sockets anyway. I think a reasonable fix would
be to simply ignore sockets that don't hold a reference to the
namespace when they are destroyed. I'll prepare a patch which does
this.
diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index d79866c..e642bfae 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -146,0 +147,7 @@ static void
sock_diag_broadcast_destroy_work(struct work_struct *work)
+
+ if (!sk->sk_net_refcnt) {
+ pr_err(
+ "broadcast kernel socket %p count: %d\n", sk,
+ atomic_read(&sock_net(sk)->count));
+ }
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: 4.1+ use after free in netlink_broadcast_filtered
2015-06-30 15:28 ` Craig Gallek
@ 2015-06-30 16:20 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2015-06-30 16:20 UTC (permalink / raw)
To: Craig Gallek; +Cc: Dave Jones, netdev
On Tue, 2015-06-30 at 11:28 -0400, Craig Gallek wrote:
> On Fri, Jun 26, 2015 at 4:26 PM, Craig Gallek <kraig@google.com> wrote:
> > On Fri, Jun 26, 2015 at 10:33 AM, Craig Gallek <kraig@google.com> wrote:
> >> On Fri, Jun 26, 2015 at 1:17 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >>> On Fri, 2015-06-26 at 00:44 -0400, Dave Jones wrote:
> >>>> I taught Trinity about NETLINK_LISTEN_ALL_NSID and NETLINK_LIST_MEMBERSHIPS
> >>>> yesterday, and this evening, this fell out..
> >>>>
> >>>> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> >>>> CPU: 1 PID: 9130 Comm: kworker/1:1 Not tainted 4.1.0-gelk-debug+ #1
> >>>> Workqueue: sock_diag_events sock_diag_broadcast_destroy_work
> >>>> task: ffff8800b94e4c40 ti: ffff8800352ec000 task.ti: ffff8800352ec000
> >>>> RIP: 0010:[<ffffffff845c82e4>] [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
> >>>> RSP: 0000:ffff8800352efd08 EFLAGS: 00010292
> >>>> RAX: ffff8800ab903d80 RBX: 0000000000000003 RCX: 0000000000000003
> >>>> RDX: 0000000000000000 RSI: 00000000000000d0 RDI: ffff8800b9c586c0
> >>>> RBP: ffff8800352efd78 R08: 00000000000000d0 R09: 0000000000000000
> >>>> R10: 0000000000000000 R11: 0000000000000220 R12: 0000000000000000
> >>>> R13: 6b6b6b6b6b6b6b6b R14: 0000000000000003 R15: 0000000000000000
> >>>> FS: 0000000000000000(0000) GS:ffff8800bf700000(0000) knlGS:0000000000000000
> >>>> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> >>>> CR2: 0000000002121ff8 CR3: 0000000030169000 CR4: 00000000000007e0
> >>>> DR0: 00007fe1f0454000 DR1: 0000000000000000 DR2: 0000000000000000
> >>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> >>>> Stack:
> >>>> ffff8800b9c586c0 ffff8800b9c586c0 ffff8800ac4692c0 ffff8800936d4a90
> >>>> ffff8800352efd38 ffffffff8469a93e ffff8800352efd98 ffffffffc09b9b90
> >>>> ffff8800352efd78 ffff8800ac4692c0 ffff8800b9c586c0 ffff8800831b6ab8
> >>>> Call Trace:
> >>>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
> >>>> [<ffffffffc09b9b90>] ? inet_diag_handler_get_info+0x110/0x1fb [inet_diag]
> >>>> [<ffffffff845c868d>] netlink_broadcast+0x1d/0x20
> >>>> [<ffffffff8469a93e>] ? mutex_unlock+0xe/0x10
> >>>> [<ffffffff845b2bf5>] sock_diag_broadcast_destroy_work+0xd5/0x160
> >>>> [<ffffffff8408ea97>] process_one_work+0x147/0x420
> >>>> [<ffffffff8408f0f9>] worker_thread+0x69/0x470
> >>>> [<ffffffff8409fda3>] ? preempt_count_sub+0xa3/0xf0
> >>>> [<ffffffff8408f090>] ? rescuer_thread+0x320/0x320
> >>>> [<ffffffff84093cd7>] kthread+0x107/0x120
> >>>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
> >>>> [<ffffffff8469d31f>] ret_from_fork+0x3f/0x70
> >>>> [<ffffffff84093bd0>] ? kthread_create_on_node+0x1b0/0x1b0
> >>>> Code: 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 e5 41 57 41 56 41 55 49 89 fd 48 89 f7 44 89 c6 41 54 41 89 d4 53 89 cb 48 83 ec 48 <49> 8b 45 30 44 89 45 a4 4c 89 4d 98 48 89 45 c0 e8 07 f6 ff ff
> >>>> RIP [<ffffffff845c82e4>] netlink_broadcast_filtered+0x24/0x3b0
> >>>> RSP <ffff8800352efd08>
> >>>> ---[ end trace e2d8a07893775a9e ]---
> >>>>
> >>>>
> >>>> r13 looks like slab poison, and the decoded instruction shows..
> >>>>
> >>>>
> >>>> int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb, u32 portid,
> >>>> u32 group, gfp_t allocation,
> >>>> int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
> >>>> void *filter_data)
> >>>> {
> >>>> 1b70: e8 00 00 00 00 callq 1b75 <netlink_broadcast_filtered+0x5>
> >>>> 1b75: 55 push %rbp
> >>>> 1b76: 48 89 e5 mov %rsp,%rbp
> >>>> 1b79: 41 57 push %r15
> >>>> 1b7b: 41 56 push %r14
> >>>> 1b7d: 41 55 push %r13
> >>>> 1b7f: 49 89 fd mov %rdi,%r13
> >>>> 1b82: 48 89 f7 mov %rsi,%rdi
> >>>> 1b85: 44 89 c6 mov %r8d,%esi
> >>>> 1b88: 41 54 push %r12
> >>>> 1b8a: 41 89 d4 mov %edx,%r12d
> >>>> 1b8d: 53 push %rbx
> >>>> 1b8e: 89 cb mov %ecx,%ebx
> >>>> 1b90: 48 83 ec 48 sub $0x48,%rsp
> >>>> 1b94: 49 8b 45 30 mov 0x30(%r13),%rax <-- trapping instruction
> >>>> 1b98: 44 89 45 a4 mov %r8d,-0x5c(%rbp)
> >>>> 1b9c: 4c 89 4d 98 mov %r9,-0x68(%rbp)
> >>>> 1ba0: 48 89 45 c0 mov %rax,-0x40(%rbp)
> >>>> struct net *net = sock_net(ssk);
> >>>>
> >>>>
> >>>> So it looks like the ssk we passed in was already freed.
> >>>> I'll dig into this some more next week, and try to find a better
> >>>> reproducer.
> >> Thanks for the pointer. In this stack, I believe ssk should always be
> >> diag_nlsk from the struct net associated with a sock that is being
> >> destroyed. Given that diag_nlsk is created/destroyed via __net_init
> >> and __net_exit and that this broadcast work happens out of band in a
> >> work queue, it seems possible that the destruction of a given
> >> diag_nlsk can race with a socked destruction event.
> >>
> >> I'll try to reproduce it and send a fix as soon as I confirm. I think
> >> a simple fix may be to change the nlmsg_multicast line in
> >> sock_diag_broadcast_destroy_work to use init_net instead of the per
> >> socket namespace.
> >
> > I haven't been able to reproduce this failure yet. Further, I think
> > I've convinced myself that the network namespace reference counting is
> > correct in the sock_diag_broadcast_destroy_work path (the socket being
> > destroyed should hold a reference to the net structure at least until
> > it calls sk_destruct).
> >
> > My new theory is that there was a pre-existing extraneous call to
> > put_net that prematurely destroys the structure. My change to add the
> > broadcast (which relies on the net structure) may have simply exposed
> > it. An additional sanity check in put_net could confirm this theory
> > (with a reliable test case). I'll keep digging...
> I still haven't been able to produce this exact crash, but I think I
> understand what can cause it. The patch below shows a reference count
> of zero when creating/destroying a network namespace.
> ~# ip netns add test-ns
> ~# ip netns delete test-ns
> [ 342.351708] broadcast kernel socket ffff880662f1f2c0 count: 0
>
> The reference counting behavior of network namespaces seems to have
> changed recently in
> https://patchwork.ozlabs.org/patch/470239/
> through
> https://patchwork.ozlabs.org/patch/470244/
> I'm not exactly sure if this is a coincidence or actually related to
> this issue. Either way, I don't think we care about broadcasting the
> destruction of kernel sockets anyway. I think a reasonable fix would
> be to simply ignore sockets that don't hold a reference to the
> namespace when they are destroyed. I'll prepare a patch which does
> this.
>
Yes, this is definitely the reason, although you probably should use
something like attached patch.
This came with commit 26abe14379f8e2fa3fd1bcf97c9a7ad9364886fe
("net: Modify sk_alloc to not reference count the netns of kernel
sockets.")
diff --git a/net/core/sock.c b/net/core/sock.c
index 1e1fe9a68d835983d760d50f9ef6a11309ffcfc1..165230cfa10ea01849ca2b4358e7e6c1e69b83aa 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1454,7 +1454,7 @@ void sk_destruct(struct sock *sk)
static void __sk_free(struct sock *sk)
{
- if (unlikely(sock_diag_has_destroy_listeners(sk)))
+ if (unlikely(sock_diag_has_destroy_listeners(sk) && sk->sk_net_refcnt))
sock_diag_broadcast_destroy(sk);
else
sk_destruct(sk);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-06-30 16:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-26 4:44 4.1+ use after free in netlink_broadcast_filtered Dave Jones
2015-06-26 5:17 ` Eric Dumazet
2015-06-26 14:33 ` Craig Gallek
2015-06-26 20:26 ` Craig Gallek
2015-06-30 15:28 ` Craig Gallek
2015-06-30 16:20 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.