Skip to contentSkip to conversation

FAILED: patch "[PATCH] smb: client: clean up failed cached directory opens" failed to apply to 6.1-stable tree

PATCH New [email protected] 2 participants 4 messages Sep 29 – Sep 30

Subsystems (2)
Stable branch SMB client (CIFS)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x d2ff5fb93ea83034025850266b5eed391f96b825
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092902-hacksaw-legwork-e770@gregkh' --subject-prefix 'PATCH 6.1.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d2ff5fb93ea83034025850266b5eed391f96b825 Mon Sep 17 00:00:00 2001
From: Zihan Xi <[email protected]>
Date: Wed, 16 Sep 2026 15:29:27 +0000
Subject: [PATCH] smb: client: clean up failed cached directory opens

open_cached_dir() sends CREATE and QUERY_INFO as a compound request. If the CREATE succeeds but a later command returns an error, the function must retain the CREATE FID so common cleanup can issue SMB2_close(). It also must not treat a response error as a valid CREATE.

Validate the CREATE response before using its fields, record the FIDs, and mark the handle open before handling errors from later compound commands. Move the -EREMCHG reconnect handling before response validation so a missing response does not hide the reconnect request. Count the handle when it is marked open; confirmed close responses decrement the counter, while existing close retry behavior remains best effort on transport failures.

Fixes b0f6df737a1c ("cifs: cache FILE_ALL_INFO for the shared root handle")
Cc [email protected]
Reported-by Vega <[email protected]>
Assisted-by LLM
Co-developed-by Luxing Yin <[email protected]>
Signed-off-by Luxing Yin <[email protected]>
Signed-off-by Zihan Xi <[email protected]>
Tested-by Frank Sorenson <[email protected]>
Signed-off-by Paulo Alcantara <[email protected]>
fs/smb/client/cached_dir.c +22 -10
index 88d5e9a32f28..647fa26da4d2 100644
@@ -8,6 +8,7 @@
 #include <linux/namei.h>
 #include "cifsglob.h"
 #include "cifsproto.h"
+#include "../common/smb2status.h"
 #include "cifs_debug.h"
 #include "smb2proto.h"
 #include "cached_dir.h"

@@ -323,25 +324,37 @@ replay_again:
 	rc = compound_send_recv(xid, ses, server,
 				flags, 2, rqst,
 				resp_buftype, rsp_iov);
-	if (rc) {
-		if (rc == -EREMCHG) {
-			tcon->need_reconnect = true;
-			pr_warn_once("server share %s deleted\n",
-				     tcon->tree_name);
-		}
+	if (rc == -EREMCHG) {
+		tcon->need_reconnect = true;
+		pr_warn_once("server share %s deleted\n",
+			     tcon->tree_name);
+	}
+
+	if (!rsp_iov[0].iov_base || rsp_iov[0].iov_len < sizeof(*o_rsp)) {
+		if (!rc)
+			rc = -EIO;
 		goto oshr_free;
 	}
-	cfid->is_open = true;
-
-	spin_lock(&cfids->cfid_list_lock);
 
 	o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
+	if (o_rsp->hdr.Status != STATUS_SUCCESS) {
+		if (!rc)
+			rc = -EIO;
+		goto oshr_free;
+	}
+
 	oparms.fid->persistent_fid = o_rsp->PersistentFileId;
 	oparms.fid->volatile_fid = o_rsp->VolatileFileId;
 #ifdef CONFIG_CIFS_DEBUG2
 	oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
 #endif /* CIFS_DEBUG2 */
+	cfid->is_open = true;
+	atomic_inc(&tcon->num_remote_opens);
 
+	if (rc)
+		goto oshr_free;
+
+	spin_lock(&cfids->cfid_list_lock);
 
 	if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
 		spin_unlock(&cfids->cfid_list_lock);

@@ -408,7 +421,6 @@ out:
 		close_cached_dir(cfid);
 	} else {
 		*ret_cfid = cfid;
-		atomic_inc(&tcon->num_remote_opens);
 	}
 	kfree(utf16_path);
maintainer
#1 lore raw

PATCH 1/3 smb: client: prevent new fids from being removed by laundromat

From: Paulo Alcantara <[email protected]>

[ Upstream commit 81ba10959970d15c388bf29866b01b62f387e6a3 ]

