Fix memory leak and typo. What's more, add release_async_work() to delete asynchronous work.
Dawei Li (1): ksmbd: fix typo, syncronous->synchronous
Hangyu Hua (1): ksmbd: fix possible memory leak in smb2_lock()
Jakob Koschel (1): ksmbd: replace usage of found with dedicated list iterator variable
Namjae Jeon (1): ksmbd: delete asynchronous work from list
fs/ksmbd/connection.c | 12 ++++---- fs/ksmbd/ksmbd_work.h | 2 +- fs/ksmbd/smb2pdu.c | 66 +++++++++++++++++++++++-------------------- fs/ksmbd/smb2pdu.h | 1 + fs/ksmbd/vfs_cache.c | 5 ++-- 5 files changed, 45 insertions(+), 41 deletions(-)
From: Jakob Koschel jakobkoschel@gmail.com
mainline inclusion from mainline-v5.18-rc1 commit edf5f0548fbb77e20b898460dc25281b0f4d974d category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I6KEWO?from=project-issue
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
To move the list iterator variable into the list_for_each_entry_*() macro in the future it should be avoided to use the list iterator variable after the loop body.
To *never* use the list iterator variable after the loop it was concluded to use a separate iterator variable instead of a found boolean [1].
This removes the need to use a found variable and simply checking if the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWX... Signed-off-by: Jakob Koschel jakobkoschel@gmail.com Reviewed-by: Hyunchul Lee hyc.lee@gmail.com Acked-by: Namjae Jeon linkinjeon@kernel.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Zizhi Wo wozizhi@huawei.com --- fs/ksmbd/smb2pdu.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 29e9c94b2f7b..979cde35721d 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6655,8 +6655,7 @@ int smb2_cancel(struct ksmbd_work *work) struct ksmbd_conn *conn = work->conn; struct smb2_hdr *hdr = smb2_get_msg(work->request_buf); struct smb2_hdr *chdr; - struct ksmbd_work *cancel_work = NULL; - int canceled = 0; + struct ksmbd_work *cancel_work = NULL, *iter; struct list_head *command_list;
ksmbd_debug(SMB, "smb2 cancel called on mid %llu, async flags 0x%x\n", @@ -6666,11 +6665,11 @@ int smb2_cancel(struct ksmbd_work *work) command_list = &conn->async_requests;
spin_lock(&conn->request_lock); - list_for_each_entry(cancel_work, command_list, + list_for_each_entry(iter, command_list, async_request_entry) { - chdr = smb2_get_msg(cancel_work->request_buf); + chdr = smb2_get_msg(iter->request_buf);
- if (cancel_work->async_id != + if (iter->async_id != le64_to_cpu(hdr->Id.AsyncId)) continue;
@@ -6678,7 +6677,7 @@ int smb2_cancel(struct ksmbd_work *work) "smb2 with AsyncId %llu cancelled command = 0x%x\n", le64_to_cpu(hdr->Id.AsyncId), le16_to_cpu(chdr->Command)); - canceled = 1; + cancel_work = iter; break; } spin_unlock(&conn->request_lock); @@ -6686,24 +6685,24 @@ int smb2_cancel(struct ksmbd_work *work) command_list = &conn->requests;
spin_lock(&conn->request_lock); - list_for_each_entry(cancel_work, command_list, request_entry) { - chdr = smb2_get_msg(cancel_work->request_buf); + list_for_each_entry(iter, command_list, request_entry) { + chdr = smb2_get_msg(iter->request_buf);
if (chdr->MessageId != hdr->MessageId || - cancel_work == work) + iter == work) continue;
ksmbd_debug(SMB, "smb2 with mid %llu cancelled command = 0x%x\n", le64_to_cpu(hdr->MessageId), le16_to_cpu(chdr->Command)); - canceled = 1; + cancel_work = iter; break; } spin_unlock(&conn->request_lock); }
- if (canceled) { + if (cancel_work) { cancel_work->state = KSMBD_WORK_CANCELLED; if (cancel_work->cancel_fn) cancel_work->cancel_fn(cancel_work->cancel_argv);
From: Hangyu Hua hbh25y@gmail.com
mainline inclusion from mainline-v6.3-rc1 commit d3ca9f7aeba793d74361d88a8800b2f205c9236b category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I6KEWO?from=project-issue
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
argv needs to be free when setup_async_work fails or when the current process is woken up.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Signed-off-by: Hangyu Hua hbh25y@gmail.com Acked-by: Namjae Jeon linkinjeon@kernel.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Zizhi Wo wozizhi@huawei.com --- fs/ksmbd/smb2pdu.c | 28 +++++++++++++--------------- fs/ksmbd/vfs_cache.c | 5 ++--- 2 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 979cde35721d..a89e6644afa9 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6655,7 +6655,7 @@ int smb2_cancel(struct ksmbd_work *work) struct ksmbd_conn *conn = work->conn; struct smb2_hdr *hdr = smb2_get_msg(work->request_buf); struct smb2_hdr *chdr; - struct ksmbd_work *cancel_work = NULL, *iter; + struct ksmbd_work *iter; struct list_head *command_list;
ksmbd_debug(SMB, "smb2 cancel called on mid %llu, async flags 0x%x\n", @@ -6677,7 +6677,9 @@ int smb2_cancel(struct ksmbd_work *work) "smb2 with AsyncId %llu cancelled command = 0x%x\n", le64_to_cpu(hdr->Id.AsyncId), le16_to_cpu(chdr->Command)); - cancel_work = iter; + iter->state = KSMBD_WORK_CANCELLED; + if (iter->cancel_fn) + iter->cancel_fn(iter->cancel_argv); break; } spin_unlock(&conn->request_lock); @@ -6696,18 +6698,12 @@ int smb2_cancel(struct ksmbd_work *work) "smb2 with mid %llu cancelled command = 0x%x\n", le64_to_cpu(hdr->MessageId), le16_to_cpu(chdr->Command)); - cancel_work = iter; + iter->state = KSMBD_WORK_CANCELLED; break; } spin_unlock(&conn->request_lock); }
- if (cancel_work) { - cancel_work->state = KSMBD_WORK_CANCELLED; - if (cancel_work->cancel_fn) - cancel_work->cancel_fn(cancel_work->cancel_argv); - } - /* For SMB2_CANCEL command itself send no response*/ work->send_no_response = 1; return 0; @@ -7071,6 +7067,14 @@ int smb2_lock(struct ksmbd_work *work)
ksmbd_vfs_posix_lock_wait(flock);
+ spin_lock(&work->conn->request_lock); + spin_lock(&fp->f_lock); + list_del(&work->fp_entry); + work->cancel_fn = NULL; + kfree(argv); + spin_unlock(&fp->f_lock); + spin_unlock(&work->conn->request_lock); + if (work->state != KSMBD_WORK_ACTIVE) { list_del(&smb_lock->llist); spin_lock(&work->conn->llist_lock); @@ -7079,9 +7083,6 @@ int smb2_lock(struct ksmbd_work *work) locks_free_lock(flock);
if (work->state == KSMBD_WORK_CANCELLED) { - spin_lock(&fp->f_lock); - list_del(&work->fp_entry); - spin_unlock(&fp->f_lock); rsp->hdr.Status = STATUS_CANCELLED; kfree(smb_lock); @@ -7103,9 +7104,6 @@ int smb2_lock(struct ksmbd_work *work) list_del(&smb_lock->clist); spin_unlock(&work->conn->llist_lock);
- spin_lock(&fp->f_lock); - list_del(&work->fp_entry); - spin_unlock(&fp->f_lock); goto retry; } else if (!rc) { spin_lock(&work->conn->llist_lock); diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c index 26db346c5c4f..2a445befcc62 100644 --- a/fs/ksmbd/vfs_cache.c +++ b/fs/ksmbd/vfs_cache.c @@ -364,12 +364,11 @@ static void __put_fd_final(struct ksmbd_work *work, struct ksmbd_file *fp)
static void set_close_state_blocked_works(struct ksmbd_file *fp) { - struct ksmbd_work *cancel_work, *ctmp; + struct ksmbd_work *cancel_work;
spin_lock(&fp->f_lock); - list_for_each_entry_safe(cancel_work, ctmp, &fp->blocked_works, + list_for_each_entry(cancel_work, &fp->blocked_works, fp_entry) { - list_del(&cancel_work->fp_entry); cancel_work->state = KSMBD_WORK_CLOSED; cancel_work->cancel_fn(cancel_work->cancel_argv); }
From: Dawei Li set_pte_at@outlook.com
mainline inclusion from mainline-v6.3-rc1 commit f8d6e7442aa716a233c7eba99dec628f8885e00b category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I6KEWO?from=project-issue
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
syncronous->synchronous
Signed-off-by: Dawei Li set_pte_at@outlook.com Acked-by: Namjae Jeon linkinjeon@kernel.org Reviewed-by: Sergey Senozhatsky senozhatsky@chromium.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Zizhi Wo wozizhi@huawei.com --- fs/ksmbd/connection.c | 4 ++-- fs/ksmbd/ksmbd_work.h | 2 +- fs/ksmbd/smb2pdu.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index 96d6653dabc1..61b74eaa6588 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -108,7 +108,7 @@ void ksmbd_conn_enqueue_request(struct ksmbd_work *work)
if (conn->ops->get_cmd_val(work) != SMB2_CANCEL_HE) { requests_queue = &conn->requests; - work->syncronous = true; + work->synchronous = true; }
if (requests_queue) { @@ -133,7 +133,7 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work) spin_lock(&conn->request_lock); if (!work->multiRsp) { list_del_init(&work->request_entry); - if (work->syncronous == false) + if (!work->synchronous) list_del_init(&work->async_request_entry); ret = 0; } diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h index 5ece58e40c97..3234f2cf6327 100644 --- a/fs/ksmbd/ksmbd_work.h +++ b/fs/ksmbd/ksmbd_work.h @@ -68,7 +68,7 @@ struct ksmbd_work { /* Request is encrypted */ bool encrypted:1; /* Is this SYNC or ASYNC ksmbd_work */ - bool syncronous:1; + bool synchronous:1; bool need_invalidate_rkey:1;
unsigned int remote_key; diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index a89e6644afa9..e050adcce05f 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -519,7 +519,7 @@ int init_smb2_rsp_hdr(struct ksmbd_work *work) rsp_hdr->SessionId = rcv_hdr->SessionId; memcpy(rsp_hdr->Signature, rcv_hdr->Signature, 16);
- work->syncronous = true; + work->synchronous = true; if (work->async_id) { ksmbd_release_id(&conn->async_ida, work->async_id); work->async_id = 0; @@ -682,7 +682,7 @@ int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg) pr_err("Failed to alloc async message id\n"); return id; } - work->syncronous = false; + work->synchronous = false; work->async_id = id; rsp_hdr->Id.AsyncId = cpu_to_le64(id);
From: Namjae Jeon linkinjeon@kernel.org
mainline inclusion from mainline-v6.3-rc6 commit 3a9b557f44ea8f216aab515a7db20e23f0eb51b9 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I6KEWO?from=project-issue
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
When smb2_lock request is canceled by smb2_cancel or smb2_close(), ksmbd is missing deleting async_request_entry async_requests list. Because calling init_smb2_rsp_hdr() in smb2_lock() mark ->synchronous as true and then it will not be deleted in ksmbd_conn_try_dequeue_request(). This patch add release_async_work() to release the ones allocated for async work.
Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon linkinjeon@kernel.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Zizhi Wo wozizhi@huawei.com --- fs/ksmbd/connection.c | 12 +++++------- fs/ksmbd/ksmbd_work.h | 2 +- fs/ksmbd/smb2pdu.c | 33 +++++++++++++++++++++------------ fs/ksmbd/smb2pdu.h | 1 + 4 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index 61b74eaa6588..f0818e764230 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -106,10 +106,8 @@ void ksmbd_conn_enqueue_request(struct ksmbd_work *work) struct ksmbd_conn *conn = work->conn; struct list_head *requests_queue = NULL;
- if (conn->ops->get_cmd_val(work) != SMB2_CANCEL_HE) { + if (conn->ops->get_cmd_val(work) != SMB2_CANCEL_HE) requests_queue = &conn->requests; - work->synchronous = true; - }
if (requests_queue) { atomic_inc(&conn->req_running); @@ -130,14 +128,14 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work)
if (!work->multiRsp) atomic_dec(&conn->req_running); - spin_lock(&conn->request_lock); if (!work->multiRsp) { + spin_lock(&conn->request_lock); list_del_init(&work->request_entry); - if (!work->synchronous) - list_del_init(&work->async_request_entry); + spin_unlock(&conn->request_lock); + if (work->asynchronous) + release_async_work(work); ret = 0; } - spin_unlock(&conn->request_lock);
wake_up_all(&conn->req_running_q); return ret; diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h index 3234f2cf6327..f8ae6144c0ae 100644 --- a/fs/ksmbd/ksmbd_work.h +++ b/fs/ksmbd/ksmbd_work.h @@ -68,7 +68,7 @@ struct ksmbd_work { /* Request is encrypted */ bool encrypted:1; /* Is this SYNC or ASYNC ksmbd_work */ - bool synchronous:1; + bool asynchronous:1; bool need_invalidate_rkey:1;
unsigned int remote_key; diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index e050adcce05f..b21ac851345f 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -519,12 +519,6 @@ int init_smb2_rsp_hdr(struct ksmbd_work *work) rsp_hdr->SessionId = rcv_hdr->SessionId; memcpy(rsp_hdr->Signature, rcv_hdr->Signature, 16);
- work->synchronous = true; - if (work->async_id) { - ksmbd_release_id(&conn->async_ida, work->async_id); - work->async_id = 0; - } - return 0; }
@@ -682,7 +676,7 @@ int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg) pr_err("Failed to alloc async message id\n"); return id; } - work->synchronous = false; + work->asynchronous = true; work->async_id = id; rsp_hdr->Id.AsyncId = cpu_to_le64(id);
@@ -702,6 +696,24 @@ int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg) return 0; }
+void release_async_work(struct ksmbd_work *work) +{ + struct ksmbd_conn *conn = work->conn; + + spin_lock(&conn->request_lock); + list_del_init(&work->async_request_entry); + spin_unlock(&conn->request_lock); + + work->asynchronous = 0; + work->cancel_fn = NULL; + kfree(work->cancel_argv); + work->cancel_argv = NULL; + if (work->async_id) { + ksmbd_release_id(&conn->async_ida, work->async_id); + work->async_id = 0; + } +} + void smb2_send_interim_resp(struct ksmbd_work *work, __le32 status) { struct smb2_hdr *rsp_hdr; @@ -7067,13 +7079,9 @@ int smb2_lock(struct ksmbd_work *work)
ksmbd_vfs_posix_lock_wait(flock);
- spin_lock(&work->conn->request_lock); spin_lock(&fp->f_lock); list_del(&work->fp_entry); - work->cancel_fn = NULL; - kfree(argv); spin_unlock(&fp->f_lock); - spin_unlock(&work->conn->request_lock);
if (work->state != KSMBD_WORK_ACTIVE) { list_del(&smb_lock->llist); @@ -7091,6 +7099,7 @@ int smb2_lock(struct ksmbd_work *work) work->send_no_response = 1; goto out; } + init_smb2_rsp_hdr(work); smb2_set_err_rsp(work); rsp->hdr.Status = @@ -7103,7 +7112,7 @@ int smb2_lock(struct ksmbd_work *work) 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) { spin_lock(&work->conn->llist_lock); diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h index ba255fd2f77c..089eb93a166f 100644 --- a/fs/ksmbd/smb2pdu.h +++ b/fs/ksmbd/smb2pdu.h @@ -1664,6 +1664,7 @@ int find_matching_smb2_dialect(int start_index, __le16 *cli_dialects, struct file_lock *smb_flock_init(struct file *f); int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg); +void release_async_work(struct ksmbd_work *work); void smb2_send_interim_resp(struct ksmbd_work *work, __le32 status); struct channel *lookup_chann_list(struct ksmbd_session *sess, struct ksmbd_conn *conn);
-
Zizhi Wo