| Message ID | 20240909184827.123071-1-aha310510@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: prevent NULL pointer dereference in rt_fibinfo_free() and rt_fibinfo_free_cpus() | expand |
On Mon, Sep 9, 2024 at 8:48 PM Jeongjun Park <aha310510@gmail.com> wrote: > > rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not > verify rt->dst and use it, which will result in NULL pointer dereference. > > Therefore, to prevent this, we need to add a check for rt->dst. > > Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") > Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") > Signed-off-by: Jeongjun Park <aha310510@gmail.com> > --- As far as I can tell, your patch is a NOP, and these Fixes: tags seem random to me. Also, I am guessing this is based on a syzbot report ?
> Eric Dumazet <edumazet@google.com> wrote: > On Mon, Sep 9, 2024 at 8:48 PM Jeongjun Park <aha310510@gmail.com> wrote: >> >> rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not >> verify rt->dst and use it, which will result in NULL pointer dereference. >> >> Therefore, to prevent this, we need to add a check for rt->dst. >> >> Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") >> Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") >> Signed-off-by: Jeongjun Park <aha310510@gmail.com> >> --- > > As far as I can tell, your patch is a NOP, and these Fixes: tags seem > random to me. I somewhat agree with the opinion that the fixes tag is random. However, I think it is absolutely necessary to add a check for &rt->dst , because the existence of rt does not guarantee that &rt->dst will not be NULL. > > Also, I am guessing this is based on a syzbot report ? Yes, but it's not a bug reported to syzbot, it's a bug that I accidentally found in my syzkaller fuzzer. The report is too long to be included in the patch notes, so I'll attach it to this email. Report: Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 4694 Comm: systemd-udevd Not tainted 6.11.0-rc6-00326-gd1f2d51b711a #16 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 RIP: 0010:dst_dev_put+0x26/0x330 net/core/dst.c:149 Code: 90 90 90 90 f3 0f 1e fa 41 57 41 56 49 89 fe 41 55 41 54 55 e8 0b 90 af f8 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 da 02 00 00 49 8d 7e 3a 4d 8b 26 48 b8 00 00 00 RSP: 0018:ffffc90000007d68 EFLAGS: 00010246 RAX: dffffc0000000000 RBX: 0000000000000001 RCX: ffffffff8976519a RDX: 0000000000000000 RSI: ffffffff88d97a95 RDI: 0000000000000001 RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed100c8020c3 R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff1ab5a81 R13: 0000607f8106c5c8 R14: 0000000000000001 R15: 0000000000000000 FS: 00007f235c5fd8c0(0000) GS:ffff88802c400000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f39c90e5168 CR3: 0000000019688000 CR4: 0000000000750ef0 PKRU: 55555554 Call Trace: <IRQ> rt_fibinfo_free_cpus.part.0+0xf4/0x1d0 net/ipv4/fib_semantics.c:206 rt_fibinfo_free_cpus net/ipv4/fib_semantics.c:198 [inline] fib_nh_common_release+0x121/0x360 net/ipv4/fib_semantics.c:217 fib_nh_release net/ipv4/fib_semantics.c:229 [inline] free_fib_info_rcu+0x18f/0x4b0 net/ipv4/fib_semantics.c:241 rcu_do_batch kernel/rcu/tree.c:2569 [inline] rcu_core+0x826/0x16d0 kernel/rcu/tree.c:2843 handle_softirqs+0x1d4/0x870 kernel/softirq.c:554 __do_softirq kernel/softirq.c:588 [inline] invoke_softirq kernel/softirq.c:428 [inline] __irq_exit_rcu kernel/softirq.c:637 [inline] irq_exit_rcu+0xbb/0x120 kernel/softirq.c:649 instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1043 [inline] sysvec_apic_timer_interrupt+0x99/0xb0 arch/x86/kernel/apic/apic.c:1043 </IRQ> <TASK> asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:702 RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:152 [inline] RIP: 0010:_raw_spin_unlock_irqrestore+0x3c/0x70 kernel/locking/spinlock.c:194 Code: 74 24 10 e8 d6 03 6f f6 48 89 ef e8 8e 77 6f f6 81 e3 00 02 00 00 75 29 9c 58 f6 c4 02 75 35 48 85 db 74 01 fb bf 01 00 00 00 e8 bf 24 61 f6 65 8b 05 c0 79 0b 75 85 c0 74 0e 5b 5d c3 cc cc cc RSP: 0018:ffffc90001f978f8 EFLAGS: 00000206 RAX: 0000000000000006 RBX: 0000000000000200 RCX: 1ffffffff1fe4be9 RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000001 RBP: ffffffff8dbc0ac0 R08: 0000000000000001 R09: 0000000000000001 R10: ffffffff8ff2a39f R11: ffffffff815d29ca R12: 0000000000000246 R13: ffff88801e866100 R14: 0000000000000200 R15: 0000000000000000 rcu_read_unlock_special kernel/rcu/tree_plugin.h:691 [inline] __rcu_read_unlock+0x2d9/0x580 kernel/rcu/tree_plugin.h:436 __netlink_sendskb net/netlink/af_netlink.c:1278 [inline] netlink_broadcast_deliver net/netlink/af_netlink.c:1408 [inline] do_one_broadcast net/netlink/af_netlink.c:1495 [inline] netlink_broadcast_filtered+0x8ec/0xe00 net/netlink/af_netlink.c:1540 netlink_broadcast net/netlink/af_netlink.c:1564 [inline] netlink_sendmsg+0x9ee/0xd80 net/netlink/af_netlink.c:1899 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg net/socket.c:745 [inline] ____sys_sendmsg+0xabe/0xc80 net/socket.c:2597 ___sys_sendmsg+0x11d/0x1c0 net/socket.c:2651 __sys_sendmsg+0xfe/0x1d0 net/socket.c:2680 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xcb/0x250 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f RIP: 0033:0x7f235c8b0e13 Code: 8b 15 b9 a1 00 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 2e 00 00 00 0f 05 48 3d 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 89 54 24 1c 48 RSP: 002b:00007ffe93910d38 EFLAGS: 00000246 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 0000556a6858ba60 RCX: 00007f235c8b0e13 RDX: 0000000000000000 RSI: 00007ffe93910d60 RDI: 000000000000000e RBP: 0000556a6858bdc0 R08: 00000000ffffffff R09: 0000556a685488e0 R10: 0000556a6858be38 R11: 0000000000000246 R12: 0000000000008010 R13: 0000556a6856fc30 R14: 0000000000000000 R15: 00007ffe93910df0 </TASK> Modules linked in: ---[ end trace 0000000000000000 ]--- RIP: 0010:dst_dev_put+0x26/0x330 net/core/dst.c:149 Code: 90 90 90 90 f3 0f 1e fa 41 57 41 56 49 89 fe 41 55 41 54 55 e8 0b 90 af f8 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 da 02 00 00 49 8d 7e 3a 4d 8b 26 48 b8 00 00 00 RSP: 0018:ffffc90000007d68 EFLAGS: 00010246 RAX: dffffc0000000000 RBX: 0000000000000001 RCX: ffffffff8976519a RDX: 0000000000000000 RSI: ffffffff88d97a95 RDI: 0000000000000001 RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed100c8020c3 R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff1ab5a81 R13: 0000607f8106c5c8 R14: 0000000000000001 R15: 0000000000000000 FS: 00007f235c5fd8c0(0000) GS:ffff88802c400000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f39c90e5168 CR3: 0000000019688000 CR4: 0000000000750ef0 PKRU: 55555554 ---------------- Code disassembly (best guess): 0: 90 nop 1: 90 nop 2: 90 nop 3: 90 nop 4: f3 0f 1e fa endbr64 8: 41 57 push %r15 a: 41 56 push %r14 c: 49 89 fe mov %rdi,%r14 f: 41 55 push %r13 11: 41 54 push %r12 13: 55 push %rbp 14: e8 0b 90 af f8 call 0xf8af9024 19: 4c 89 f2 mov %r14,%rdx 1c: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax 23: fc ff df 26: 48 c1 ea 03 shr $0x3,%rdx * 2a: 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) <-- trapping instruction 2e: 0f 85 da 02 00 00 jne 0x30e 34: 49 8d 7e 3a lea 0x3a(%r14),%rdi 38: 4d 8b 26 mov (%r14),%r12 3b: 48 rex.W 3c: b8 .byte 0xb8 3d: 00 00 add %al,(%rax)
On Tue, Sep 10, 2024 at 5:23 AM Jeongjun Park <aha310510@gmail.com> wrote: > > > > Eric Dumazet <edumazet@google.com> wrote: > > On Mon, Sep 9, 2024 at 8:48 PM Jeongjun Park <aha310510@gmail.com> wrote: > >> > >> rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not > >> verify rt->dst and use it, which will result in NULL pointer dereference. > >> > >> Therefore, to prevent this, we need to add a check for rt->dst. > >> > >> Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") > >> Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") > >> Signed-off-by: Jeongjun Park <aha310510@gmail.com> > >> --- > > > > As far as I can tell, your patch is a NOP, and these Fixes: tags seem > > random to me. > > I somewhat agree with the opinion that the fixes tag is random. > However, I think it is absolutely necessary to add a check for > &rt->dst , because the existence of rt does not guarantee that > &rt->dst will not be NULL. dst is the first field of 'struct rtable'. Always has been. Unless your compiler is buggy, (void *)rt == (void *)&rt->dst
On Tue, Sep 10, 2024 at 5:23 AM Jeongjun Park <aha310510@gmail.com> wrote: > > > > Eric Dumazet <edumazet@google.com> wrote: > > On Mon, Sep 9, 2024 at 8:48 PM Jeongjun Park <aha310510@gmail.com> wrote: > >> > >> rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not > >> verify rt->dst and use it, which will result in NULL pointer dereference. > >> > >> Therefore, to prevent this, we need to add a check for rt->dst. > >> > >> Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") > >> Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") > >> Signed-off-by: Jeongjun Park <aha310510@gmail.com> > >> --- > > > > As far as I can tell, your patch is a NOP, and these Fixes: tags seem > > random to me. > > I somewhat agree with the opinion that the fixes tag is random. > However, I think it is absolutely necessary to add a check for > &rt->dst , because the existence of rt does not guarantee that > &rt->dst will not be NULL. > > > > > Also, I am guessing this is based on a syzbot report ? > > Yes, but it's not a bug reported to syzbot, it's a bug that > I accidentally found in my syzkaller fuzzer. The report is too long > to be included in the patch notes, so I'll attach it to this email. syzbot has a similar report in its queue, I put it on hold because this is some unrelated memory corruption. rt (R14 in your case) is 0x1 at this point, which is not a valid memory pointer. So I am definitely saying no to your patch. > > Report: > > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI > KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > CPU: 0 UID: 0 PID: 4694 Comm: systemd-udevd Not tainted 6.11.0-rc6-00326-gd1f2d51b711a #16 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:dst_dev_put+0x26/0x330 net/core/dst.c:149 > Code: 90 90 90 90 f3 0f 1e fa 41 57 41 56 49 89 fe 41 55 41 54 55 e8 0b 90 af f8 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 da 02 00 00 49 8d 7e 3a 4d 8b 26 48 b8 00 00 00 > RSP: 0018:ffffc90000007d68 EFLAGS: 00010246 > RAX: dffffc0000000000 RBX: 0000000000000001 RCX: ffffffff8976519a > RDX: 0000000000000000 RSI: ffffffff88d97a95 RDI: 0000000000000001 > RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed100c8020c3 > R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff1ab5a81 > R13: 0000607f8106c5c8 R14: 0000000000000001 R15: 0000000000000000 > FS: 00007f235c5fd8c0(0000) GS:ffff88802c400000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f39c90e5168 CR3: 0000000019688000 CR4: 0000000000750ef0 > PKRU: 55555554 > Call Trace: > <IRQ> > rt_fibinfo_free_cpus.part.0+0xf4/0x1d0 net/ipv4/fib_semantics.c:206 > rt_fibinfo_free_cpus net/ipv4/fib_semantics.c:198 [inline] > fib_nh_common_release+0x121/0x360 net/ipv4/fib_semantics.c:217 > fib_nh_release net/ipv4/fib_semantics.c:229 [inline] > free_fib_info_rcu+0x18f/0x4b0 net/ipv4/fib_semantics.c:241 > rcu_do_batch kernel/rcu/tree.c:2569 [inline] > rcu_core+0x826/0x16d0 kernel/rcu/tree.c:2843 > handle_softirqs+0x1d4/0x870 kernel/softirq.c:554 > __do_softirq kernel/softirq.c:588 [inline] > invoke_softirq kernel/softirq.c:428 [inline] > __irq_exit_rcu kernel/softirq.c:637 [inline] > irq_exit_rcu+0xbb/0x120 kernel/softirq.c:649 > instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1043 [inline] > sysvec_apic_timer_interrupt+0x99/0xb0 arch/x86/kernel/apic/apic.c:1043 > </IRQ> > <TASK> > asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:702 > RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:152 [inline] > RIP: 0010:_raw_spin_unlock_irqrestore+0x3c/0x70 kernel/locking/spinlock.c:194 > Code: 74 24 10 e8 d6 03 6f f6 48 89 ef e8 8e 77 6f f6 81 e3 00 02 00 00 75 29 9c 58 f6 c4 02 75 35 48 85 db 74 01 fb bf 01 00 00 00 e8 bf 24 61 f6 65 8b 05 c0 79 0b 75 85 c0 74 0e 5b 5d c3 cc cc cc > RSP: 0018:ffffc90001f978f8 EFLAGS: 00000206 > RAX: 0000000000000006 RBX: 0000000000000200 RCX: 1ffffffff1fe4be9 > RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000001 > RBP: ffffffff8dbc0ac0 R08: 0000000000000001 R09: 0000000000000001 > R10: ffffffff8ff2a39f R11: ffffffff815d29ca R12: 0000000000000246 > R13: ffff88801e866100 R14: 0000000000000200 R15: 0000000000000000 > rcu_read_unlock_special kernel/rcu/tree_plugin.h:691 [inline] > __rcu_read_unlock+0x2d9/0x580 kernel/rcu/tree_plugin.h:436 > __netlink_sendskb net/netlink/af_netlink.c:1278 [inline] > netlink_broadcast_deliver net/netlink/af_netlink.c:1408 [inline] > do_one_broadcast net/netlink/af_netlink.c:1495 [inline] > netlink_broadcast_filtered+0x8ec/0xe00 net/netlink/af_netlink.c:1540 > netlink_broadcast net/netlink/af_netlink.c:1564 [inline] > netlink_sendmsg+0x9ee/0xd80 net/netlink/af_netlink.c:1899 > sock_sendmsg_nosec net/socket.c:730 [inline] > __sock_sendmsg net/socket.c:745 [inline] > ____sys_sendmsg+0xabe/0xc80 net/socket.c:2597 > ___sys_sendmsg+0x11d/0x1c0 net/socket.c:2651 > __sys_sendmsg+0xfe/0x1d0 net/socket.c:2680 > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > do_syscall_64+0xcb/0x250 arch/x86/entry/common.c:83 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > RIP: 0033:0x7f235c8b0e13 > Code: 8b 15 b9 a1 00 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 2e 00 00 00 0f 05 48 3d 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 89 54 24 1c 48 > RSP: 002b:00007ffe93910d38 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > RAX: ffffffffffffffda RBX: 0000556a6858ba60 RCX: 00007f235c8b0e13 > RDX: 0000000000000000 RSI: 00007ffe93910d60 RDI: 000000000000000e > RBP: 0000556a6858bdc0 R08: 00000000ffffffff R09: 0000556a685488e0 > R10: 0000556a6858be38 R11: 0000000000000246 R12: 0000000000008010 > R13: 0000556a6856fc30 R14: 0000000000000000 R15: 00007ffe93910df0 > </TASK> > Modules linked in: > ---[ end trace 0000000000000000 ]--- > RIP: 0010:dst_dev_put+0x26/0x330 net/core/dst.c:149 > Code: 90 90 90 90 f3 0f 1e fa 41 57 41 56 49 89 fe 41 55 41 54 55 e8 0b 90 af f8 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 da 02 00 00 49 8d 7e 3a 4d 8b 26 48 b8 00 00 00 > RSP: 0018:ffffc90000007d68 EFLAGS: 00010246 > RAX: dffffc0000000000 RBX: 0000000000000001 RCX: ffffffff8976519a > RDX: 0000000000000000 RSI: ffffffff88d97a95 RDI: 0000000000000001 > RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed100c8020c3 > R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff1ab5a81 > R13: 0000607f8106c5c8 R14: 0000000000000001 R15: 0000000000000000 > FS: 00007f235c5fd8c0(0000) GS:ffff88802c400000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f39c90e5168 CR3: 0000000019688000 CR4: 0000000000750ef0 > PKRU: 55555554 > ---------------- > Code disassembly (best guess): > 0: 90 nop > 1: 90 nop > 2: 90 nop > 3: 90 nop > 4: f3 0f 1e fa endbr64 > 8: 41 57 push %r15 > a: 41 56 push %r14 > c: 49 89 fe mov %rdi,%r14 > f: 41 55 push %r13 > 11: 41 54 push %r12 > 13: 55 push %rbp > 14: e8 0b 90 af f8 call 0xf8af9024 > 19: 4c 89 f2 mov %r14,%rdx > 1c: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax > 23: fc ff df > 26: 48 c1 ea 03 shr $0x3,%rdx > * 2a: 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) <-- trapping instruction > 2e: 0f 85 da 02 00 00 jne 0x30e > 34: 49 8d 7e 3a lea 0x3a(%r14),%rdi > 38: 4d 8b 26 mov (%r14),%r12 > 3b: 48 rex.W > 3c: b8 .byte 0xb8 > 3d: 00 00 add %al,(%rax)
Eric Dumazet <edumazet@google.com> wrote: > > On Tue, Sep 10, 2024 at 5:23 AM Jeongjun Park <aha310510@gmail.com> wrote: > > > > > > > Eric Dumazet <edumazet@google.com> wrote: > > > On Mon, Sep 9, 2024 at 8:48 PM Jeongjun Park <aha310510@gmail.com> wrote: > > >> > > >> rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not > > >> verify rt->dst and use it, which will result in NULL pointer dereference. > > >> > > >> Therefore, to prevent this, we need to add a check for rt->dst. > > >> > > >> Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") > > >> Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") > > >> Signed-off-by: Jeongjun Park <aha310510@gmail.com> > > >> --- > > > > > > As far as I can tell, your patch is a NOP, and these Fixes: tags seem > > > random to me. > > > > I somewhat agree with the opinion that the fixes tag is random. > > However, I think it is absolutely necessary to add a check for > > &rt->dst , because the existence of rt does not guarantee that > > &rt->dst will not be NULL. > > > > > > > > Also, I am guessing this is based on a syzbot report ? > > > > Yes, but it's not a bug reported to syzbot, it's a bug that > > I accidentally found in my syzkaller fuzzer. The report is too long > > to be included in the patch notes, so I'll attach it to this email. > > syzbot has a similar report in its queue, I put it on hold because > this is some unrelated memory corruption. > > rt (R14 in your case) is 0x1 at this point, which is not a valid memory pointer. > > So I am definitely saying no to your patch. > I see. Thanks to the explanation, I understood that this patch is wrong. However, while continuing to analyze this bug, I found out something. According to the rcu_dereference_protected() doc, when using rcu_dereference_protected(), it is specified that ptr should be protected using a lock, but free_fib_info_rcu() does not have any protection for the fib_info structure. I think this may cause a data-race, which modifies the values of rt and &rt->dst, causing the bug. Even if this is not the root cause, I don't think there is a reason why free_fib_info_rcu() should not be protected with fib_info_lock. What do you think about protecting the fib_info structure like the patch below? Regards, Jeongjun Park --- net/ipv4/fib_semantics.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index 2b57cd2b96e2..77431879ee39 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c @@ -234,6 +234,7 @@ static void free_fib_info_rcu(struct rcu_head *head) { struct fib_info *fi = container_of(head, struct fib_info, rcu); + spin_lock(&fib_info_lock); if (fi->nh) { nexthop_put(fi->nh); } else { @@ -245,6 +246,7 @@ static void free_fib_info_rcu(struct rcu_head *head) ip_fib_metrics_put(fi->fib_metrics); kfree(fi); + spin_unlock(&fib_info_lock); } void free_fib_info(struct fib_info *fi) -- > > > > Report: > > > > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI > > KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > > CPU: 0 UID: 0 PID: 4694 Comm: systemd-udevd Not tainted 6.11.0-rc6-00326-gd1f2d51b711a #16 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > > RIP: 0010:dst_dev_put+0x26/0x330 net/core/dst.c:149 > > Code: 90 90 90 90 f3 0f 1e fa 41 57 41 56 49 89 fe 41 55 41 54 55 e8 0b 90 af f8 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 da 02 00 00 49 8d 7e 3a 4d 8b 26 48 b8 00 00 00 > > RSP: 0018:ffffc90000007d68 EFLAGS: 00010246 > > RAX: dffffc0000000000 RBX: 0000000000000001 RCX: ffffffff8976519a > > RDX: 0000000000000000 RSI: ffffffff88d97a95 RDI: 0000000000000001 > > RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed100c8020c3 > > R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff1ab5a81 > > R13: 0000607f8106c5c8 R14: 0000000000000001 R15: 0000000000000000 > > FS: 00007f235c5fd8c0(0000) GS:ffff88802c400000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 00007f39c90e5168 CR3: 0000000019688000 CR4: 0000000000750ef0 > > PKRU: 55555554 > > Call Trace: > > <IRQ> > > rt_fibinfo_free_cpus.part.0+0xf4/0x1d0 net/ipv4/fib_semantics.c:206 > > rt_fibinfo_free_cpus net/ipv4/fib_semantics.c:198 [inline] > > fib_nh_common_release+0x121/0x360 net/ipv4/fib_semantics.c:217 > > fib_nh_release net/ipv4/fib_semantics.c:229 [inline] > > free_fib_info_rcu+0x18f/0x4b0 net/ipv4/fib_semantics.c:241 > > rcu_do_batch kernel/rcu/tree.c:2569 [inline] > > rcu_core+0x826/0x16d0 kernel/rcu/tree.c:2843 > > handle_softirqs+0x1d4/0x870 kernel/softirq.c:554 > > __do_softirq kernel/softirq.c:588 [inline] > > invoke_softirq kernel/softirq.c:428 [inline] > > __irq_exit_rcu kernel/softirq.c:637 [inline] > > irq_exit_rcu+0xbb/0x120 kernel/softirq.c:649 > > instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1043 [inline] > > sysvec_apic_timer_interrupt+0x99/0xb0 arch/x86/kernel/apic/apic.c:1043 > > </IRQ> > > <TASK> > > asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:702 > > RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:152 [inline] > > RIP: 0010:_raw_spin_unlock_irqrestore+0x3c/0x70 kernel/locking/spinlock.c:194 > > Code: 74 24 10 e8 d6 03 6f f6 48 89 ef e8 8e 77 6f f6 81 e3 00 02 00 00 75 29 9c 58 f6 c4 02 75 35 48 85 db 74 01 fb bf 01 00 00 00 e8 bf 24 61 f6 65 8b 05 c0 79 0b 75 85 c0 74 0e 5b 5d c3 cc cc cc > > RSP: 0018:ffffc90001f978f8 EFLAGS: 00000206 > > RAX: 0000000000000006 RBX: 0000000000000200 RCX: 1ffffffff1fe4be9 > > RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000001 > > RBP: ffffffff8dbc0ac0 R08: 0000000000000001 R09: 0000000000000001 > > R10: ffffffff8ff2a39f R11: ffffffff815d29ca R12: 0000000000000246 > > R13: ffff88801e866100 R14: 0000000000000200 R15: 0000000000000000 > > rcu_read_unlock_special kernel/rcu/tree_plugin.h:691 [inline] > > __rcu_read_unlock+0x2d9/0x580 kernel/rcu/tree_plugin.h:436 > > __netlink_sendskb net/netlink/af_netlink.c:1278 [inline] > > netlink_broadcast_deliver net/netlink/af_netlink.c:1408 [inline] > > do_one_broadcast net/netlink/af_netlink.c:1495 [inline] > > netlink_broadcast_filtered+0x8ec/0xe00 net/netlink/af_netlink.c:1540 > > netlink_broadcast net/netlink/af_netlink.c:1564 [inline] > > netlink_sendmsg+0x9ee/0xd80 net/netlink/af_netlink.c:1899 > > sock_sendmsg_nosec net/socket.c:730 [inline] > > __sock_sendmsg net/socket.c:745 [inline] > > ____sys_sendmsg+0xabe/0xc80 net/socket.c:2597 > > ___sys_sendmsg+0x11d/0x1c0 net/socket.c:2651 > > __sys_sendmsg+0xfe/0x1d0 net/socket.c:2680 > > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > > do_syscall_64+0xcb/0x250 arch/x86/entry/common.c:83 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > RIP: 0033:0x7f235c8b0e13 > > Code: 8b 15 b9 a1 00 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 2e 00 00 00 0f 05 48 3d 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 89 54 24 1c 48 > > RSP: 002b:00007ffe93910d38 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > > RAX: ffffffffffffffda RBX: 0000556a6858ba60 RCX: 00007f235c8b0e13 > > RDX: 0000000000000000 RSI: 00007ffe93910d60 RDI: 000000000000000e > > RBP: 0000556a6858bdc0 R08: 00000000ffffffff R09: 0000556a685488e0 > > R10: 0000556a6858be38 R11: 0000000000000246 R12: 0000000000008010 > > R13: 0000556a6856fc30 R14: 0000000000000000 R15: 00007ffe93910df0 > > </TASK> > > Modules linked in: > > ---[ end trace 0000000000000000 ]--- > > RIP: 0010:dst_dev_put+0x26/0x330 net/core/dst.c:149 > > Code: 90 90 90 90 f3 0f 1e fa 41 57 41 56 49 89 fe 41 55 41 54 55 e8 0b 90 af f8 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 da 02 00 00 49 8d 7e 3a 4d 8b 26 48 b8 00 00 00 > > RSP: 0018:ffffc90000007d68 EFLAGS: 00010246 > > RAX: dffffc0000000000 RBX: 0000000000000001 RCX: ffffffff8976519a > > RDX: 0000000000000000 RSI: ffffffff88d97a95 RDI: 0000000000000001 > > RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed100c8020c3 > > R10: 0000000000000000 R11: 0000000000000000 R12: fffffbfff1ab5a81 > > R13: 0000607f8106c5c8 R14: 0000000000000001 R15: 0000000000000000 > > FS: 00007f235c5fd8c0(0000) GS:ffff88802c400000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 00007f39c90e5168 CR3: 0000000019688000 CR4: 0000000000750ef0 > > PKRU: 55555554 > > ---------------- > > Code disassembly (best guess): > > 0: 90 nop > > 1: 90 nop > > 2: 90 nop > > 3: 90 nop > > 4: f3 0f 1e fa endbr64 > > 8: 41 57 push %r15 > > a: 41 56 push %r14 > > c: 49 89 fe mov %rdi,%r14 > > f: 41 55 push %r13 > > 11: 41 54 push %r12 > > 13: 55 push %rbp > > 14: e8 0b 90 af f8 call 0xf8af9024 > > 19: 4c 89 f2 mov %r14,%rdx > > 1c: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax > > 23: fc ff df > > 26: 48 c1 ea 03 shr $0x3,%rdx > > * 2a: 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) <-- trapping instruction > > 2e: 0f 85 da 02 00 00 jne 0x30e > > 34: 49 8d 7e 3a lea 0x3a(%r14),%rdi > > 38: 4d 8b 26 mov (%r14),%r12 > > 3b: 48 rex.W > > 3c: b8 .byte 0xb8 > > 3d: 00 00 add %al,(%rax)
On Tue, Sep 10, 2024 at 4:06 PM Jeongjun Park <aha310510@gmail.com> wrote: > > Eric Dumazet <edumazet@google.com> wrote: > > > > On Tue, Sep 10, 2024 at 5:23 AM Jeongjun Park <aha310510@gmail.com> wrote: > > > > > > > > > > Eric Dumazet <edumazet@google.com> wrote: > > > > On Mon, Sep 9, 2024 at 8:48 PM Jeongjun Park <aha310510@gmail.com> wrote: > > > >> > > > >> rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not > > > >> verify rt->dst and use it, which will result in NULL pointer dereference. > > > >> > > > >> Therefore, to prevent this, we need to add a check for rt->dst. > > > >> > > > >> Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") > > > >> Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") > > > >> Signed-off-by: Jeongjun Park <aha310510@gmail.com> > > > >> --- > > > > > > > > As far as I can tell, your patch is a NOP, and these Fixes: tags seem > > > > random to me. > > > > > > I somewhat agree with the opinion that the fixes tag is random. > > > However, I think it is absolutely necessary to add a check for > > > &rt->dst , because the existence of rt does not guarantee that > > > &rt->dst will not be NULL. > > > > > > > > > > > Also, I am guessing this is based on a syzbot report ? > > > > > > Yes, but it's not a bug reported to syzbot, it's a bug that > > > I accidentally found in my syzkaller fuzzer. The report is too long > > > to be included in the patch notes, so I'll attach it to this email. > > > > syzbot has a similar report in its queue, I put it on hold because > > this is some unrelated memory corruption. > > > > rt (R14 in your case) is 0x1 at this point, which is not a valid memory pointer. > > > > So I am definitely saying no to your patch. > > > > I see. Thanks to the explanation, I understood that this patch is wrong. > > However, while continuing to analyze this bug, I found out something. > According to the rcu_dereference_protected() doc, when using > rcu_dereference_protected(), it is specified that ptr should be protected > using a lock, but free_fib_info_rcu() does not have any protection for > the fib_info structure. > > I think this may cause a data-race, which modifies the values of rt and > &rt->dst, causing the bug. Even if this is not the root cause, I don't > think there is a reason why free_fib_info_rcu() should not be protected > with fib_info_lock. > > What do you think about protecting the fib_info structure like the patch > below? > By the time free_fib_info_rcu() is called, we have the guarantee that no other threads/cpu can possibly use the struct fib_info. nexthop_put(), fib_nh_release(), ip_fib_metrics_put, kfree() do not need this spinlock protection. fib_info_lock has absolutely no effect here. Also, 0x1 is not a valid pointer, there is absolutely no chance a network layer could use 0x1 as a rt/dst pointer. As I said, the bug is elsewhere, something is mangling some piece of memory.
Hi Jeongjun, kernel test robot noticed the following build warnings: [auto build test WARNING on net/main] url: https://github.com/intel-lab-lkp/linux/commits/Jeongjun-Park/net-prevent-NULL-pointer-dereference-in-rt_fibinfo_free-and-rt_fibinfo_free_cpus/20240910-025008 base: net/main patch link: https://lore.kernel.org/r/20240909184827.123071-1-aha310510%40gmail.com patch subject: [PATCH net] net: prevent NULL pointer dereference in rt_fibinfo_free() and rt_fibinfo_free_cpus() config: arc-randconfig-002-20240910 (https://download.01.org/0day-ci/archive/20240910/202409102210.krjQJVRr-lkp@intel.com/config) compiler: arc-elf-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240910/202409102210.krjQJVRr-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409102210.krjQJVRr-lkp@intel.com/ All warnings (new ones prefixed by >>): net/ipv4/fib_semantics.c: In function 'rt_fibinfo_free': >> net/ipv4/fib_semantics.c:156:13: warning: the comparison will always evaluate as 'true' for the address of 'dst' will never be NULL [-Waddress] 156 | if (!&rt->dst) | ^ In file included from include/net/ip.h:30, from net/ipv4/fib_semantics.c:37: include/net/route.h:56:33: note: 'dst' declared here 56 | struct dst_entry dst; | ^~~ net/ipv4/fib_semantics.c: In function 'rt_fibinfo_free_cpus': net/ipv4/fib_semantics.c:209:21: warning: the comparison will always evaluate as 'true' for the address of 'dst' will never be NULL [-Waddress] 209 | if (!&rt->dst) | ^ include/net/route.h:56:33: note: 'dst' declared here 56 | struct dst_entry dst; | ^~~ vim +156 net/ipv4/fib_semantics.c 149 150 static void rt_fibinfo_free(struct rtable __rcu **rtp) 151 { 152 struct rtable *rt = rcu_dereference_protected(*rtp, 1); 153 154 if (!rt) 155 return; > 156 if (!&rt->dst) 157 return; 158 159 /* Not even needed : RCU_INIT_POINTER(*rtp, NULL); 160 * because we waited an RCU grace period before calling 161 * free_fib_info_rcu() 162 */ 163 164 dst_dev_put(&rt->dst); 165 dst_release_immediate(&rt->dst); 166 } 167
Hi Jeongjun,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Jeongjun-Park/net-prevent-NULL-pointer-dereference-in-rt_fibinfo_free-and-rt_fibinfo_free_cpus/20240910-025008
base: net/main
patch link: https://lore.kernel.org/r/20240909184827.123071-1-aha310510%40gmail.com
patch subject: [PATCH net] net: prevent NULL pointer dereference in rt_fibinfo_free() and rt_fibinfo_free_cpus()
config: arm-randconfig-001-20240910 (https://download.01.org/0day-ci/archive/20240911/202409110000.E9IVjdB7-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 05f5a91d00b02f4369f46d076411c700755ae041)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240911/202409110000.E9IVjdB7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409110000.E9IVjdB7-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from net/ipv4/fib_semantics.c:17:
In file included from include/linux/mm.h:2232:
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
517 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> net/ipv4/fib_semantics.c:156:12: warning: address of 'rt->dst' will always evaluate to 'true' [-Wpointer-bool-conversion]
156 | if (!&rt->dst)
| ~ ~~~~^~~
net/ipv4/fib_semantics.c:209:13: warning: address of 'rt->dst' will always evaluate to 'true' [-Wpointer-bool-conversion]
209 | if (!&rt->dst)
| ~ ~~~~^~~
3 warnings generated.
vim +156 net/ipv4/fib_semantics.c
> 17 #include <linux/mm.h>
18 #include <linux/string.h>
19 #include <linux/socket.h>
20 #include <linux/sockios.h>
21 #include <linux/errno.h>
22 #include <linux/in.h>
23 #include <linux/inet.h>
24 #include <linux/inetdevice.h>
25 #include <linux/netdevice.h>
26 #include <linux/if_arp.h>
27 #include <linux/proc_fs.h>
28 #include <linux/skbuff.h>
29 #include <linux/init.h>
30 #include <linux/slab.h>
31 #include <linux/netlink.h>
32 #include <linux/hash.h>
33 #include <linux/nospec.h>
34
35 #include <net/arp.h>
36 #include <net/inet_dscp.h>
37 #include <net/ip.h>
38 #include <net/protocol.h>
39 #include <net/route.h>
40 #include <net/tcp.h>
41 #include <net/sock.h>
42 #include <net/ip_fib.h>
43 #include <net/ip6_fib.h>
44 #include <net/nexthop.h>
45 #include <net/netlink.h>
46 #include <net/rtnh.h>
47 #include <net/lwtunnel.h>
48 #include <net/fib_notifier.h>
49 #include <net/addrconf.h>
50
51 #include "fib_lookup.h"
52
53 static DEFINE_SPINLOCK(fib_info_lock);
54 static struct hlist_head *fib_info_hash;
55 static struct hlist_head *fib_info_laddrhash;
56 static unsigned int fib_info_hash_size;
57 static unsigned int fib_info_hash_bits;
58 static unsigned int fib_info_cnt;
59
60 #define DEVINDEX_HASHBITS 8
61 #define DEVINDEX_HASHSIZE (1U << DEVINDEX_HASHBITS)
62 static struct hlist_head fib_info_devhash[DEVINDEX_HASHSIZE];
63
64 /* for_nexthops and change_nexthops only used when nexthop object
65 * is not set in a fib_info. The logic within can reference fib_nh.
66 */
67 #ifdef CONFIG_IP_ROUTE_MULTIPATH
68
69 #define for_nexthops(fi) { \
70 int nhsel; const struct fib_nh *nh; \
71 for (nhsel = 0, nh = (fi)->fib_nh; \
72 nhsel < fib_info_num_path((fi)); \
73 nh++, nhsel++)
74
75 #define change_nexthops(fi) { \
76 int nhsel; struct fib_nh *nexthop_nh; \
77 for (nhsel = 0, nexthop_nh = (struct fib_nh *)((fi)->fib_nh); \
78 nhsel < fib_info_num_path((fi)); \
79 nexthop_nh++, nhsel++)
80
81 #else /* CONFIG_IP_ROUTE_MULTIPATH */
82
83 /* Hope, that gcc will optimize it to get rid of dummy loop */
84
85 #define for_nexthops(fi) { \
86 int nhsel; const struct fib_nh *nh = (fi)->fib_nh; \
87 for (nhsel = 0; nhsel < 1; nhsel++)
88
89 #define change_nexthops(fi) { \
90 int nhsel; \
91 struct fib_nh *nexthop_nh = (struct fib_nh *)((fi)->fib_nh); \
92 for (nhsel = 0; nhsel < 1; nhsel++)
93
94 #endif /* CONFIG_IP_ROUTE_MULTIPATH */
95
96 #define endfor_nexthops(fi) }
97
98
99 const struct fib_prop fib_props[RTN_MAX + 1] = {
100 [RTN_UNSPEC] = {
101 .error = 0,
102 .scope = RT_SCOPE_NOWHERE,
103 },
104 [RTN_UNICAST] = {
105 .error = 0,
106 .scope = RT_SCOPE_UNIVERSE,
107 },
108 [RTN_LOCAL] = {
109 .error = 0,
110 .scope = RT_SCOPE_HOST,
111 },
112 [RTN_BROADCAST] = {
113 .error = 0,
114 .scope = RT_SCOPE_LINK,
115 },
116 [RTN_ANYCAST] = {
117 .error = 0,
118 .scope = RT_SCOPE_LINK,
119 },
120 [RTN_MULTICAST] = {
121 .error = 0,
122 .scope = RT_SCOPE_UNIVERSE,
123 },
124 [RTN_BLACKHOLE] = {
125 .error = -EINVAL,
126 .scope = RT_SCOPE_UNIVERSE,
127 },
128 [RTN_UNREACHABLE] = {
129 .error = -EHOSTUNREACH,
130 .scope = RT_SCOPE_UNIVERSE,
131 },
132 [RTN_PROHIBIT] = {
133 .error = -EACCES,
134 .scope = RT_SCOPE_UNIVERSE,
135 },
136 [RTN_THROW] = {
137 .error = -EAGAIN,
138 .scope = RT_SCOPE_UNIVERSE,
139 },
140 [RTN_NAT] = {
141 .error = -EINVAL,
142 .scope = RT_SCOPE_NOWHERE,
143 },
144 [RTN_XRESOLVE] = {
145 .error = -EINVAL,
146 .scope = RT_SCOPE_NOWHERE,
147 },
148 };
149
150 static void rt_fibinfo_free(struct rtable __rcu **rtp)
151 {
152 struct rtable *rt = rcu_dereference_protected(*rtp, 1);
153
154 if (!rt)
155 return;
> 156 if (!&rt->dst)
157 return;
158
159 /* Not even needed : RCU_INIT_POINTER(*rtp, NULL);
160 * because we waited an RCU grace period before calling
161 * free_fib_info_rcu()
162 */
163
164 dst_dev_put(&rt->dst);
165 dst_release_immediate(&rt->dst);
166 }
167
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index 2b57cd2b96e2..3a2a92599366 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c @@ -153,6 +153,8 @@ static void rt_fibinfo_free(struct rtable __rcu **rtp) if (!rt) return; + if (!&rt->dst) + return; /* Not even needed : RCU_INIT_POINTER(*rtp, NULL); * because we waited an RCU grace period before calling @@ -202,10 +204,13 @@ static void rt_fibinfo_free_cpus(struct rtable __rcu * __percpu *rtp) struct rtable *rt; rt = rcu_dereference_protected(*per_cpu_ptr(rtp, cpu), 1); - if (rt) { - dst_dev_put(&rt->dst); - dst_release_immediate(&rt->dst); - } + if (!rt) + continue; + if (!&rt->dst) + continue; + + dst_dev_put(&rt->dst); + dst_release_immediate(&rt->dst); } free_percpu(rtp); }
rt_fibinfo_free() and rt_fibinfo_free_cpus() only check for rt and do not verify rt->dst and use it, which will result in NULL pointer dereference. Therefore, to prevent this, we need to add a check for rt->dst. Fixes: 0830106c5390 ("ipv4: take dst->__refcnt when caching dst in fib") Fixes: c5038a8327b9 ("ipv4: Cache routes in nexthop exception entries.") Signed-off-by: Jeongjun Park <aha310510@gmail.com> --- net/ipv4/fib_semantics.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) --