lkml.org 
[lkml]   [2022]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 69/73] net: fix use-after-free in tw_timer_handler
    Date
    From: Muchun Song <songmuchun@bytedance.com>

    commit e22e45fc9e41bf9fcc1e92cfb78eb92786728ef0 upstream.

    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: Cong Wang <cong.wang@bytedance.com>
    Cc: Fam Zheng <fam.zheng@bytedance.com>
    Cc: <stable@vger.kernel.org>
    Link: https://lore.kernel.org/r/20211228104145.9426-1-songmuchun@bytedance.com
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/ipv4/af_inet.c | 10 ++++------
    1 file changed, 4 insertions(+), 6 deletions(-)

    --- a/net/ipv4/af_inet.c
    +++ b/net/ipv4/af_inet.c
    @@ -2004,6 +2004,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();

    @@ -2034,12 +2038,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();


    \
     
     \ /
      Last update: 2022-01-03 15:36    [W:4.895 / U:0.824 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site