Check if @cfid->time is set in laundromat so we guarantee that only fully cached fids will be selected for removal. While we're at it, add missing locks to protect access of @cfid fields in order to avoid races with open_cached_dir() and cfids_laundromat_worker(), respectively.

Signed-off-by Paulo Alcantara (SUSE) <[email protected]>
Reviewed-by Shyam Prasad N <[email protected]>
Signed-off-by Steve French <[email protected]>

Stable dependency adaptation for d2ff5fb93ea8:

The 6.1 tree has no cached-directory laundromat worker or its supporting fields, so omit the worker-only hunks instead of introducing that feature.

Keep the cache lookup and existing-lease check under one list lock, and protect response parsing and publication of the cached directory. Move the tcon assignment ahead of the request so the target's compound-response error path can close a successful CREATE.

Use the existing close_cached_dir() for common error cleanup. Track whether the prospective lease reference was handed to the cache so a concurrent lease break cannot cause cleanup to drop that reference twice. Keep the response and final cleanup context needed for d2ff5fb93ea8 to apply without changes. No functions are added.

Stable-dep-of: d2ff5fb93ea8 ("smb: client: clean up failed cached directory opens")

Signed-off-by Sasha Levin <[email protected]>
---
 fs/smb/client/cached_dir.c | 60 ++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 32 deletions(-)
fs/smb/client/cached_dir.c +28 -32
index fca8656929399..f6c72c6567f9d 100644
@@ -22,7 +22,6 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 {
 	struct cached_fid *cfid;
 
-	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry(cfid, &cfids->entries, entry) {
 		if (!strcmp(cfid->path, path)) {
 			/*

@@ -31,25 +30,20 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 			 * being deleted due to a lease break.
 			 */
 			if (!cfid->has_lease) {
-				spin_unlock(&cfids->cfid_list_lock);
 				return NULL;
 			}
 			kref_get(&cfid->refcount);
-			spin_unlock(&cfids->cfid_list_lock);
 			return cfid;
 		}
 	}
 	if (lookup_only) {
-		spin_unlock(&cfids->cfid_list_lock);
 		return NULL;
 	}
 	if (cfids->num_entries >= MAX_CACHED_FIDS) {
-		spin_unlock(&cfids->cfid_list_lock);
 		return NULL;
 	}
 	cfid = init_cached_dir(path);
 	if (cfid == NULL) {
-		spin_unlock(&cfids->cfid_list_lock);
 		return NULL;
 	}
 	cfid->cfids = cfids;

@@ -57,7 +51,6 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 	list_add(&cfid->entry, &cfids->entries);
 	cfid->on_list = true;
 	kref_get(&cfid->refcount);
-	spin_unlock(&cfids->cfid_list_lock);
 	return cfid;
 }
 

@@ -143,6 +136,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	struct cached_fid *cfid;
 	struct cached_fids *cfids;
 	const char *npath;
+	bool cached = false;
 
 	if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
 	    is_smb1_server(tcon->ses->server))

@@ -162,21 +156,21 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	if (!utf16_path)
 		return -ENOMEM;
 
+	spin_lock(&cfids->cfid_list_lock);
 	cfid = find_or_create_cached_dir(cfids, path, lookup_only);
 	if (cfid == NULL) {
+		spin_unlock(&cfids->cfid_list_lock);
 		kfree(utf16_path);
 		return -ENOENT;
 	}
-	/*
-	 * At this point we either have a lease already and we can just
-	 * return it. If not we are guaranteed to be the only thread accessing
-	 * this cfid.
-	 */
+	/* Return an existing leased fid while still holding the list lock. */
 	if (cfid->has_lease) {
+		spin_unlock(&cfids->cfid_list_lock);
 		*ret_cfid = cfid;
 		kfree(utf16_path);
 		return 0;
 	}
+	spin_unlock(&cfids->cfid_list_lock);
 
 	/*
 	 * Skip any prefix paths in @path as lookup_positive_unlocked() ends up

@@ -202,6 +196,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	if (smb3_encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
+	cfid->tcon = tcon;
 	pfid = &cfid->fid;
 	server->ops->new_lease_key(pfid);
 

@@ -264,9 +259,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 		}
 		goto oshr_free;
 	}
-	cfid->tcon = tcon;
 	cfid->is_open = true;
 
+	spin_lock(&cfids->cfid_list_lock);
+
 	o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
 	oparms.fid->persistent_fid = o_rsp->PersistentFileId;
 	oparms.fid->volatile_fid = o_rsp->VolatileFileId;

@@ -274,13 +270,17 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
 #endif /* CIFS_DEBUG2 */
 
