Linux kernel -stable discussions
 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>,
	smfrench@gmail.com, linux-cifs@vger.kernel.org
Subject: [PATCH AUTOSEL 6.16 11/35] smb/server: avoid deadlock when linking with ReplaceIfExists
Date: Sun,  3 Aug 2025 17:17:11 -0400	[thread overview]
Message-ID: <20250803211736.3545028-11-sashal@kernel.org> (raw)
In-Reply-To: <20250803211736.3545028-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/smb/server/smb2pdu.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 63d17cea2e95..27d04180789d 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -6053,7 +6053,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) +
@@ -6086,11 +6085,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;
@@ -6098,21 +6094,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:18 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-03 21:17 [PATCH AUTOSEL 6.16 01/35] hfs: fix general protection fault in hfs_find_init() Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 02/35] hfs: fix slab-out-of-bounds in hfs_bnode_read() Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 03/35] hfsplus: fix slab-out-of-bounds in hfsplus_bnode_read() Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 04/35] hfsplus: fix slab-out-of-bounds read in hfsplus_uni2asc() Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 05/35] hfsplus: don't use BUG_ON() in hfsplus_create_attributes_file() Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 06/35] arm64: Handle KCOV __init vs inline mismatches Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 07/35] tpm: Check for completion after timeout Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 08/35] tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 09/35] firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 10/35] btrfs: fix -ENOSPC mmap write failure on NOCOW files/extents Sasha Levin
2025-08-03 21:17 ` Sasha Levin [this message]
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 12/35] nvme-pci: try function level reset on init failure Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 13/35] dm-stripe: limit chunk_sectors to the stripe size Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 14/35] md/raid10: set chunk_sectors limit Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 15/35] nvme-tcp: log TLS handshake failures at error level Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 16/35] gfs2: Validate i_depth for exhash directories Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 17/35] gfs2: Set .migrate_folio in gfs2_{rgrp,meta}_aops Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 18/35] md: call del_gendisk in control path Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 19/35] loop: Avoid updating block size under exclusive owner Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 20/35] udf: Verify partition map count Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 21/35] drbd: add missing kref_get in handle_write_conflicts Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 22/35] hfs: fix not erasing deleted b-tree node issue Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 23/35] better lockdep annotations for simple_recursive_removal() Sasha Levin
2025-08-03 21:17 ` [PATCH AUTOSEL 6.16 24/35] ata: ahci: Disallow LPM policy control if not supported 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=20250803211736.3545028-11-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=patches@lists.linux.dev \
    --cc=smfrench@gmail.com \
    --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