• Shuah Khan's avatar
    usbip: fix vudc usbip_sockfd_store races leading to gpf · 46613c9d
    Shuah Khan authored
    usbip_sockfd_store() is invoked when user requests attach (import)
    detach (unimport) usb gadget device from usbip host. vhci_hcd sends
    import request and usbip_sockfd_store() exports the device if it is
    free for export.
    
    Export and unexport are governed by local state and shared state
    - Shared state (usbip device status, sockfd) - sockfd and Device
      status are used to determine if stub should be brought up or shut
      down. Device status is shared between host and client.
    - Local state (tcp_socket, rx and tx thread task_struct ptrs)
      A valid tcp_socket controls rx and tx thread operations while the
      device is in exported state.
    - While the device is exported, device status is marked used and socket,
      sockfd, and thread pointers are valid.
    
    Export sequence (stub-up) includes validating the socket and creating
    receive (rx) and transmit (tx) threads to talk to the client to provide
    access to the exported device. rx and tx threads depends on local and
    shared state to be correct and in sync.
    
    Unexport (stub-down) sequence shuts the socket down and stops the rx and
    tx threads. Stub-down sequence relies on local and shared states to be
    in sync.
    
    There are races in updating the local and shared status in the current
    stub-up sequence resulting in crashes. These stem from starting rx and
    tx threads before local and global state is updated correctly to be in
    sync.
    
    1. Doesn't handle kthread_create() error and saves invalid ptr in local
       state that drives rx and tx threads.
    2. Updates tcp_socket and sockfd,  starts stub_rx and stub_tx threads
       before updating usbip_device status to SDEV_ST_USED. This opens up a
       race condition between the threads and usbip_sockfd_store() stub up
       and down handling.
    
    Fix the above problems:
    - Stop using kthread_get_run() macro to create/start threads.
    - Create threads and get task struct reference.
    - Add kthread_create() failure handling and bail out.
    - Hold usbip_device lock to update local and shared states after
      creating rx and tx threads.
    - Update usbip_device status to SDEV_ST_USED.
    - Update usbip_device tcp_socket, sockfd, tcp_rx, and tcp_tx
    - Start threads after usbip_device (tcp_socket, sockfd, tcp_rx, tcp_tx,
      and status) is complete.
    
    Credit goes to syzbot and Tetsuo Handa for finding and root-causing the
    kthread_get_run() improper error handling problem and others. This is a
    hard problem to find and debug since the races aren't seen in a normal
    case. Fuzzing forces the race window to be small enough for the
    kthread_get_run() error path bug and starting threads before updating the
    local and shared state bug in the stub-up sequence.
    
    Fixes: 9720b4bc ("staging/usbip: convert to kthread")
    Cc: stable@vger.kernel.org
    Reported-by: default avatarsyzbot <syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com>
    Reported-by: default avatarsyzbot <syzbot+bf1a360e305ee719e364@syzkaller.appspotmail.com>
    Reported-by: default avatarsyzbot <syzbot+95ce4b142579611ef0a9@syzkaller.appspotmail.com>
    Reported-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
    Link: https://lore.kernel.org/r/b1c08b983ffa185449c9f0f7d1021dc8c8454b60.1615171203.git.skhan@linuxfoundation.orgSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    46613c9d
vudc_sysfs.c 5.77 KB