2017-04-04 14:50:15

by Andrey Konovalov

[permalink] [raw]
Subject: net/ipv4: use-after-free in ipv4_mtu

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

On commit a71c9a1c779f2499fb2afc0553e543f18aff6edf (4.11-rc5).

Unfortunately it's not reproducible.

==================================================================
BUG: KASAN: use-after-free in dst_metric_raw include/net/dst.h:176
[inline] at addr ffff88003d6a965c
BUG: KASAN: use-after-free in ipv4_mtu+0x3f2/0x4b0
net/ipv4/route.c:1270 at addr ffff88003d6a965c
Read of size 4 by task syz-executor3/20611
CPU: 3 PID: 20611 Comm: syz-executor3 Not tainted 4.11.0-rc5+ #199
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x292/0x398 lib/dump_stack.c:52
kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
print_address_description mm/kasan/report.c:202 [inline]
kasan_report_error mm/kasan/report.c:291 [inline]
kasan_report+0x252/0x510 mm/kasan/report.c:347
__asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:367
dst_metric_raw include/net/dst.h:176 [inline]
ipv4_mtu+0x3f2/0x4b0 net/ipv4/route.c:1270
dst_mtu include/net/dst.h:221 [inline]
do_ip_getsockopt+0x71d/0x2290 net/ipv4/ip_sockglue.c:1433
ip_getsockopt+0x90/0x230 net/ipv4/ip_sockglue.c:1578
tcp_getsockopt+0x82/0xd0 net/ipv4/tcp.c:3131
sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2709
SYSC_getsockopt net/socket.c:1829 [inline]
SyS_getsockopt+0x252/0x390 net/socket.c:1811
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x4458d9
RSP: 002b:00007fe87f452b58 EFLAGS: 00000286 ORIG_RAX: 0000000000000037
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00000000004458d9
RDX: 000000000000000e RSI: 0000000000000000 RDI: 0000000000000005
RBP: 00000000006e0020 R08: 0000000020db6000 R09: 0000000000000000
R10: 00000000207e8000 R11: 0000000000000286 R12: 0000000000708150
R13: 0000000020db8000 R14: 0000000000001000 R15: 0000000000000003
Object at ffff88003d6a9658, in cache kmalloc-64 size: 64
Allocated:
PID = 20110
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:513
set_track mm/kasan/kasan.c:525 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2745
kmalloc include/linux/slab.h:490 [inline]
kzalloc include/linux/slab.h:663 [inline]
fib_create_info+0x8e0/0x3a30 net/ipv4/fib_semantics.c:1040
fib_table_insert+0x1a5/0x1550 net/ipv4/fib_trie.c:1221
ip_rt_ioctl+0xddc/0x1590 net/ipv4/fib_frontend.c:597
inet_ioctl+0xf2/0x1c0 net/ipv4/af_inet.c:882
sctp: [Deprecated]: syz-executor0 (pid 20638) Use of int in max_burst
socket option.
Use struct sctp_assoc_value instead
sock_do_ioctl+0x65/0xb0 net/socket.c:906
sock_ioctl+0x28f/0x440 net/socket.c:1004
vfs_ioctl fs/ioctl.c:45 [inline]
do_vfs_ioctl+0x1bf/0x1780 fs/ioctl.c:685
SYSC_ioctl fs/ioctl.c:700 [inline]
SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
entry_SYSCALL_64_fastpath+0x1f/0xc2
Freed:
PID = 4439
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:513
set_track mm/kasan/kasan.c:525 [inline]
kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
slab_free_hook mm/slub.c:1357 [inline]
slab_free_freelist_hook mm/slub.c:1379 [inline]
slab_free mm/slub.c:2961 [inline]
kfree+0xe8/0x2b0 mm/slub.c:3882
free_fib_info_rcu+0x4ba/0x5e0 net/ipv4/fib_semantics.c:218
__rcu_reclaim kernel/rcu/rcu.h:118 [inline]
rcu_do_batch.isra.64+0x947/0xcc0 kernel/rcu/tree.c:2879
invoke_rcu_callbacks kernel/rcu/tree.c:3142 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:3109 [inline]
rcu_process_callbacks+0x2cc/0xb90 kernel/rcu/tree.c:3126
__do_softirq+0x2fb/0xb7d kernel/softirq.c:284
Memory state around the buggy address:
ffff88003d6a9500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88003d6a9580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88003d6a9600: fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb fb
^
ffff88003d6a9680: fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88003d6a9700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


