smb: client: fix races in cifsd thread creation

The cifsd demultiplex thread can run and access tcp_ses before the parent
thread has finished populating tcp_ses, which the worker thread accesses
locklessly.

Also, the kthread_run macro may start the thread before returning the
thread pointer. Because the pointer is part of the structure that the
thread can access, if the kernel is preempted after the thread is spawned,
but before the thread pointer is populated and the thread attempts to exit,
it will sleep, waiting for a SIGKILL signal.

Fix this by moving creation of the thread to after all of tcp_ses'es
fields are populated, and spawning the thread last, using a split
kthread_create/wake_up_process logic.

Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index cbeb5637eeb9251fc24f65cda066c4e99ac2b43b..104658e318b6547094197d6acb8b98632dd6cb2f 100644 (file)
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -1871,14 +1871,6 @@ smbd_connected:
         * this will succeed. No need for try_module_get().
         */
        __module_get(THIS_MODULE);
-       tcp_ses->tsk = kthread_run(cifs_demultiplex_thread,
-                                 tcp_ses, "cifsd");
-       if (IS_ERR(tcp_ses->tsk)) {
-               rc = PTR_ERR(tcp_ses->tsk);
-               cifs_dbg(VFS, "error %d create cifsd thread\n", rc);
-               module_put(THIS_MODULE);
-               goto out_err_crypto_release;
-       }
        tcp_ses->min_offload = ctx->min_offload;
        tcp_ses->retrans = ctx->retrans;
        /*
@@ -1886,9 +1878,7 @@ smbd_connected:
         * to the struct since the kernel thread not created yet
         * no need to spinlock this update of tcpStatus
         */
-       spin_lock(&tcp_ses->srv_lock);
        tcp_ses->tcpStatus = CifsNeedNegotiate;
-       spin_unlock(&tcp_ses->srv_lock);
 
        if ((ctx->max_credits < 20) || (ctx->max_credits > 60000))
                tcp_ses->max_credits = SMB2_MAX_CREDITS_AVAILABLE;
@@ -1897,7 +1887,16 @@ smbd_connected:
 
        tcp_ses->nr_targets = 1;
        tcp_ses->ignore_signature = ctx->ignore_signature;
-       /* thread spawned, put it on the list */
+
+       tcp_ses->tsk = kthread_create(cifs_demultiplex_thread,
+                                 tcp_ses, "cifsd");
+       if (IS_ERR(tcp_ses->tsk)) {
+               rc = PTR_ERR(tcp_ses->tsk);
+               cifs_dbg(VFS, "error %d create cifsd thread\n", rc);
+               module_put(THIS_MODULE);
+               goto out_err_crypto_release;
+       }
+       /* thread created, put it on the list */
        spin_lock(&cifs_tcp_ses_lock);
        list_add(&tcp_ses->tcp_ses_list, &cifs_tcp_ses_list);
        spin_unlock(&cifs_tcp_ses_lock);
@@ -1905,6 +1904,12 @@ smbd_connected:
        /* queue echo request delayed work */
        queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
 
+       /*
+        * Use split create/wake logic to ensure that tcp_ses is fully populated
+        * and tcp_ses->tsk is valid
+        */
+       wake_up_process(tcp_ses->tsk);
+
        return tcp_ses;
 
 out_err_crypto_release: