| Message ID | 20211228104145.9426-1-songmuchun@bytedance.com |
|---|---|
| State | Accepted |
| Commit | e22e45fc9e41bf9fcc1e92cfb78eb92786728ef0 |
| Headers | show |
| Series | net: fix use-after-free in tw_timer_handler | expand |
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 28 Dec 2021 18:41:45 +0800 you wrote: > A real world panic issue was found as follow in Linux 5.4. > > BUG: unable to handle page fault for address: ffffde49a863de28 > PGD 7e6fe62067 P4D 7e6fe62067 PUD 7e6fe63067 PMD f51e064067 PTE 0 > RIP: 0010:tw_timer_handler+0x20/0x40 > Call Trace: > <IRQ> > call_timer_fn+0x2b/0x120 > run_timer_softirq+0x1ef/0x450 > __do_softirq+0x10d/0x2b8 > irq_exit+0xc7/0xd0 > smp_apic_timer_interrupt+0x68/0x120 > apic_timer_interrupt+0xf/0x20 > > [...] Here is the summary with links: - net: fix use-after-free in tw_timer_handler https://git.kernel.org/netdev/net/c/e22e45fc9e41 You are awesome, thank you!
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 04067b249bf3..cb8ba7a0a114 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1985,6 +1985,10 @@ static int __init inet_init(void) ip_init(); + /* Initialise per-cpu ipv4 mibs */ + if (init_ipv4_mibs()) + panic("%s: Cannot init ipv4 mibs\n", __func__); + /* Setup TCP slab cache for open requests. */ tcp_init(); @@ -2015,12 +2019,6 @@ static int __init inet_init(void) if (init_inet_pernet_ops()) pr_crit("%s: Cannot init ipv4 inet pernet ops\n", __func__); - /* - * Initialise per-cpu ipv4 mibs - */ - - if (init_ipv4_mibs()) - pr_crit("%s: Cannot init ipv4 mibs\n", __func__); ipv4_proc_init();
A real world panic issue was found as follow in Linux 5.4. BUG: unable to handle page fault for address: ffffde49a863de28 PGD 7e6fe62067 P4D 7e6fe62067 PUD 7e6fe63067 PMD f51e064067 PTE 0 RIP: 0010:tw_timer_handler+0x20/0x40 Call Trace: <IRQ> call_timer_fn+0x2b/0x120 run_timer_softirq+0x1ef/0x450 __do_softirq+0x10d/0x2b8 irq_exit+0xc7/0xd0 smp_apic_timer_interrupt+0x68/0x120 apic_timer_interrupt+0xf/0x20 This issue was also reported since 2017 in the thread [1], unfortunately, the issue was still can be reproduced after fixing DCCP. The ipv4_mib_exit_net is called before tcp_sk_exit_batch when a net namespace is destroyed since tcp_sk_ops is registered befrore ipv4_mib_ops, which means tcp_sk_ops is in the front of ipv4_mib_ops in the list of pernet_list. There will be a use-after-free on net->mib.net_statistics in tw_timer_handler after ipv4_mib_exit_net if there are some inflight time-wait timers. This bug is not introduced by commit f2bf415cfed7 ("mib: add net to NET_ADD_STATS_BH") since the net_statistics is a global variable instead of dynamic allocation and freeing. Actually, commit 61a7e26028b9 ("mib: put net statistics on struct net") introduces the bug since it put net statistics on struct net and free it when net namespace is destroyed. Moving init_ipv4_mibs() to the front of tcp_init() to fix this bug and replace pr_crit() with panic() since continuing is meaningless when init_ipv4_mibs() fails. [1] https://groups.google.com/g/syzkaller/c/p1tn-_Kc6l4/m/smuL_FMAAgAJ?pli=1 Fixes: 61a7e26028b9 ("mib: put net statistics on struct net") Signed-off-by: Muchun Song <songmuchun@bytedance.com> Cc: Xiongchun Duan <songmuchun@bytedance.com> Cc: Cong Wang <cong.wang@bytedance.com> Cc: Fam Zheng <fam.zheng@bytedance.com> Cc: <stable@vger.kernel.org> --- net/ipv4/af_inet.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)