2017-04-04 18:51:48

by Eric Dumazet

[permalink] [raw]
Subject: Re: net/ipv4: use-after-free in ipv4_mtu

On Tue, Apr 4, 2017 at 7:50 AM, Andrey Konovalov <[email protected]> wrote:
>
> Hi,
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> On commit a71c9a1c779f2499fb2afc0553e543f18aff6edf (4.11-rc5).
>
> Unfortunately it's not reproducible.
>
> ==================================================================
> BUG: KASAN: use-after-free in dst_metric_raw include/net/dst.h:176
> [inline] at addr ffff88003d6a965c
> BUG: KASAN: use-after-free in ipv4_mtu+0x3f2/0x4b0
> net/ipv4/route.c:1270 at addr ffff88003d6a965c
> Read of size 4 by task syz-executor3/20611
> CPU: 3 PID: 20611 Comm: syz-executor3 Not tainted 4.11.0-rc5+ #199
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:16 [inline]
> dump_stack+0x292/0x398 lib/dump_stack.c:52
> kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
> print_address_description mm/kasan/report.c:202 [inline]
> kasan_report_error mm/kasan/report.c:291 [inline]
> kasan_report+0x252/0x510 mm/kasan/report.c:347
> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:367
> dst_metric_raw include/net/dst.h:176 [inline]
> ipv4_mtu+0x3f2/0x4b0 net/ipv4/route.c:1270
> dst_mtu include/net/dst.h:221 [inline]
> do_ip_getsockopt+0x71d/0x2290 net/ipv4/ip_sockglue.c:1433
> ip_getsockopt+0x90/0x230 net/ipv4/ip_sockglue.c:1578
> tcp_getsockopt+0x82/0xd0 net/ipv4/tcp.c:3131
> sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2709
> SYSC_getsockopt net/socket.c:1829 [inline]
> SyS_getsockopt+0x252/0x390 net/socket.c:1811
> entry_SYSCALL_64_fastpath+0x1f/0xc2
> RIP: 0033:0x4458d9
> RSP: 002b:00007fe87f452b58 EFLAGS: 00000286 ORIG_RAX: 0000000000000037
> RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00000000004458d9
> RDX: 000000000000000e RSI: 0000000000000000 RDI: 0000000000000005
> RBP: 00000000006e0020 R08: 0000000020db6000 R09: 0000000000000000
> R10: 00000000207e8000 R11: 0000000000000286 R12: 0000000000708150
> R13: 0000000020db8000 R14: 0000000000001000 R15: 0000000000000003
> Object at ffff88003d6a9658, in cache kmalloc-64 size: 64
> Allocated:
> PID = 20110
> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> set_track mm/kasan/kasan.c:525 [inline]
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
> kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2745
> kmalloc include/linux/slab.h:490 [inline]
> kzalloc include/linux/slab.h:663 [inline]
> fib_create_info+0x8e0/0x3a30 net/ipv4/fib_semantics.c:1040
> fib_table_insert+0x1a5/0x1550 net/ipv4/fib_trie.c:1221
> ip_rt_ioctl+0xddc/0x1590 net/ipv4/fib_frontend.c:597
> inet_ioctl+0xf2/0x1c0 net/ipv4/af_inet.c:882
> sctp: [Deprecated]: syz-executor0 (pid 20638) Use of int in max_burst
> socket option.
> Use struct sctp_assoc_value instead
> sock_do_ioctl+0x65/0xb0 net/socket.c:906
> sock_ioctl+0x28f/0x440 net/socket.c:1004
> vfs_ioctl fs/ioctl.c:45 [inline]
> do_vfs_ioctl+0x1bf/0x1780 fs/ioctl.c:685
> SYSC_ioctl fs/ioctl.c:700 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
> entry_SYSCALL_64_fastpath+0x1f/0xc2
> Freed:
> PID = 4439
> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> set_track mm/kasan/kasan.c:525 [inline]
> kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
> slab_free_hook mm/slub.c:1357 [inline]
> slab_free_freelist_hook mm/slub.c:1379 [inline]
> slab_free mm/slub.c:2961 [inline]
> kfree+0xe8/0x2b0 mm/slub.c:3882
> free_fib_info_rcu+0x4ba/0x5e0 net/ipv4/fib_semantics.c:218
> __rcu_reclaim kernel/rcu/rcu.h:118 [inline]
> rcu_do_batch.isra.64+0x947/0xcc0 kernel/rcu/tree.c:2879
> invoke_rcu_callbacks kernel/rcu/tree.c:3142 [inline]
> __rcu_process_callbacks kernel/rcu/tree.c:3109 [inline]
> rcu_process_callbacks+0x2cc/0xb90 kernel/rcu/tree.c:3126
> __do_softirq+0x2fb/0xb7d kernel/softirq.c:284
> Memory state around the buggy address:
> ffff88003d6a9500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88003d6a9580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >ffff88003d6a9600: fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb fb
> ^
> ffff88003d6a9680: fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88003d6a9700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==================================================================

