Commit 75ac9a3d authored by Namjae Jeon's avatar Namjae Jeon Committed by Steve French
Browse files

ksmbd: fix race condition from parallel smb2 lock requests 



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: default avatarluosili <rootlab@huawei.com>
Signed-off-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 7ca9da7d

fs/smb/server/smb2pdu.c

+1 −11
Original line number Diff line number Diff line
@@ -7038,10 +7038,6 @@ int smb2_lock(struct ksmbd_work *work) 


				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);
@@ -7072,9 +7068,6 @@ int smb2_lock(struct ksmbd_work *work) 


				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) {
@@ -7094,19 +7087,16 @@ int smb2_lock(struct ksmbd_work *work) 
				}



				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;