ksmbd: fix race condition from parallel smb2 lock requests

[ Upstream commit 75ac9a3dd65f7eab4d12b0a0f744234b5300a491 ]

There is a race condition issue between parallel smb2 lock request.

                                            Time
                                             +
Thread A                                     | Thread A
smb2_lock                                    | smb2_lock
                                             |
 insert smb_lock to lock_list                |
 spin_unlock(&work->conn->llist_lock)        |
                                             |
                                             |   spin_lock(&conn->llist_lock);
                                             |   kfree(cmp_lock);
                                             |
 // UAF!                                     |
 list_add(&smb_lock->llist, &rollback_list)  +

This patch swaps the line for adding the smb lock to the rollback list and
adding the lock list of connection to fix the race issue.

Reported-by: luosili <rootlab@huawei.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 2af9f01f385686afd172f8f75784dfdc1dc9f6ea..b2e0eb9cb7c43ec70cadaa89bf97254deedc9db4 100644 (file)
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -7040,10 +7040,6 @@ skip:
 
                                ksmbd_debug(SMB,
                                            "would have to wait for getting lock\n");
-                               spin_lock(&work->conn->llist_lock);
-                               list_add_tail(&smb_lock->clist,
-                                             &work->conn->lock_list);
-                               spin_unlock(&work->conn->llist_lock);
                                list_add(&smb_lock->llist, &rollback_list);
 
                                argv = kmalloc(sizeof(void *), GFP_KERNEL);
@@ -7075,9 +7071,6 @@ skip:
 
                                if (work->state != KSMBD_WORK_ACTIVE) {
                                        list_del(&smb_lock->llist);
-                                       spin_lock(&work->conn->llist_lock);
-                                       list_del(&smb_lock->clist);
-                                       spin_unlock(&work->conn->llist_lock);
                                        locks_free_lock(flock);
 
                                        if (work->state == KSMBD_WORK_CANCELLED) {
@@ -7097,19 +7090,16 @@ skip:
                                }
 
                                list_del(&smb_lock->llist);
-                               spin_lock(&work->conn->llist_lock);
-                               list_del(&smb_lock->clist);
-                               spin_unlock(&work->conn->llist_lock);
                                release_async_work(work);
                                goto retry;
                        } else if (!rc) {
+                               list_add(&smb_lock->llist, &rollback_list);
                                spin_lock(&work->conn->llist_lock);
                                list_add_tail(&smb_lock->clist,
                                              &work->conn->lock_list);
                                list_add_tail(&smb_lock->flist,
                                              &fp->lock_list);
                                spin_unlock(&work->conn->llist_lock);
-                               list_add(&smb_lock->llist, &rollback_list);
                                ksmbd_debug(SMB, "successful in taking lock\n");
                        } else {
                                goto out;