lkml.org 
[lkml]   [2020]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.6 11/41] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
    Date
    From: Jia He <justin.he@arm.com>

    [ Upstream commit 8692cefc433f282228fd44938dd4d26ed38254a2 ]

    When client on the host tries to connect(SOCK_STREAM, O_NONBLOCK) to the
    server on the guest, there will be a panic on a ThunderX2 (armv8a server):

    [ 463.718844] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
    [ 463.718848] Mem abort info:
    [ 463.718849] ESR = 0x96000044
    [ 463.718852] EC = 0x25: DABT (current EL), IL = 32 bits
    [ 463.718853] SET = 0, FnV = 0
    [ 463.718854] EA = 0, S1PTW = 0
    [ 463.718855] Data abort info:
    [ 463.718856] ISV = 0, ISS = 0x00000044
    [ 463.718857] CM = 0, WnR = 1
    [ 463.718859] user pgtable: 4k pages, 48-bit VAs, pgdp=0000008f6f6e9000
    [ 463.718861] [0000000000000000] pgd=0000000000000000
    [ 463.718866] Internal error: Oops: 96000044 [#1] SMP
    [...]
    [ 463.718977] CPU: 213 PID: 5040 Comm: vhost-5032 Tainted: G O 5.7.0-rc7+ #139
    [ 463.718980] Hardware name: GIGABYTE R281-T91-00/MT91-FS1-00, BIOS F06 09/25/2018
    [ 463.718982] pstate: 60400009 (nZCv daif +PAN -UAO)
    [ 463.718995] pc : virtio_transport_recv_pkt+0x4c8/0xd40 [vmw_vsock_virtio_transport_common]
    [ 463.718999] lr : virtio_transport_recv_pkt+0x1fc/0xd40 [vmw_vsock_virtio_transport_common]
    [ 463.719000] sp : ffff80002dbe3c40
    [...]
    [ 463.719025] Call trace:
    [ 463.719030] virtio_transport_recv_pkt+0x4c8/0xd40 [vmw_vsock_virtio_transport_common]
    [ 463.719034] vhost_vsock_handle_tx_kick+0x360/0x408 [vhost_vsock]
    [ 463.719041] vhost_worker+0x100/0x1a0 [vhost]
    [ 463.719048] kthread+0x128/0x130
    [ 463.719052] ret_from_fork+0x10/0x18

    The race condition is as follows:
    Task1 Task2
    ===== =====
    __sock_release virtio_transport_recv_pkt
    __vsock_release vsock_find_bound_socket (found sk)
    lock_sock_nested
    vsock_remove_sock
    sock_orphan
    sk_set_socket(sk, NULL)
    sk->sk_shutdown = SHUTDOWN_MASK
    ...
    release_sock
    lock_sock
    virtio_transport_recv_connecting
    sk->sk_socket->state (panic!)

    The root cause is that vsock_find_bound_socket can't hold the lock_sock,
    so there is a small race window between vsock_find_bound_socket() and
    lock_sock(). If __vsock_release() is running in another task,
    sk->sk_socket will be set to NULL inadvertently.

    This fixes it by checking sk->sk_shutdown(suggested by Stefano) after
    lock_sock since sk->sk_shutdown is set to SHUTDOWN_MASK under the
    protection of lock_sock_nested.

    Signed-off-by: Jia He <justin.he@arm.com>
    Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/vmw_vsock/virtio_transport_common.c | 8 ++++++++
    1 file changed, 8 insertions(+)

    --- a/net/vmw_vsock/virtio_transport_common.c
    +++ b/net/vmw_vsock/virtio_transport_common.c
    @@ -1128,6 +1128,14 @@ void virtio_transport_recv_pkt(struct vi

    lock_sock(sk);

    + /* Check if sk has been released before lock_sock */
    + if (sk->sk_shutdown == SHUTDOWN_MASK) {
    + (void)virtio_transport_reset_no_sock(t, pkt);
    + release_sock(sk);
    + sock_put(sk);
    + goto free_pkt;
    + }
    +
    /* Update CID in case it has changed after a transport reset event */
    vsk->local_addr.svm_cid = dst.svm_cid;


    \
     
     \ /
      Last update: 2020-06-09 20:00    [W:4.183 / U:0.248 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site