Thanks for the report Andrey

Looking at fib->fib_metrics, I fail to understand how the following can work :

dst_init_metrics(&rt->dst, fi->fib_metrics, true);

In the cases fi->fib_metrics is _not_ dst_default_metrics,
fi->fib_metrics can be freed when the fib is deleted,
while dst(s) have still the 'read only pointer'.

RCU grace period before fi->fib_metrics freeing does not help.

Without refcounts, it looks like we need to copy the fib_metrics.

2017-04-05 01:11:43

by Cong Wang

[permalink] [raw]
Subject: Re: net/ipv4: use-after-free in ipv4_mtu

On Tue, Apr 4, 2017 at 11:51 AM, Eric Dumazet <[email protected]> wrote:
> On Tue, Apr 4, 2017 at 7:50 AM, Andrey Konovalov <[email protected]> wrote:
>>
>> Hi,
>>
>> I've got the following error report while fuzzing the kernel with syzkaller.
>>
>> On commit a71c9a1c779f2499fb2afc0553e543f18aff6edf (4.11-rc5).
>>
>> Unfortunately it's not reproducible.
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in dst_metric_raw include/net/dst.h:176
>> [inline] at addr ffff88003d6a965c
>> BUG: KASAN: use-after-free in ipv4_mtu+0x3f2/0x4b0
>> net/ipv4/route.c:1270 at addr ffff88003d6a965c
>> Read of size 4 by task syz-executor3/20611
>> CPU: 3 PID: 20611 Comm: syz-executor3 Not tainted 4.11.0-rc5+ #199
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> Call Trace:
>> __dump_stack lib/dump_stack.c:16 [inline]
>> dump_stack+0x292/0x398 lib/dump_stack.c:52
>> kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
>> print_address_description mm/kasan/report.c:202 [inline]
>> kasan_report_error mm/kasan/report.c:291 [inline]
>> kasan_report+0x252/0x510 mm/kasan/report.c:347
>> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:367
>> dst_metric_raw include/net/dst.h:176 [inline]
>> ipv4_mtu+0x3f2/0x4b0 net/ipv4/route.c:1270
>> dst_mtu include/net/dst.h:221 [inline]
>> do_ip_getsockopt+0x71d/0x2290 net/ipv4/ip_sockglue.c:1433
>> ip_getsockopt+0x90/0x230 net/ipv4/ip_sockglue.c:1578
>> tcp_getsockopt+0x82/0xd0 net/ipv4/tcp.c:3131
>> sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2709
>> SYSC_getsockopt net/socket.c:1829 [inline]
>> SyS_getsockopt+0x252/0x390 net/socket.c:1811
>> entry_SYSCALL_64_fastpath+0x1f/0xc2
>> RIP: 0033:0x4458d9
>> RSP: 002b:00007fe87f452b58 EFLAGS: 00000286 ORIG_RAX: 0000000000000037
>> RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00000000004458d9
>> RDX: 000000000000000e RSI: 0000000000000000 RDI: 0000000000000005
>> RBP: 00000000006e0020 R08: 0000000020db6000 R09: 0000000000000000
>> R10: 00000000207e8000 R11: 0000000000000286 R12: 0000000000708150
>> R13: 0000000020db8000 R14: 0000000000001000 R15: 0000000000000003
>> Object at ffff88003d6a9658, in cache kmalloc-64 size: 64
>> Allocated:
>> PID = 20110
>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>> set_track mm/kasan/kasan.c:525 [inline]
>> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
>> kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2745
>> kmalloc include/linux/slab.h:490 [inline]
>> kzalloc include/linux/slab.h:663 [inline]
>> fib_create_info+0x8e0/0x3a30 net/ipv4/fib_semantics.c:1040
>> fib_table_insert+0x1a5/0x1550 net/ipv4/fib_trie.c:1221
>> ip_rt_ioctl+0xddc/0x1590 net/ipv4/fib_frontend.c:597
>> inet_ioctl+0xf2/0x1c0 net/ipv4/af_inet.c:882
>> sctp: [Deprecated]: syz-executor0 (pid 20638) Use of int in max_burst
>> socket option.
>> Use struct sctp_assoc_value instead
>> sock_do_ioctl+0x65/0xb0 net/socket.c:906
>> sock_ioctl+0x28f/0x440 net/socket.c:1004
>> vfs_ioctl fs/ioctl.c:45 [inline]
>> do_vfs_ioctl+0x1bf/0x1780 fs/ioctl.c:685
>> SYSC_ioctl fs/ioctl.c:700 [inline]
>> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
>> entry_SYSCALL_64_fastpath+0x1f/0xc2
>> Freed:
>> PID = 4439
>> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
>> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>> set_track mm/kasan/kasan.c:525 [inline]
>> kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
>> slab_free_hook mm/slub.c:1357 [inline]
>> slab_free_freelist_hook mm/slub.c:1379 [inline]
>> slab_free mm/slub.c:2961 [inline]
>> kfree+0xe8/0x2b0 mm/slub.c:3882
>> free_fib_info_rcu+0x4ba/0x5e0 net/ipv4/fib_semantics.c:218
>> __rcu_reclaim kernel/rcu/rcu.h:118 [inline]
>> rcu_do_batch.isra.64+0x947/0xcc0 kernel/rcu/tree.c:2879
>> invoke_rcu_callbacks kernel/rcu/tree.c:3142 [inline]
>> __rcu_process_callbacks kernel/rcu/tree.c:3109 [inline]
>> rcu_process_callbacks+0x2cc/0xb90 kernel/rcu/tree.c:3126
>> __do_softirq+0x2fb/0xb7d kernel/softirq.c:284
>> Memory state around the buggy address:
>> ffff88003d6a9500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88003d6a9580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> >ffff88003d6a9600: fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb fb
>> ^
>> ffff88003d6a9680: fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88003d6a9700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ==================================================================
>
> Thanks for the report Andrey
>
> Looking at fib->fib_metrics, I fail to understand how the following can work :
>
> dst_init_metrics(&rt->dst, fi->fib_metrics, true);
>
> In the cases fi->fib_metrics is _not_ dst_default_metrics,
> fi->fib_metrics can be freed when the fib is deleted,
> while dst(s) have still the 'read only pointer'.
>
> RCU grace period before fi->fib_metrics freeing does not help.
>
> Without refcounts, it looks like we need to copy the fib_metrics.

