On 2021/03/08 12:53, Shuah Khan wrote: > 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. No, the whole usbip_sockfd_store() etc. should be serialized using a mutex, for two different threads can open same file and write the same content at the same moment. This results in seeing SDEV_ST_AVAILABLE and creating two threads and overwiting global variables and setting SDEV_ST_USED and starting two threads by each of two thread, which will later fail to call kthread_stop() on one of two thread because global variables are overwritten. kthread_crate() (which involves GFP_KERNEL allocation) can take long time enough to hit usbip_sockfd_store() must perform if (sdev->ud.status != SDEV_ST_AVAILABLE) { /* misc assignments for attach operation */ sdev->ud.status = SDEV_ST_USED; } under a lock, or multiple ud->tcp_{tx,rx} are created (which will later cause a crash like [1]) and refcount on ud->tcp_socket is leaked when usbip_sockfd_store() is concurrently called. problem. That's why my patch introduced usbip_event_mutex lock.