-	if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE)
+
+	if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
+		spin_unlock(&cfids->cfid_list_lock);
 		goto oshr_free;
+	}
 
 	rc = smb2_parse_contexts(server, rsp_iov,
 			    &oparms.fid->epoch,
 			    oparms.fid->lease_key,
 			    &oplock, NULL, NULL);
+	spin_unlock(&cfids->cfid_list_lock);
 	if (rc)
 		goto oshr_free;
 	if (!(oplock & SMB2_LEASE_READ_CACHING_HE))

@@ -304,12 +304,14 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 			goto oshr_free;
 		}
 	}
+	spin_lock(&cfids->cfid_list_lock);
 	cfid->dentry = dentry;
 	cfid->time = jiffies;
 	cfid->has_lease = true;
+	cached = true;
+	spin_unlock(&cfids->cfid_list_lock);
 
 oshr_free:
-	kfree(utf16_path);
 	SMB2_open_free(&rqst[0]);
 	SMB2_query_info_free(&rqst[1]);
 	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);

@@ -324,27 +326,21 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 		rc = -ENOENT;
 	}
 	spin_unlock(&cfids->cfid_list_lock);
-	if (!rc && !cfid->has_lease) {
-		/*
-		 * We are guaranteed to have two references at this point.
-		 * One for the caller and one for a potential lease.
-		 * Release the Lease-ref so that the directory will be closed
-		 * when the caller closes the cached handle.
-		 */
-		kref_put(&cfid->refcount, smb2_close_cached_fid);
-	}
+	/*
+	 * A new entry has a caller reference and a prospective lease reference.
+	 * Release the latter unless it was handed to the cache.  Once cached,
+	 * lease-break or invalidation work owns that reference, even if it has
+	 * already cleared has_lease by the time we get here.
+	 */
+	if (!cached)
+		close_cached_dir(cfid);
 	if (rc) {
-		if (cfid->is_open)
-			SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
-				   cfid->fid.volatile_fid);
-		free_cached_dir(cfid);
-		cfid = NULL;
-	}
-
-	if (rc == 0) {
+		close_cached_dir(cfid);
+	} else {
 		*ret_cfid = cfid;
 		atomic_inc(&tcon->num_remote_opens);
 	}
+	kfree(utf16_path);
 
 	return rc;
 }
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/3 smb: client: fix use-after-free in smb2_query_info_compound()

From: Paulo Alcantara <[email protected]>

[ Upstream commit 5c86919455c1edec99ebd3338ad213b59271a71b ]