The dst is obtained from sk_dst_cache which is cached for a fast
path where fib_info is obtained in fib_lookup() without refcnt:

err = fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF);


...
if (!(fib_flags & FIB_LOOKUP_NOREF))
atomic_inc(&fi->fib_clntref);


This probably starts from:

commit ebc0ffae5dfb4447e0a431ffe7fe1d467c48bbb9
Author: Eric Dumazet <[email protected]>
Date: Tue Oct 5 10:41:36 2010 +0000

fib: RCU conversion of fib_lookup()

2017-04-05 02:45:43

by Eric Dumazet

[permalink] [raw]
Subject: Re: net/ipv4: use-after-free in ipv4_mtu

On Tue, 2017-04-04 at 18:11 -0700, Cong Wang wrote:
> On Tue, Apr 4, 2017 at 11:51 AM, Eric Dumazet <[email protected]> wrote:
> > On Tue, Apr 4, 2017 at 7:50 AM, Andrey Konovalov <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> I've got the following error report while fuzzing the kernel with syzkaller.
> >>
> >> On commit a71c9a1c779f2499fb2afc0553e543f18aff6edf (4.11-rc5).
> >>
> >> Unfortunately it's not reproducible.
> >>
> >> ==================================================================
> >> BUG: KASAN: use-after-free in dst_metric_raw include/net/dst.h:176
> >> [inline] at addr ffff88003d6a965c
> >> BUG: KASAN: use-after-free in ipv4_mtu+0x3f2/0x4b0
> >> net/ipv4/route.c:1270 at addr ffff88003d6a965c
> >> Read of size 4 by task syz-executor3/20611
> >> CPU: 3 PID: 20611 Comm: syz-executor3 Not tainted 4.11.0-rc5+ #199
> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> >> Call Trace:
> >> __dump_stack lib/dump_stack.c:16 [inline]
> >> dump_stack+0x292/0x398 lib/dump_stack.c:52
> >> kasan_object_err+0x1c/0x70 mm/kasan/report.c:164
> >> print_address_description mm/kasan/report.c:202 [inline]
> >> kasan_report_error mm/kasan/report.c:291 [inline]
> >> kasan_report+0x252/0x510 mm/kasan/report.c:347
> >> __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:367
> >> dst_metric_raw include/net/dst.h:176 [inline]
> >> ipv4_mtu+0x3f2/0x4b0 net/ipv4/route.c:1270
> >> dst_mtu include/net/dst.h:221 [inline]
> >> do_ip_getsockopt+0x71d/0x2290 net/ipv4/ip_sockglue.c:1433
> >> ip_getsockopt+0x90/0x230 net/ipv4/ip_sockglue.c:1578
> >> tcp_getsockopt+0x82/0xd0 net/ipv4/tcp.c:3131
> >> sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2709
> >> SYSC_getsockopt net/socket.c:1829 [inline]
> >> SyS_getsockopt+0x252/0x390 net/socket.c:1811
> >> entry_SYSCALL_64_fastpath+0x1f/0xc2
> >> RIP: 0033:0x4458d9
> >> RSP: 002b:00007fe87f452b58 EFLAGS: 00000286 ORIG_RAX: 0000000000000037
> >> RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00000000004458d9
> >> RDX: 000000000000000e RSI: 0000000000000000 RDI: 0000000000000005
> >> RBP: 00000000006e0020 R08: 0000000020db6000 R09: 0000000000000000
> >> R10: 00000000207e8000 R11: 0000000000000286 R12: 0000000000708150
> >> R13: 0000000020db8000 R14: 0000000000001000 R15: 0000000000000003
> >> Object at ffff88003d6a9658, in cache kmalloc-64 size: 64
> >> Allocated:
> >> PID = 20110
> >> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> >> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> >> set_track mm/kasan/kasan.c:525 [inline]
> >> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:616
> >> kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2745
> >> kmalloc include/linux/slab.h:490 [inline]
> >> kzalloc include/linux/slab.h:663 [inline]
> >> fib_create_info+0x8e0/0x3a30 net/ipv4/fib_semantics.c:1040
> >> fib_table_insert+0x1a5/0x1550 net/ipv4/fib_trie.c:1221
> >> ip_rt_ioctl+0xddc/0x1590 net/ipv4/fib_frontend.c:597
> >> inet_ioctl+0xf2/0x1c0 net/ipv4/af_inet.c:882
> >> sctp: [Deprecated]: syz-executor0 (pid 20638) Use of int in max_burst
> >> socket option.
> >> Use struct sctp_assoc_value instead
> >> sock_do_ioctl+0x65/0xb0 net/socket.c:906
> >> sock_ioctl+0x28f/0x440 net/socket.c:1004
> >> vfs_ioctl fs/ioctl.c:45 [inline]
> >> do_vfs_ioctl+0x1bf/0x1780 fs/ioctl.c:685
> >> SYSC_ioctl fs/ioctl.c:700 [inline]
> >> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
> >> entry_SYSCALL_64_fastpath+0x1f/0xc2
> >> Freed:
> >> PID = 4439
> >> save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
> >> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> >> set_track mm/kasan/kasan.c:525 [inline]
> >> kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:589
> >> slab_free_hook mm/slub.c:1357 [inline]
> >> slab_free_freelist_hook mm/slub.c:1379 [inline]
> >> slab_free mm/slub.c:2961 [inline]
> >> kfree+0xe8/0x2b0 mm/slub.c:3882
> >> free_fib_info_rcu+0x4ba/0x5e0 net/ipv4/fib_semantics.c:218
> >> __rcu_reclaim kernel/rcu/rcu.h:118 [inline]
> >> rcu_do_batch.isra.64+0x947/0xcc0 kernel/rcu/tree.c:2879
> >> invoke_rcu_callbacks kernel/rcu/tree.c:3142 [inline]
> >> __rcu_process_callbacks kernel/rcu/tree.c:3109 [inline]
> >> rcu_process_callbacks+0x2cc/0xb90 kernel/rcu/tree.c:3126
> >> __do_softirq+0x2fb/0xb7d kernel/softirq.c:284
> >> Memory state around the buggy address:
> >> ffff88003d6a9500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >> ffff88003d6a9580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >> >ffff88003d6a9600: fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb fb
> >> ^
> >> ffff88003d6a9680: fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc
> >> ffff88003d6a9700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >> ==================================================================
> >
> > Thanks for the report Andrey
> >
> > Looking at fib->fib_metrics, I fail to understand how the following can work :
> >
> > dst_init_metrics(&rt->dst, fi->fib_metrics, true);
> >
> > In the cases fi->fib_metrics is _not_ dst_default_metrics,
> > fi->fib_metrics can be freed when the fib is deleted,
> > while dst(s) have still the 'read only pointer'.
> >
> > RCU grace period before fi->fib_metrics freeing does not help.
> >
> > Without refcounts, it looks like we need to copy the fib_metrics.
>
> The dst is obtained from sk_dst_cache which is cached for a fast
> path where fib_info is obtained in fib_lookup() without refcnt:
>
> err = fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF);
>
>
> ...
> if (!(fib_flags & FIB_LOOKUP_NOREF))
> atomic_inc(&fi->fib_clntref);
>
>
> This probably starts from:
>
> commit ebc0ffae5dfb4447e0a431ffe7fe1d467c48bbb9
> Author: Eric Dumazet <[email protected]>
> Date: Tue Oct 5 10:41:36 2010 +0000
>
> fib: RCU conversion of fib_lookup()

