• Lorenz Bauer's avatar
    bpf, sockmap: Check update requirements after locking · 85b8ac01
    Lorenz Bauer authored
    It's currently possible to insert sockets in unexpected states into
    a sockmap, due to a TOCTTOU when updating the map from a syscall.
    sock_map_update_elem checks that sk->sk_state == TCP_ESTABLISHED,
    locks the socket and then calls sock_map_update_common. At this
    point, the socket may have transitioned into another state, and
    the earlier assumptions don't hold anymore. Crucially, it's
    conceivable (though very unlikely) that a socket has become unhashed.
    This breaks the sockmap's assumption that it will get a callback
    via sk->sk_prot->unhash.
    
    Fix this by checking the (fixed) sk_type and sk_protocol without the
    lock, followed by a locked check of sk_state.
    
    Unfortunately it's not possible to push the check down into
    sock_(map|hash)_update_common, since BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB
    run before the socket has transitioned from TCP_SYN_RECV into
    TCP_ESTABLISHED.
    
    Fixes: 604326b4 ("bpf, sockmap: convert to generic sk_msg interface")
    Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Reviewed-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
    Link: https://lore.kernel.org/bpf/20200207103713.28175-1-lmb@cloudflare.com
    85b8ac01
sock_map.c 24 KB