The following UAF was triggered when running fstests generic/072 with KASAN enabled against Windows Server 2022 and mount options 'multichannel,max_channels=2,vers=3.1.1,mfsymlinks,noperm'

  BUG: KASAN: slab-use-after-free in smb2_query_info_compound+0x423/0x6d0 [cifs]
  Read of size 8 at addr ffff888014941048 by task xfs_io/27534

  CPU: 0 PID: 27534 Comm: xfs_io Not tainted 6.6.0-rc7 #1
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
  rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014
  Call Trace:
   dump_stack_lvl+0x4a/0x80
   print_report+0xcf/0x650
   ? srso_alias_return_thunk+0x5/0x7f
   ? srso_alias_return_thunk+0x5/0x7f
   ? __phys_addr+0x46/0x90
   kasan_report+0xda/0x110
   ? smb2_query_info_compound+0x423/0x6d0 [cifs]
   ? smb2_query_info_compound+0x423/0x6d0 [cifs]
   smb2_query_info_compound+0x423/0x6d0 [cifs]
   ? __pfx_smb2_query_info_compound+0x10/0x10 [cifs]
   ? srso_alias_return_thunk+0x5/0x7f
   ? __stack_depot_save+0x39/0x480
   ? kasan_save_stack+0x33/0x60
   ? kasan_set_track+0x25/0x30
   ? ____kasan_slab_free+0x126/0x170
   smb2_queryfs+0xc2/0x2c0 [cifs]
   ? __pfx_smb2_queryfs+0x10/0x10 [cifs]
   ? __pfx___lock_acquire+0x10/0x10
   smb311_queryfs+0x210/0x220 [cifs]
   ? __pfx_smb311_queryfs+0x10/0x10 [cifs]
   ? srso_alias_return_thunk+0x5/0x7f
   ? __lock_acquire+0x480/0x26c0
   ? lock_release+0x1ed/0x640
   ? srso_alias_return_thunk+0x5/0x7f
   ? do_raw_spin_unlock+0x9b/0x100
   cifs_statfs+0x18c/0x4b0 [cifs]
   statfs_by_dentry+0x9b/0xf0
   fd_statfs+0x4e/0xb0
   __do_sys_fstatfs+0x7f/0xe0
   ? __pfx___do_sys_fstatfs+0x10/0x10
   ? srso_alias_return_thunk+0x5/0x7f
   ? lockdep_hardirqs_on_prepare+0x136/0x200
   ? srso_alias_return_thunk+0x5/0x7f
   do_syscall_64+0x3f/0x90
   entry_SYSCALL_64_after_hwframe+0x6e/0xd8

  Allocated by task 27534:
   kasan_save_stack+0x33/0x60
   kasan_set_track+0x25/0x30
   __kasan_kmalloc+0x8f/0xa0
   open_cached_dir+0x71b/0x1240 [cifs]
   smb2_query_info_compound+0x5c3/0x6d0 [cifs]
   smb2_queryfs+0xc2/0x2c0 [cifs]
   smb311_queryfs+0x210/0x220 [cifs]
   cifs_statfs+0x18c/0x4b0 [cifs]
   statfs_by_dentry+0x9b/0xf0
   fd_statfs+0x4e/0xb0
   __do_sys_fstatfs+0x7f/0xe0
   do_syscall_64+0x3f/0x90
   entry_SYSCALL_64_after_hwframe+0x6e/0xd8

  Freed by task 27534:
   kasan_save_stack+0x33/0x60
   kasan_set_track+0x25/0x30
   kasan_save_free_info+0x2b/0x50
   ____kasan_slab_free+0x126/0x170
   slab_free_freelist_hook+0xd0/0x1e0
   __kmem_cache_free+0x9d/0x1b0
   open_cached_dir+0xff5/0x1240 [cifs]
   smb2_query_info_compound+0x5c3/0x6d0 [cifs]
   smb2_queryfs+0xc2/0x2c0 [cifs]

This is a race between open_cached_dir() and cached_dir_lease_break() where the cache entry for the open directory handle receives a lease break while creating it. And before returning from open_cached_dir(), we put the last reference of the new @cfid because of !@cfid->has_lease.

Besides the UAF, while running xfstests a lot of missed lease breaks have been noticed in tests that run several concurrent statfs(2) calls on those cached fids

CIFS: VFS: \\w22-root1.gandalf.test No task to wake, unknown frame...
CIFS: VFS: \\w22-root1.gandalf.test Cmd: 18 Err: 0x0 Flags: 0x1...
CIFS: VFS: \\w22-root1.gandalf.test smb buf 00000000715bfe83 len 108
CIFS: VFS: Dump pending requests:
CIFS: VFS: \\w22-root1.gandalf.test No task to wake, unknown frame...
CIFS: VFS: \\w22-root1.gandalf.test Cmd: 18 Err: 0x0 Flags: 0x1...
CIFS: VFS: \\w22-root1.gandalf.test smb buf 000000005aa7316e len 108
...

To fix both, in open_cached_dir() ensure that @cfid->has_lease is set right before sending out compounded request so that any potential lease break will be get processed by demultiplex thread while we're still caching @cfid. And, if open failed for some reason, re-check @cfid->has_lease to decide whether or not put lease reference.

Cc [email protected]
Signed-off-by Paulo Alcantara (SUSE) <[email protected]>
Signed-off-by Steve French <[email protected]>

Stable dependency adaptation for d2ff5fb93ea8:

The preceding stable dependency already preserves the caller reference through concurrent lease-break cleanup using an explicit cache-ownership flag and close_cached_dir(). Keep that lifetime model and the current error-returning smb2_parse_contexts() API.

Retain the rejected-response error handling: fail a CREATE without a lease, a lease without read caching, or a short FILE_ALL_INFO result. Restore success only after the directory has been fully cached. Route prefix-path errors through common reference cleanup, bypassing the request buffers that have not yet been initialized.

Omit early lease publication and the associated lookup/dentry and cleanup rewrites. This branch clears has_lease in deferred lease-break work, so testing that flag alone cannot establish ownership of the lease reference. The existing cache-ownership tracking must not be replaced by the upstream unconditional free path. No functions are added, and the compound-response and final cleanup context is kept for the target.

