[ciqlts9_2] CVE-2025-38527 #736
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r the same file. jira VULN-131073 cve-pre CVE-2025-38527 commit-author Bharath SM <bharathsm@microsoft.com> commit 9e31678 Solution is to send lease break ack immediately even in case of deferred close handles to avoid lease break request timing out and let deferred closed handle gets closed as scheduled. Later patches could optimize cases where we then close some of these handles sooner for the cases where lease break is to 'none' Cc: stable@kernel.org Signed-off-by: Bharath SM <bharathsm@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit 9e31678) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jira VULN-131073 cve-pre CVE-2025-38527 commit-author Bharath SM <bharathsm@microsoft.com> commit d906be3 We should not cache deferred file handles if we dont have handle lease on a file. And we should immediately close all deferred handles in case of handle lease break. Fixes: 9e31678 ("SMB3: fix lease break timeout when multiple deferred close handles for the same file.") Signed-off-by: Bharath SM <bharathsm@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit d906be3) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jira VULN-131073 cve-pre CVE-2025-38527 commit-author Bharath SM <bharathsm@microsoft.com> commit 47592fa Oplock break may occur for different file handle than the deferred handle. Check for inode deferred closes list, if it's not empty then close all the deferred handles of inode because we should not cache handles if we dont have handle lease. Eg: If openfilelist has one deferred file handle and another open file handle from app for a same file, then on a lease break we choose the first handle in openfile list. The first handle in list can be deferred handle or actual open file handle from app. In case if it is actual open handle then today, we don't close deferred handles if we lose handle lease on a file. Problem with this is, later if app decides to close the existing open handle then we still be caching deferred handles until deferred close timeout. Leaving open handle may result in sharing violation when windows client tries to open a file with limited file share access. So we should check for deferred list of inode and walk through the list of deferred files in inode and close all deferred files. Fixes: 9e31678 ("SMB3: fix lease break timeout when multiple deferred close handles for the same file.") Cc: stable@kernel.org Signed-off-by: Bharath SM <bharathsm@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit 47592fa) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jira VULN-131073 cve-pre CVE-2025-38527 commit-author Bharath SM <bharathsm@microsoft.com> commit 59a556a In cifs_oplock_break function we drop reference to a cfile at the end of function, due to which close command goes on wire after lease break acknowledgment even if file is already closed by application but we had deferred the handle close. If other client with limited file shareaccess waiting on lease break ack proceeds operation on that file as soon as first client sends ack, then we may encounter status sharing violation error because of open handle. Solution is to put reference to cfile(send close on wire if last ref) and then send oplock acknowledgment to server. Fixes: 9e31678 ("SMB3: fix lease break timeout when multiple deferred close handles for the same file.") Cc: stable@kernel.org Signed-off-by: Bharath SM <bharathsm@microsoft.com> Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit 59a556a) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jira VULN-131073 cve-pre CVE-2025-38527 commit-author Steve French <stfrench@microsoft.com> commit c774e67 upstream-diff Applied to fs/cifs/file.c since it hasn't been moved to fs/smb/client/file.c in this kernel umount can race with lease break so need to check if tcon->ses->server is still valid to send the lease break response. Reviewed-by: Bharath SM <bharathsm@microsoft.com> Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Fixes: 59a556a ("SMB3: drop reference to cfile before sending oplock break") Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit c774e67) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
… been closed jira VULN-131073 cve-pre CVE-2025-38527 commit-author Bharath SM <bharathsm@microsoft.com> commit da787d5 upstream-diff Applied to fs/cifs/file.c since it hasn't been moved to fs/smb/client/file.c in this kernel In case if all existing file handles are deferred handles and if all of them gets closed due to handle lease break then we dont need to send lease break acknowledgment to server, because last handle close will be considered as lease break ack. After closing deferred handels, we check for openfile list of inode, if its empty then we skip sending lease break ack. Fixes: 59a556a ("SMB3: drop reference to cfile before sending oplock break") Reviewed-by: Tom Talpey <tom@talpey.com> Signed-off-by: Bharath SM <bharathsm@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit da787d5) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jira VULN-131073 cve-pre CVE-2025-38527 commit-author Steve French <stfrench@microsoft.com> commit e8f5f84 upstream-diff Applied to fs/cifs/file.c since it hasn't been moved to fs/smb/client/file.c in this kernel With deferred close we can have closes that race with lease breaks, and so with the current checks for whether to send the lease response, oplock_response(), this can mean that an unmount (kill_sb) can occur just before we were checking if the tcon->ses is valid. See below: [Fri Aug 4 04:12:50 2023] RIP: 0010:cifs_oplock_break+0x1f7/0x5b0 [cifs] [Fri Aug 4 04:12:50 2023] Code: 7d a8 48 8b 7d c0 c0 e9 02 48 89 45 b8 41 89 cf e8 3e f5 ff ff 4c 89 f7 41 83 e7 01 e8 82 b3 03 f2 49 8b 45 50 48 85 c0 74 5e <48> 83 78 60 00 74 57 45 84 ff 75 52 48 8b 43 98 48 83 eb 68 48 39 [Fri Aug 4 04:12:50 2023] RSP: 0018:ffffb30607ddbdf8 EFLAGS: 00010206 [Fri Aug 4 04:12:50 2023] RAX: 632d223d32612022 RBX: ffff97136944b1e0 RCX: 0000000080100009 [Fri Aug 4 04:12:50 2023] RDX: 0000000000000001 RSI: 0000000080100009 RDI: ffff97136944b188 [Fri Aug 4 04:12:50 2023] RBP: ffffb30607ddbe58 R08: 0000000000000001 R09: ffffffffc08e0900 [Fri Aug 4 04:12:50 2023] R10: 0000000000000001 R11: 000000000000000f R12: ffff97136944b138 [Fri Aug 4 04:12:50 2023] R13: ffff97149147c000 R14: ffff97136944b188 R15: 0000000000000000 [Fri Aug 4 04:12:50 2023] FS: 0000000000000000(0000) GS:ffff9714f7c00000(0000) knlGS:0000000000000000 [Fri Aug 4 04:12:50 2023] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [Fri Aug 4 04:12:50 2023] CR2: 00007fd8de9c7590 CR3: 000000011228e000 CR4: 0000000000350ef0 [Fri Aug 4 04:12:50 2023] Call Trace: [Fri Aug 4 04:12:50 2023] <TASK> [Fri Aug 4 04:12:50 2023] process_one_work+0x225/0x3d0 [Fri Aug 4 04:12:50 2023] worker_thread+0x4d/0x3e0 [Fri Aug 4 04:12:50 2023] ? process_one_work+0x3d0/0x3d0 [Fri Aug 4 04:12:50 2023] kthread+0x12a/0x150 [Fri Aug 4 04:12:50 2023] ? set_kthread_struct+0x50/0x50 [Fri Aug 4 04:12:50 2023] ret_from_fork+0x22/0x30 [Fri Aug 4 04:12:50 2023] </TASK> To fix this change the ordering of the checks before sending the oplock_response to first check if the openFileList is empty. Fixes: da787d5 ("SMB3: Do not send lease break acknowledgment if all file handles have been closed") Suggested-by: Bharath SM <bharathsm@microsoft.com> Reviewed-by: Bharath SM <bharathsm@microsoft.com> Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit e8f5f84) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
jira VULN-131073 cve CVE-2025-38527 commit-author Wang Zhaolong <wangzhaolong@huaweicloud.com> commit 705c791 A race condition can occur in cifs_oplock_break() leading to a use-after-free of the cinode structure when unmounting: cifs_oplock_break() _cifsFileInfo_put(cfile) cifsFileInfo_put_final() cifs_sb_deactive() [last ref, start releasing sb] kill_sb() kill_anon_super() generic_shutdown_super() evict_inodes() dispose_list() evict() destroy_inode() call_rcu(&inode->i_rcu, i_callback) spin_lock(&cinode->open_file_lock) <- OK [later] i_callback() cifs_free_inode() kmem_cache_free(cinode) spin_unlock(&cinode->open_file_lock) <- UAF cifs_done_oplock_break(cinode) <- UAF The issue occurs when umount has already released its reference to the superblock. When _cifsFileInfo_put() calls cifs_sb_deactive(), this releases the last reference, triggering the immediate cleanup of all inodes under RCU. However, cifs_oplock_break() continues to access the cinode after this point, resulting in use-after-free. Fix this by holding an extra reference to the superblock during the entire oplock break operation. This ensures that the superblock and its inodes remain valid until the oplock break completes. Link: https://bugzilla.kernel.org/show_bug.cgi?id=220309 Fixes: b98749c ("CIFS: keep FileInfo handle live during oplock break") Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Signed-off-by: Steve French <stfrench@microsoft.com> (cherry picked from commit 705c791) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
🔍 Interdiff Analysis
diff -u b/fs/cifs/file.c b/fs/smb/client/file.c
--- b/fs/cifs/file.c
+++ b/fs/smb/client/file.c
@@ -5140,4 +5140,4 @@
cifs_sb_deactive(sb);
}
-static int cifs_swap_activate(struct swap_info_struct *sis,
+/*This is an automated interdiff check for backported commits. |
Collaborator
Author
|
I am not sure why all the PRs are created from my name. Need to really fix this as I am unable to review PRs created in my name. |
bmastbergen
changed the title
Collaborator
Author
|
Hmmm kselftest comparison skipped for this :( For now, this PR looks good to me as Roxana's PR had 135 kselftest passed and this one has 138. |
Collaborator
I guess that means I can review my own then 😜 |
Collaborator
Author
hahaha yes 🤣 |
roxanan1996
approved these changes
Dec 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
The original intent was to backport 705c791 to address CVE-2025-38527. There ended up being 7 prerequisite patches to make that apply cleanly. That is kind of a lot, but they all seem like good fixes to have. When I looked at those seven changes it seemed to me that they were pretty well focused on the lease break functionality. They didn't touch a lot of other functions/functionality. So it seemed like low risk to other areas of functionality to get improved overall lease break functionality even above the CVE fix. They were all clean, EXCEPT for the fact that the last 4 commits were applied to fs/smb/client/file.c in the upstream kernel, but in this kernel that file is at its original location fs/cifs/file.c. I am pleasantly surprised by how well interdiff seems to have dealt with that.
Also, as you can see below, this PR was auto- built, tested, and generated. It didn't find earlier test results to compare against, but I think it should be compared against the results in this PR: #728 There were more passed tests here than there so I guess thats good.
Summary
This PR has been automatically created after successful completion of all CI stages.
Commit Message(s)
Test Results
✅ Build Stage
✅ Boot Verification
✅ Kernel Selftests
🤖 This PR was automatically generated by GitHub Actions
Run ID: 19830536990