• Guillaume Nault's avatar
    l2tp: fix races in tunnel creation · 6b9f3423
    Guillaume Nault authored
    l2tp_tunnel_create() inserts the new tunnel into the namespace's tunnel
    list and sets the socket's ->sk_user_data field, before returning it to
    the caller. Therefore, there are two ways the tunnel can be accessed
    and freed, before the caller even had the opportunity to take a
    reference. In practice, syzbot could crash the module by closing the
    socket right after a new tunnel was returned to pppol2tp_create().
    
    This patch moves tunnel registration out of l2tp_tunnel_create(), so
    that the caller can safely hold a reference before publishing the
    tunnel. This second step is done with the new l2tp_tunnel_register()
    function, which is now responsible for associating the tunnel to its
    socket and for inserting it into the namespace's list.
    
    While moving the code to l2tp_tunnel_register(), a few modifications
    have been done. First, the socket validation tests are done in a helper
    function, for clarity. Also, modifying the socket is now done after
    having inserted the tunnel to the namespace's tunnels list. This will
    allow insertion to fail, without having to revert theses modifications
    in the error path (a followup patch will check for duplicate tunnels
    before insertion). Either the socket is a kernel socket which we
    control, or it is a user-space socket for which we have a reference on
    the file descriptor. In any case, the socket isn't going to be closed
    from under us.
    
    Reported-by: syzbot+fbeeb5c3b538e8545644@syzkaller.appspotmail.com
    Fixes: fd558d18 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
    Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    6b9f3423
l2tp_ppp.c 46.6 KB