Move the existing smb2status.h unchanged from client/ to common/ and adjust its seven existing users. The target includes the common path, which does not exist in this stable tree; without this preparation the target applies cleanly but fails to compile. This relocates definitions only and does not import the newer status-table implementation.

Stable-dep-of: d2ff5fb93ea8 ("smb: client: clean up failed cached directory opens")

Signed-off-by Sasha Levin <[email protected]>
---
 fs/smb/client/cached_dir.c             | 8 ++++++--
 fs/smb/client/smb2file.c               | 2 +-
 fs/smb/client/smb2inode.c              | 2 +-
 fs/smb/client/smb2maperror.c           | 2 +-
 fs/smb/client/smb2misc.c               | 2 +-
 fs/smb/client/smb2ops.c                | 2 +-
 fs/smb/client/smb2pdu.c                | 2 +-
 fs/smb/client/smb2transport.c          | 2 +-
 fs/smb/{client => common}/smb2status.h | 0
 9 files changed, 13 insertions(+), 9 deletions(-)
 rename fs/smb/{client => common}/smb2status.h (100%)
fs/smb/client/cached_dir.c +6 -2
index f6c72c6567f9d..d7cab84644a45 100644
@@ -182,8 +182,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	npath = path_no_prefix(cifs_sb, path);
 	if (IS_ERR(npath)) {
 		rc = PTR_ERR(npath);
-		kfree(utf16_path);
-		return rc;
+		goto out;
 	}
 
 	/*

@@ -273,6 +272,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 
 	if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
 		spin_unlock(&cfids->cfid_list_lock);
+		rc = -EINVAL;
 		goto oshr_free;
 	}
 

@@ -283,6 +283,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	spin_unlock(&cfids->cfid_list_lock);
 	if (rc)
 		goto oshr_free;
+	rc = -EINVAL;
 	if (!(oplock & SMB2_LEASE_READ_CACHING_HE))
 		goto oshr_free;
 	qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;

@@ -310,12 +311,15 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	cfid->has_lease = true;
 	cached = true;
 	spin_unlock(&cfids->cfid_list_lock);
+	/* At this point the directory handle is fully cached */
+	rc = 0;
 
 oshr_free:
 	SMB2_open_free(&rqst[0]);
 	SMB2_query_info_free(&rqst[1]);
 	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
 	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
