ksmbd: fix race condition from parallel smb2 lock requests

commit 75ac9a3dd65f7eab4d12b0a0f744234b5300a491 upstream.

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/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 99a9c024e2dc99ec8fd076b524ccb7fddc2bde39..144a4e23efcd82bb7182ecf01057e499e2055c85 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -7029,10 +7029,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);
@@ -7063,9 +7059,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) {
@@ -7087,19 +7080,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;