Interesting. I might had too many beers tonight, but ...

refcount was removed in 2860583fe840 many months later

-static void rt_init_metrics(struct rtable *rt, struct fib_info *fi)
-{
- if (fi->fib_metrics != (u32 *) dst_default_metrics) {
- rt->fi = fi;
- atomic_inc(&fi->fib_clntref);
- }
- dst_init_metrics(&rt->dst, fi->fib_metrics, true);
-}
-
static struct fib_nh_exception *find_exception(struct fib_nh *nh,
__be32 daddr)
{
struct fnhe_hash_bucket *hash = nh->nh_exceptions;
@@ -1261,7 +1239,7 @@ static void rt_set_nexthop(struct rtable *rt,
__be32 daddr,
rt->rt_gateway = nh->nh_gw;
if (unlikely(fnhe))
rt_bind_exception(rt, fnhe, daddr);
- rt_init_metrics(rt, fi);
+ dst_init_metrics(&rt->dst, fi->fib_metrics, true);
#ifdef CONFIG_IP_ROUTE_CLASSID
rt->dst.tclassid = nh->nh_tclassid;
#endif



Subject: Re: net/ipv4: use-after-free in ipv4_mtu

>
> Interesting. I might had too many beers tonight, but ...
>
> refcount was removed in 2860583fe840 many months later
>
> -static void rt_init_metrics(struct rtable *rt, struct fib_info *fi)
> -{
> - if (fi->fib_metrics != (u32 *) dst_default_metrics) {
> - rt->fi = fi;
> - atomic_inc(&fi->fib_clntref);
> - }
> - dst_init_metrics(&rt->dst, fi->fib_metrics, true);
> -}
> -
> static struct fib_nh_exception *find_exception(struct fib_nh *nh,
> __be32 daddr)
> {
> struct fnhe_hash_bucket *hash = nh->nh_exceptions;
> @@ -1261,7 +1239,7 @@ static void rt_set_nexthop(struct rtable *rt,
> __be32 daddr,
> rt->rt_gateway = nh->nh_gw;
> if (unlikely(fnhe))
> rt_bind_exception(rt, fnhe, daddr);
> - rt_init_metrics(rt, fi);
> + dst_init_metrics(&rt->dst, fi->fib_metrics, true);
> #ifdef CONFIG_IP_ROUTE_CLASSID
> rt->dst.tclassid = nh->nh_tclassid;
> #endif

Hi Eric

I encountered a crash on 4.4 kernel pointing to ipv4_mtu.
Is the crash similar to this one?
(target is ARM64 Android, was seen on a stability rack, so no reproducer
unfortunately)

<6> Kernel BUG at 00000000000005dc [verbose debug info unavailable]
<6> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
<6> CPU: 1 PID: 4649 Comm: iperf Tainted: G W O 4.4.21+ #1
<6> task: ffffffef02242f00 ti: ffffffef021b8000 task.ti:
ffffffef021b8000
<2> PC is at 0x5dc
<2> LR is at ipv4_mtu+0x70/0x84
<2> pc : [<00000000000005dc>] lr : [<ffffff9bd2c35ab8>] pstate: a0000145
<2> sp : ffffffef021bb9b0
<2> x29: ffffffef021bb9b0 x28: 0000000000000000
<2> x27: ffffffef318122c0 x26: 00000000000005be
<2> x25: ffffffef31812678 x24: ffffffef31812678
<2> x23: ffffffef8794c000 x22: ffffff9bd43f4380
<2> x21: ffffffef318122c0 x20: ffffffef6aef6ac0
<2> x19: ffffffef05026ac0 x18: 0000000001026749
<2> x17: 0000007fabaf145c x16: ffffff9bd1fe72bc
<2> x15: 00368fbefea52a8e x14: 3736353433323130
<2> x13: 3938373635343332 x12: 0000000000000003
<2> x11: 0000000000000028 x10: 0101010101010101
<2> x9 : 0000000000000001 x8 : 0000000000000098
<2> x7 : ffffff9bd2c8cbc0 x6 : 0000000000000000
<2> x5 : ffffffef68481c00 x4 : 00000000ffffefbf
<2> x3 : 0000000000000000 x2 : 0000000000000000
<2> x1 : 000000000000ef7f x0 : 0000000001280058
<2>
LR: 0xffffff9bd2c35a78:
<2> 5a78 b7f80241 f9401661 927ef421 b9400422 2a0203e0 350001a2 f9400e60
b9400021
<2> 5a98 b9422800 361000c1 39428e61 34000081 7109001f 52804801 1a819000
529fffe1
<2> 5ab8 6b01001f 1a819000 f9400bf3 a8c27bfd d65f03c0 a9ba7bfd 910003fd
a90153f3
<2> 5ad8 a9025bf5 a90363f7 a9046bf9 aa0003f3 aa1e03e0 f9002fa1 2a0203f8
2a0303f9
<2>
SP: 0xffffffef021bb970:
<2> b970 d2c35ab8 ffffff9b 021bb9b0 ffffffef 000005dc 00000000 a0000145
00000000
<2> b990 6aef6ac0 ffffffef 6aef6ac0 ffffffef 00000000 00000080 d2c015b0
ffffff9b
<2> b9b0 021bb9d0 ffffffef d2c3e4d4 ffffff9b 6aef6ac0 ffffffef 021bba18
ffffffef
<2> b9d0 021bba20 ffffffef d2c3f05c ffffff9b d37d9418 ffffff9b 6aef6ac0
ffffffef
<2>
<6> Process iperf (pid: 4649, stack limit = 0xffffffef021b8020)
<2> Call trace:
<2> [<00000000000005dc>] 0x5dc
<2> [<ffffff9bd2c3e4d4>] ip_finish_output+0xbc/0x1dc
<2> [<ffffff9bd2c3f05c>] ip_output+0xe8/0x15c
<2> [<ffffff9bd2c3e78c>] ip_local_out+0x58/0x68
<2> [<ffffff9bd2c3fa88>] ip_send_skb+0x2c/0xa8
<2> [<ffffff9bd2c643d0>] udp_send_skb+0x194/0x29c
<2> [<ffffff9bd2c66584>] udp_sendmsg+0x4e0/0x700
<2> [<ffffff9bd2c70788>] inet_sendmsg+0x98/0xc8
<2> [<ffffff9bd2ba82e8>] sock_sendmsg+0x48/0x60
<2> [<ffffff9bd2ba8394>] sock_write_iter+0x94/0xc0
<2> [<ffffff9bd1fe61c8>] __vfs_write+0xc0/0xf0
<2> [<ffffff9bd1fe6abc>] vfs_write+0xb8/0x150
<2> [<ffffff9bd1fe7314>] SyS_write+0x58/0x94
<2> [<ffffff9bd1e84e30>] el0_svc_naked+0x24/0x28
<6> Code: bad PC value
<6> ---[ end trace debf337ba02da94f ]---
<6> Kernel panic - not syncing: Fatal exception

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project

2017-04-05 22:34:11

by Cong Wang

[permalink] [raw]
Subject: Re: net/ipv4: use-after-free in ipv4_mtu

On Tue, Apr 4, 2017 at 7:45 PM, Eric Dumazet <[email protected]> wrote:
> On Tue, 2017-04-04 at 18:11 -0700, Cong Wang wrote:
>> On Tue, Apr 4, 2017 at 11:51 AM, Eric Dumazet <[email protected]> wrote:
>> > Looking at fib->fib_metrics, I fail to understand how the following can work :
>> >
>> > dst_init_metrics(&rt->dst, fi->fib_metrics, true);
>> >
>> > In the cases fi->fib_metrics is _not_ dst_default_metrics,
>> > fi->fib_metrics can be freed when the fib is deleted,
>> > while dst(s) have still the 'read only pointer'.
>> >
>> > RCU grace period before fi->fib_metrics freeing does not help.
>> >
>> > Without refcounts, it looks like we need to copy the fib_metrics.
>>
>> The dst is obtained from sk_dst_cache which is cached for a fast
>> path where fib_info is obtained in fib_lookup() without refcnt:
>>
>> err = fib_table_lookup(tb, flp, res, flags | FIB_LOOKUP_NOREF);
>>
>>
>> ...
>> if (!(fib_flags & FIB_LOOKUP_NOREF))
>> atomic_inc(&fi->fib_clntref);
>>
>>
>> This probably starts from:
>>
>> commit ebc0ffae5dfb4447e0a431ffe7fe1d467c48bbb9
>> Author: Eric Dumazet <[email protected]>
>> Date: Tue Oct 5 10:41:36 2010 +0000
>>
>> fib: RCU conversion of fib_lookup()
>
> Interesting. I might had too many beers tonight, but ...
>
> refcount was removed in 2860583fe840 many months later

Good find! I missed the refcnt in rt_set_nexthop() before that commit.

We need to revert that commit to restore the refcnt for fib_info.


Attachments:
ipv4-fib-info.diff (1.76 kB)

2017-04-06 10:49:22

by Eric Dumazet

[permalink] [raw]
Subject: Re: net/ipv4: use-after-free in ipv4_mtu

On Wed, 2017-04-05 at 15:33 -0700, Cong Wang wrote:

> Good find! I missed the refcnt in rt_set_nexthop() before that commit.
>
> We need to revert that commit to restore the refcnt for fib_info.

Well, there are other spots , in decnet and IPv6.

This is why my original mail stated the problem was in the calls to :

dst_init_metrics(&rt->dst, fi->fib_metrics, true);

Lets do not think in "reverting" spirit, but adding the missing bits.

The problem here is that the metrics should not be freed until last user
is gone.

So maybe a refcount should be added to metrics, and we do not have to
add a fib pointer again in all dsts.

Thanks.



2017-04-07 17:10:51

by Cong Wang

[permalink] [raw]
Subject: Re: net/ipv4: use-after-free in ipv4_mtu

On Thu, Apr 6, 2017 at 3:49 AM, Eric Dumazet <[email protected]> wrote:
> On Wed, 2017-04-05 at 15:33 -0700, Cong Wang wrote:
>
>> Good find! I missed the refcnt in rt_set_nexthop() before that commit.
>>
>> We need to revert that commit to restore the refcnt for fib_info.
>
> Well, there are other spots , in decnet and IPv6.

IPv6 is very different, it copies or steals the metrics from mx6_config:

static int fib6_commit_metrics(struct dst_entry *dst, struct mx6_config *mxc)
{
if (!mxc->mx)
return 0;

if (dst->flags & DST_HOST) {
u32 *mp = dst_metrics_write_ptr(dst);

if (unlikely(!mp))
return -ENOMEM;

fib6_copy_metrics(mp, mxc);
} else {
dst_init_metrics(dst, mxc->mx, false);

/* We've stolen mx now. */
mxc->mx = NULL;
}

return 0;
}

so probably doesn't need a refcnt.

Decnet has already done the refcnt'ing, see dn_fib_semantic_match().


>
> This is why my original mail stated the problem was in the calls to :
>
> dst_init_metrics(&rt->dst, fi->fib_metrics, true);
>
> Lets do not think in "reverting" spirit, but adding the missing bits.
>
> The problem here is that the metrics should not be freed until last user
> is gone.
>
> So maybe a refcount should be added to metrics, and we do not have to
> add a fib pointer again in all dsts.
>

Good point, but it is harder than just revert given that fact that dst metrics
is a magic pointer to an array and COW.