+out:
 	spin_lock(&cfids->cfid_list_lock);
 	if (rc && !cfid->has_lease) {
 		if (cfid->on_list) {
fs/smb/client/smb2file.c +1 -1
index 2d0dd87dae0dd..4bf62a2487bb7 100644
@@ -20,7 +20,7 @@
 #include "cifs_unicode.h"
 #include "fscache.h"
 #include "smb2proto.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 
 static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
 {
fs/smb/client/smb2inode.c +1 -1
index 452a60e09cbed..9dc7b1877a75b 100644
@@ -24,7 +24,7 @@
 #include "smb2pdu.h"
 #include "smb2proto.h"
 #include "cached_dir.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 
 static void
 free_set_inf_compound(struct smb_rqst *rqst)
fs/smb/client/smb2maperror.c +1 -1
index 194799ddd3828..4c63d3439bf4f 100644
@@ -12,7 +12,7 @@
 #include "cifs_debug.h"
 #include "smb2pdu.h"
 #include "smb2proto.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 #include "smb2glob.h"
 #include "trace.h"
fs/smb/client/smb2misc.c +1 -1
index cd00bf48c9a56..c67b77c4858f4 100644
@@ -13,7 +13,7 @@
 #include "smb2proto.h"
 #include "cifs_debug.h"
 #include "cifs_unicode.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 #include "smb2glob.h"
 #include "nterr.h"
 #include "cached_dir.h"
fs/smb/client/smb2ops.c +1 -1
index 2b38e8e7efd4e..fbd9f7f03fdfc 100644
@@ -21,7 +21,7 @@
 #include "cifsproto.h"
 #include "cifs_debug.h"
 #include "cifs_unicode.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 #include "smb2glob.h"
 #include "cifs_ioctl.h"
 #include "smbdirect.h"
fs/smb/client/smb2pdu.c +1 -1
index 98d6fcdf22365..9ea286d20f88a 100644
@@ -30,7 +30,7 @@
 #include "cifs_unicode.h"
 #include "cifs_debug.h"
 #include "ntlmssp.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 #include "smb2glob.h"
 #include "cifspdu.h"
 #include "cifs_spnego.h"
fs/smb/client/smb2transport.c +1 -1
index 3a69f3500acde..797a147366851 100644
@@ -24,7 +24,7 @@
 #include "cifsproto.h"
 #include "smb2proto.h"
 #include "cifs_debug.h"
-#include "smb2status.h"
+#include "../common/smb2status.h"
 #include "smb2glob.h"
 
 static int
fs/smb/common/smb2status.h renamed from fs/smb/client/smb2status.h +0 -0
similarity index 100%
rename from fs/smb/client/smb2status.h
rename to fs/smb/common/smb2status.h
Signature · 1 line
2.53.0
maintainer
#3 lore raw

PATCH 3/3 smb: client: clean up failed cached directory opens

In reply to Sasha Levin

From: Zihan Xi <[email protected]>

[ Upstream commit d2ff5fb93ea83034025850266b5eed391f96b825 ]

open_cached_dir() sends CREATE and QUERY_INFO as a compound request. If the CREATE succeeds but a later command returns an error, the function must retain the CREATE FID so common cleanup can issue SMB2_close(). It also must not treat a response error as a valid CREATE.

Validate the CREATE response before using its fields, record the FIDs, and mark the handle open before handling errors from later compound commands. Move the -EREMCHG reconnect handling before response validation so a missing response does not hide the reconnect request. Count the handle when it is marked open; confirmed close responses decrement the counter, while existing close retry behavior remains best effort on transport failures.

Fixes b0f6df737a1c ("cifs: cache FILE_ALL_INFO for the shared root handle")
Cc [email protected]
Reported-by Vega <[email protected]>
Assisted-by LLM
Co-developed-by Luxing Yin <[email protected]>
Signed-off-by Luxing Yin <[email protected]>
Signed-off-by Zihan Xi <[email protected]>
Tested-by Frank Sorenson <[email protected]>
Signed-off-by Paulo Alcantara <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
---
 fs/smb/client/cached_dir.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)
fs/smb/client/cached_dir.c +22 -10
index d7cab84644a45..476ef68ba0c68 100644
@@ -8,6 +8,7 @@
 #include <linux/namei.h>
 #include "cifsglob.h"
 #include "cifsproto.h"
+#include "../common/smb2status.h"
 #include "cifs_debug.h"
 #include "smb2proto.h"
 #include "cached_dir.h"

@@ -250,25 +251,37 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 	rc = compound_send_recv(xid, ses, server,
 				flags, 2, rqst,
 				resp_buftype, rsp_iov);
-	if (rc) {
-		if (rc == -EREMCHG) {
-			tcon->need_reconnect = true;
-			pr_warn_once("server share %s deleted\n",
-				     tcon->tree_name);
-		}
-		goto oshr_free;
+	if (rc == -EREMCHG) {
+		tcon->need_reconnect = true;
+		pr_warn_once("server share %s deleted\n",
+			     tcon->tree_name);
 	}
-	cfid->is_open = true;
 
-	spin_lock(&cfids->cfid_list_lock);
+	if (!rsp_iov[0].iov_base || rsp_iov[0].iov_len < sizeof(*o_rsp)) {
+		if (!rc)
+			rc = -EIO;
+		goto oshr_free;
+	}
 
 	o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
+	if (o_rsp->hdr.Status != STATUS_SUCCESS) {
+		if (!rc)
+			rc = -EIO;
+		goto oshr_free;
+	}
+
 	oparms.fid->persistent_fid = o_rsp->PersistentFileId;
 	oparms.fid->volatile_fid = o_rsp->VolatileFileId;
 #ifdef CONFIG_CIFS_DEBUG2
 	oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
 #endif /* CIFS_DEBUG2 */
+	cfid->is_open = true;
+	atomic_inc(&tcon->num_remote_opens);
 
+	if (rc)
+		goto oshr_free;
+
+	spin_lock(&cfids->cfid_list_lock);
 
 	if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
 		spin_unlock(&cfids->cfid_list_lock);

@@ -342,7 +355,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 		close_cached_dir(cfid);
 	} else {
 		*ret_cfid = cfid;
-		atomic_inc(&tcon->num_remote_opens);
 	}
 	kfree(utf16_path);
Signature · 1 line
2.53.0