Archive-only list for patches
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: NeilBrown <neil@brown.name>, Namjae Jeon <linkinjeon@kernel.org>,
	Steve French <stfrench@microsoft.com>,
	Sasha Levin <sashal@kernel.org>
Subject: [PATCH AUTOSEL 5.15 06/15] smb/server: avoid deadlock when linking with ReplaceIfExists
Date: Sun,  3 Aug 2025 17:21:56 -0400	[thread overview]
Message-ID: <20250803212206.3548990-6-sashal@kernel.org> (raw)
In-Reply-To: <20250803212206.3548990-1-sashal@kernel.org>

From: NeilBrown <neil@brown.name>

[ Upstream commit d5fc1400a34b4ea5e8f2ce296ea12bf8c8421694 ]

If smb2_create_link() is called with ReplaceIfExists set and the name
does exist then a deadlock will happen.

ksmbd_vfs_kern_path_locked() will return with success and the parent
directory will be locked.  ksmbd_vfs_remove_file() will then remove the
file.  ksmbd_vfs_link() will then be called while the parent is still
locked.  It will try to lock the same parent and will deadlock.

This patch moves the ksmbd_vfs_kern_path_unlock() call to *before*
ksmbd_vfs_link() and then simplifies the code, removing the file_present
flag variable.

Signed-off-by: NeilBrown <neil@brown.name>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

**Backport Status: YES**

This commit should be backported to stable kernel trees. Here's my
detailed analysis:

## Bug Analysis

This commit fixes a critical **deadlock** issue in the ksmbd (SMB
server) subsystem. The deadlock occurs in the `smb2_create_link()`
function when creating a hard link with the `ReplaceIfExists` flag set
and the target file already exists.

### Deadlock Sequence:

1. `ksmbd_vfs_kern_path_locked()` is called and locks the parent
   directory
2. If the file exists and `ReplaceIfExists` is true,
   `ksmbd_vfs_remove_file()` is called while the parent is still locked
3. `ksmbd_vfs_link()` is then called, which attempts to lock the same
   parent directory again
4. **Result: Deadlock** - the function tries to acquire a lock it
   already holds

### Fix Implementation:

The fix reorders the operations by:
- Moving the `ksmbd_vfs_kern_path_unlock()` call to execute **before**
  `ksmbd_vfs_link()`
- This ensures the parent directory is unlocked before attempting the
  link operation
- The code is also simplified by removing the `file_present` flag
  variable

## Backport Justification:

1. **Critical Bug**: This fixes a deadlock that can hang the kernel's
   SMB server functionality
2. **User Impact**: The deadlock can be triggered by normal SMB
   operations (creating hard links with replace), affecting file server
   availability
3. **Small and Contained**: The fix is minimal - just reordering
   function calls and simplifying logic
4. **No Architectural Changes**: This is a straightforward bug fix with
   no feature additions
5. **Low Risk**: The change only affects the specific error path in link
   creation
6. **Similar to Other Fixes**: The kernel has accepted similar deadlock
   fixes in ksmbd (e.g., commit 864fb5d37163 "ksmbd: fix possible
   deadlock in smb2_open")

## Code Quality:

The fix properly maintains the locking semantics while avoiding the
deadlock. The simplified code flow makes the logic clearer and less
error-prone.

This is exactly the type of bug fix that stable kernels should receive -
it fixes a real issue that affects users, has minimal risk of
regression, and improves system stability.

 fs/ksmbd/smb2pdu.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 76334a983cd2..3439dbad9389 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -5612,7 +5612,6 @@ static int smb2_create_link(struct ksmbd_work *work,
 {
 	char *link_name = NULL, *target_name = NULL, *pathname = NULL;
 	struct path path, parent_path;
-	bool file_present = false;
 	int rc;
 
 	if (buf_len < (u64)sizeof(struct smb2_file_link_info) +
@@ -5645,11 +5644,8 @@ static int smb2_create_link(struct ksmbd_work *work,
 	if (rc) {
 		if (rc != -ENOENT)
 			goto out;
-	} else
-		file_present = true;
-
-	if (file_info->ReplaceIfExists) {
-		if (file_present) {
+	} else {
+		if (file_info->ReplaceIfExists) {
 			rc = ksmbd_vfs_remove_file(work, &path);
 			if (rc) {
 				rc = -EINVAL;
@@ -5657,21 +5653,17 @@ static int smb2_create_link(struct ksmbd_work *work,
 					    link_name);
 				goto out;
 			}
-		}
-	} else {
-		if (file_present) {
+		} else {
 			rc = -EEXIST;
 			ksmbd_debug(SMB, "link already exists\n");
 			goto out;
 		}
+		ksmbd_vfs_kern_path_unlock(&parent_path, &path);
 	}
-
 	rc = ksmbd_vfs_link(work, target_name, link_name);
 	if (rc)
 		rc = -EINVAL;
 out:
-	if (file_present)
-		ksmbd_vfs_kern_path_unlock(&parent_path, &path);
 
 	if (!IS_ERR(link_name))
 		kfree(link_name);
-- 
2.39.5


  parent reply	other threads:[~2025-08-03 21:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-03 21:21 [PATCH AUTOSEL 5.15 01/15] hfs: fix slab-out-of-bounds in hfs_bnode_read() Sasha Levin
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 02/15] hfsplus: fix slab-out-of-bounds in hfsplus_bnode_read() Sasha Levin
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 03/15] hfsplus: fix slab-out-of-bounds read in hfsplus_uni2asc() Sasha Levin
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 04/15] hfsplus: don't use BUG_ON() in hfsplus_create_attributes_file() Sasha Levin
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 05/15] arm64: Handle KCOV __init vs inline mismatches Sasha Levin
2025-08-03 21:21 ` Sasha Levin [this message]
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 07/15] udf: Verify partition map count Sasha Levin
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 08/15] drbd: add missing kref_get in handle_write_conflicts Sasha Levin
2025-08-03 21:21 ` [PATCH AUTOSEL 5.15 09/15] hfs: fix not erasing deleted b-tree node issue Sasha Levin
2025-08-03 21:22 ` [PATCH AUTOSEL 5.15 10/15] better lockdep annotations for simple_recursive_removal() Sasha Levin
2025-08-03 21:22 ` [PATCH AUTOSEL 5.15 11/15] ata: libata-sata: Disallow changing LPM state if not supported Sasha Levin
2025-08-03 21:22 ` [PATCH AUTOSEL 5.15 12/15] fs/ntfs3: Add sanity check for file name Sasha Levin
2025-08-03 21:22 ` [PATCH AUTOSEL 5.15 13/15] fs/ntfs3: correctly create symlink for relative path Sasha Levin
2025-08-03 21:22 ` [PATCH AUTOSEL 5.15 14/15] ext2: Handle fiemap on empty files to prevent EINVAL Sasha Levin
2025-08-03 21:22 ` [PATCH AUTOSEL 5.15 15/15] securityfs: don't pin dentries twice, once is enough Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250803212206.3548990-6-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=linkinjeon@kernel.org \
    --cc=neil@brown.name \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=stfrench@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox