All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Steve French <sfrench@samba.org>,
	Paulo Alcantara <pc@manguebit.com>,
	ronnie sahlberg <ronniesahlberg@gmail.com>
Cc: linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 10/35] cifs: Improve detect_directory_symlink_target() function
Date: Sun, 31 Aug 2025 14:35:37 +0200	[thread overview]
Message-ID: <20250831123602.14037-11-pali@kernel.org> (raw)
In-Reply-To: <20250831123602.14037-1-pali@kernel.org>

Function detect_directory_symlink_target() is not currently able to detect
if the target path is directory in case the path is in the DELETE_PENDING
state or the user has not granted FILE_READ_ATTRIBUTES permission on the
path. This limitation is written in TODO comment.

Resolve this problem by replacing code which determinate path type by the
query_path_info() callback, which now is able to handle all these cases.

Depends on "cifs: Improve SMB2+ stat() to work also for paths in
DELETE_PENDING state".

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/reparse.c | 75 ++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 50 deletions(-)

diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c
index 7869cec58f52..236629778e1c 100644
--- a/fs/smb/client/reparse.c
+++ b/fs/smb/client/reparse.c
@@ -250,18 +250,16 @@ static int detect_directory_symlink_target(struct cifs_sb_info *cifs_sb,
 					   bool *directory)
 {
 	char sep = CIFS_DIR_SEP(cifs_sb);
-	struct cifs_open_parms oparms;
+	struct cifs_open_info_data query_info;
 	struct tcon_link *tlink;
 	struct cifs_tcon *tcon;
 	const char *basename;
-	struct cifs_fid fid;
 	char *resolved_path;
 	int full_path_len;
 	int basename_len;
 	int symname_len;
 	char *path_sep;
-	__u32 oplock;
-	int open_rc;
+	int query_rc;
 
 	/*
 	 * First do some simple check. If the original Linux symlink target ends
@@ -284,7 +282,8 @@ static int detect_directory_symlink_target(struct cifs_sb_info *cifs_sb,
 	if (symname[0] == '/') {
 		cifs_dbg(FYI,
 			 "%s: cannot determinate if the symlink target path '%s' "
-			 "is directory or not, creating '%s' as file symlink\n",
+			 "is directory or not because path is absolute, "
+			 "creating '%s' as file symlink\n",
 			 __func__, symname, full_path);
 		return 0;
 	}
@@ -322,58 +321,34 @@ static int detect_directory_symlink_target(struct cifs_sb_info *cifs_sb,
 	if (sep == '\\')
 		convert_delimiter(path_sep, sep);
 
+	/*
+	 * Query resolved SMB symlink path and check if it is a directory or not.
+	 * Callback query_path_info() already handles cases when the server does
+	 * not grant FILE_READ_ATTRIBUTES permission for object, or when server
+	 * denies opening the object (e.g. because of DELETE_PENDING state).
+	 */
 	tcon = tlink_tcon(tlink);
-	oparms = CIFS_OPARMS(cifs_sb, tcon, resolved_path,
-			     FILE_READ_ATTRIBUTES, FILE_OPEN, 0, ACL_NO_MODE);
-	oparms.fid = &fid;
-
-	/* Try to open as a directory (NOT_FILE) */
-	oplock = 0;
-	oparms.create_options = cifs_create_options(cifs_sb,
-						    CREATE_NOT_FILE | OPEN_REPARSE_POINT);
-	open_rc = tcon->ses->server->ops->open(xid, &oparms, &oplock, NULL);
-	if (open_rc == 0) {
-		/* Successful open means that the target path is definitely a directory. */
-		*directory = true;
-		tcon->ses->server->ops->close(xid, tcon, &fid);
-	} else if (open_rc == -ENOTDIR) {
-		/* -ENOTDIR means that the target path is definitely a file. */
-		*directory = false;
-	} else if (open_rc == -ENOENT) {
+	query_rc = tcon->ses->server->ops->query_path_info(xid, tcon, cifs_sb,
+							   resolved_path, &query_info);
+	if (query_rc == 0) {
+		/* Query on path was successful, so just check for directory attr. */
+		*directory = le32_to_cpu(query_info.fi.Attributes) & ATTR_DIRECTORY;
+	} else if (query_rc == -ENOENT) {
 		/* -ENOENT means that the target path does not exist. */
 		cifs_dbg(FYI,
 			 "%s: symlink target path '%s' does not exist, "
 			 "creating '%s' as file symlink\n",
 			 __func__, symname, full_path);
 	} else {
-		/* Try to open as a file (NOT_DIR) */
-		oplock = 0;
-		oparms.create_options = cifs_create_options(cifs_sb,
-							    CREATE_NOT_DIR | OPEN_REPARSE_POINT);
-		open_rc = tcon->ses->server->ops->open(xid, &oparms, &oplock, NULL);
-		if (open_rc == 0) {
-			/* Successful open means that the target path is definitely a file. */
-			*directory = false;
-			tcon->ses->server->ops->close(xid, tcon, &fid);
-		} else if (open_rc == -EISDIR) {
-			/* -EISDIR means that the target path is definitely a directory. */
-			*directory = true;
-		} else {
-			/*
-			 * This code branch is called when we do not have a permission to
-			 * open the resolved_path or some other client/process denied
-			 * opening the resolved_path.
-			 *
-			 * TODO: Try to use ops->query_dir_first on the parent directory
-			 * of resolved_path, search for basename of resolved_path and
-			 * check if the ATTR_DIRECTORY is set in fi.Attributes. In some
-			 * case this could work also when opening of the path is denied.
-			 */
-			cifs_dbg(FYI,
-				 "%s: cannot determinate if the symlink target path '%s' "
-				 "is directory or not, creating '%s' as file symlink\n",
-				 __func__, symname, full_path);
-		}
+		/*
+		 * This code branch is called when we do not have a permission to
+		 * query the resolved_path or some other error occurred during query.
+		 */
+		cifs_dbg(FYI,
+			 "%s: cannot determinate if the symlink target path '%s' "
+			 "is directory or not because query path failed (%d), "
+			 "creating '%s' as file symlink\n",
+			 __func__, symname, query_rc, full_path);
 	}
 
 	kfree(resolved_path);
-- 
2.20.1


  parent reply	other threads:[~2025-08-31 12:36 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
2025-08-31 12:35 ` [PATCH 01/35] cifs: Fix and improve cifs_is_path_accessible() function Pali Rohár
2025-08-31 12:35 ` [PATCH 02/35] cifs: Allow fallback code in smb_set_file_info() also for directories Pali Rohár
2025-08-31 12:35 ` [PATCH 03/35] cifs: Add fallback code path for cifs_mkdir_setinfo() Pali Rohár
2025-08-31 12:35 ` [PATCH 04/35] cifs: Remove code for querying FILE_INFO_STANDARD via CIFSSMBQPathInfo() Pali Rohár
2025-08-31 12:35 ` [PATCH 05/35] cifs: Remove CIFSSMBSetPathInfoFB() fallback function Pali Rohár
2025-08-31 12:35 ` [PATCH 06/35] cifs: Remove cifs_backup_query_path_info() and replace it by cifs_query_path_info() Pali Rohár
2025-08-31 12:35 ` [PATCH 07/35] cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY Pali Rohár
2025-08-31 12:35 ` [PATCH 08/35] cifs: Improve SMB2+ stat() to work also for paths in DELETE_PENDING state Pali Rohár
2025-09-20 17:14   ` Steve French
2025-09-20 17:36     ` Pali Rohár
2025-09-20 17:39       ` Steve French
2025-09-20 17:40         ` Steve French
2025-09-20 17:51           ` Pali Rohár
2025-09-20 17:56             ` Steve French
2025-09-20 18:08               ` Pali Rohár
2025-09-20 17:26   ` Steve French
2025-09-20 17:42     ` Pali Rohár
2025-09-20 17:48       ` Steve French
2025-09-20 18:00         ` Pali Rohár
2025-09-20 18:04           ` Steve French
2025-09-24 10:47             ` Pali Rohár
2025-08-31 12:35 ` [PATCH 09/35] cifs: Improve SMB1 " Pali Rohár
2025-08-31 12:35 ` Pali Rohár [this message]
2025-08-31 12:35 ` [PATCH 11/35] cifs: Fix random name construction for cifs_rename_pending_delete() Pali Rohár
2025-08-31 12:35 ` [PATCH 12/35] cifs: Fix DELETE comments in cifs_rename_pending_delete() Pali Rohár
2025-08-31 12:35 ` [PATCH 13/35] cifs: Avoid dynamic memory allocation of FILE_BASIC_INFO buffer " Pali Rohár
2025-08-31 12:35 ` [PATCH 14/35] cifs: Extend CIFSSMBRenameOpenFile() function for overwrite parameter Pali Rohár
2025-08-31 12:35 ` [PATCH 15/35] cifs: Do not try to overwrite existing sillyname in cifs_rename_pending_delete() Pali Rohár
2025-08-31 12:35 ` [PATCH 16/35] cifs: Add comments for DeletePending assignments in open functions Pali Rohár
2025-08-31 12:35 ` [PATCH 17/35] cifs: Use NT_STATUS_DELETE_PENDING for filling fi.DeletePending in cifs_query_path_info() Pali Rohár
2025-08-31 12:35 ` [PATCH 18/35] cifs: Do not set NumberOfLinks to 1 from open or query calls Pali Rohár
2025-08-31 12:35 ` [PATCH 19/35] cifs: Fix cifs_rename_pending_delete() for files with more hardlinks Pali Rohár
2025-08-31 12:35 ` [PATCH 20/35] cifs: Fix permission logic in cifs_rename_pending_delete() Pali Rohár
2025-08-31 12:35 ` [PATCH 21/35] cifs: Propagate error code from CIFSSMBSetFileInfo() to cifs_rename_pending_delete() Pali Rohár
2025-08-31 12:35 ` [PATCH 22/35] cifs: Improve cifs_rename_pending_delete() to work without the PASSTHRU support Pali Rohár
2025-08-31 12:35 ` [PATCH 23/35] cifs: Fix SMBLegacyOpen() function Pali Rohár
2025-08-31 12:35 ` [PATCH 24/35] cifs: Add a new callback set_file_disp() for setting file disposition (delete pending state) Pali Rohár
2025-08-31 12:35 ` [PATCH 25/35] cifs: Add a new callback rename_opened_file() for renaming an opened file Pali Rohár
2025-08-31 12:35 ` [PATCH 26/35] cifs: Add SMB2+ support into cifs_rename_pending_delete() function Pali Rohár
2025-09-20 17:34   ` Steve French
2025-08-31 12:35 ` [PATCH 27/35] cifs: Move SMB1 usage of CIFSPOSIXDelFile() from inode.c to cifssmb.c Pali Rohár
2025-09-20 17:33   ` Steve French
2025-08-31 12:35 ` [PATCH 28/35] cifs: Fix smb2_unlink() to fail on directory Pali Rohár
2025-09-20 17:34   ` Steve French
2025-09-20 17:37     ` Steve French
2025-08-31 12:35 ` [PATCH 29/35] cifs: Fix smb2_rmdir() on reparse point Pali Rohár
2025-08-31 12:35 ` [PATCH 30/35] cifs: Simplify SMB2_OP_DELETE API usage Pali Rohár
2025-08-31 12:35 ` [PATCH 31/35] cifs: Deduplicate smb2_unlink() and smb2_rmdir() into one common function Pali Rohár
2025-08-31 12:35 ` [PATCH 32/35] cifs: Use cifs_rename_pending_delete() fallback also for rmdir() Pali Rohár
2025-08-31 12:36 ` [PATCH 33/35] cifs: Add a new open flag CREATE_OPTION_EXCLUSIVE to open with deny all shared reservation Pali Rohár
2025-08-31 12:36 ` [PATCH 34/35] cifs: Use CREATE_OPTION_EXCLUSIVE when opening file/dir for SMB2+ non-POSIX unlink/rmdir Pali Rohár
2025-08-31 12:36 ` [PATCH 35/35] cifs: Use CREATE_OPTION_EXCLUSIVE when doing SMB1 rmdir on NT server Pali Rohár
2025-09-01  7:55 ` [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Stefan Metzmacher
2025-09-01 17:02   ` Pali Rohár
2025-09-02 15:17     ` Stefan Metzmacher
2025-09-02 16:30       ` Pali Rohár
2025-09-07 11:05         ` Pali Rohár
2025-09-20 13:33           ` Pali Rohár

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=20250831123602.14037-11-pali@kernel.org \
    --to=pali@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pc@manguebit.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sfrench@samba.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.