linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers
@ 2025-08-31 12:35 Pali Rohár
  2025-08-31 12:35 ` [PATCH 01/35] cifs: Fix and improve cifs_is_path_accessible() function Pali Rohár
                   ` (35 more replies)
  0 siblings, 36 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

This patch series improves Linux rmdir() and unlink() syscalls called on
SMB mounts exported from Windows SMB servers which do not implement
POSIX semantics of the file and directory removal.

This patch series should have no impact and no function change when
communicating with the POSIX SMB servers, as they should implement
proper rmdir and unlink logic.

When issuing remove path command against non-POSIX / Windows SMB server,
it let the directory entry which is being removed in the directory until
all users / clients close all handles / references to that path.

POSIX requires from rmdir() and unlink() syscalls that after successful
call, the requested path / directory entry is released and allows to
create a new file or directory with that name. This is currently not
working against non-POSIX / Windows SMB servers.

To workaround this problem fix and improve existing cifs silly rename
code and extend it also to SMB2 and SMB3 dialects when communicating
with Windows SMB servers. Silly rename is applied only when it is
necessary (when some other client has opened file or directory).
If no other client has the file / dir open then silly rename is not
used.

With this patch series, after successful rmdir() or unlink() call from
Linux userspace application, the path is released, it is not visible in
the followup readdir() call, also stat() properly returns ENOENT and it
is possible to call creat() or mkdir() on the same path to create new
node. Silly rename is used to ensure that path is released.

Before this change, the original path was visible in the readdir() call
and create() or mkdir() was failing with EEXIST, even rmdir() or
unlink() on that path returned success.


Test cases when "path" is opened on Windows server by other client, so
path cannot be removed.

1.
  unlink("path");
  mkdir("path");

  before: unlink returns success but mkdir returns EEXIST.
  after: unlink returns success, mkdir creates new directory and the
  orignal one is silly renamed.

2.
  unlink("path")
  open("path") without O_CREAT
  mkdir("path")

  before: unlink returns success, open returns ENOENT and mkdir returns
  EEXIST.
  after: unlink returns success, open returns ENOENT, mkdir creates
  new directory and the original path is silly renamed.

3.
  unlink("path")
  stat("path")
  mkdir("path")

  before: unlink returns success, stat returns ENOENT and mkdir returns
  EEXIST.
  after: unlink returns success, stat returns ENOENT, mkdir creates new
  directory and the original path is silly renamed.

4.
  unlink("path")
  stat("path")
  open("path") with O_CREAT | O_EXCL

  before: unlink returns success, stat returns ENOENT and open returns
  EEXIST.
  after: unlink returns success, stat returns ENOENT, open creates new
  file and the original path is silly renamed.

5.
  unlink("path")
  readdir("parent_of_path")

  before: unlink returns success but readdir returns entry corresponding
  to path.
  after: unlink returns success, original path is silly renamed and
  readdir returns the entry under new silly name.


Test case when "tmp_path" on the Windows server was created (or opened)
as a exclusive temporary file with the DELETE_PENDING flag set:

Before:

  stat("tmp_path") - returns ENOENT
  mkdir("tmp_path") - returns EEXIST
  open("tmp_path") without O_CREAT - returns ENOENT
  open("tmp_path") with O_CREAT | O_EXCL - return EEXIST
  unlink("tmp_path") - returns ENOENT
  readdir(parent_of_tmp_path) - returns the "tmp_path" entry

After:

  stat("tmp_path") - success
  mkdir("tmp_path") - returns EEXIST
  open("tmp_path") without O_CREAT - returns EBUSY
  open("tmp_path") with O_CREAT | O_EXCL - return EEXIST
  unlink("tmp_path") - returns EBUSY
  readdir(parent_of_tmp_path) - returns the "tmp_path" entry

The difference is in stat(), open() and unlink() syscalls. Now Linux
applications can stat such files, so file attributes are now visible in
"ls -l -a" output, but trying to open / modify / delete them fails with
EBUSY instead of ENOENT.


Pali Rohár (35):
  cifs: Fix and improve cifs_is_path_accessible() function
  cifs: Allow fallback code in smb_set_file_info() also for directories
  cifs: Add fallback code path for cifs_mkdir_setinfo()
  cifs: Remove code for querying FILE_INFO_STANDARD via
    CIFSSMBQPathInfo()
  cifs: Remove CIFSSMBSetPathInfoFB() fallback function
  cifs: Remove cifs_backup_query_path_info() and replace it by
    cifs_query_path_info()
  cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY
  cifs: Improve SMB2+ stat() to work also for paths in DELETE_PENDING
    state
  cifs: Improve SMB1 stat() to work also for paths in DELETE_PENDING
    state
  cifs: Improve detect_directory_symlink_target() function
  cifs: Fix random name construction for cifs_rename_pending_delete()
  cifs: Fix DELETE comments in cifs_rename_pending_delete()
  cifs: Avoid dynamic memory allocation of FILE_BASIC_INFO buffer in
    cifs_rename_pending_delete()
  cifs: Extend CIFSSMBRenameOpenFile() function for overwrite parameter
  cifs: Do not try to overwrite existing sillyname in
    cifs_rename_pending_delete()
  cifs: Add comments for DeletePending assignments in open functions
  cifs: Use NT_STATUS_DELETE_PENDING for filling fi.DeletePending in
    cifs_query_path_info()
  cifs: Do not set NumberOfLinks to 1 from open or query calls
  cifs: Fix cifs_rename_pending_delete() for files with more hardlinks
  cifs: Fix permission logic in cifs_rename_pending_delete()
  cifs: Propagate error code from CIFSSMBSetFileInfo() to
    cifs_rename_pending_delete()
  cifs: Improve cifs_rename_pending_delete() to work without the
    PASSTHRU support
  cifs: Fix SMBLegacyOpen() function
  cifs: Add a new callback set_file_disp() for setting file disposition
    (delete pending state)
  cifs: Add a new callback rename_opened_file() for renaming an opened
    file
  cifs: Add SMB2+ support into cifs_rename_pending_delete() function.
  cifs: Move SMB1 usage of CIFSPOSIXDelFile() from inode.c to cifssmb.c
  cifs: Fix smb2_unlink() to fail on directory
  cifs: Fix smb2_rmdir() on reparse point
  cifs: Simplify SMB2_OP_DELETE API usage
  cifs: Deduplicate smb2_unlink() and smb2_rmdir() into one common
    function
  cifs: Use cifs_rename_pending_delete() fallback also for rmdir()
  cifs: Add a new open flag CREATE_OPTION_EXCLUSIVE to open with deny
    all shared reservation
  cifs: Use CREATE_OPTION_EXCLUSIVE when opening file/dir for SMB2+
    non-POSIX unlink/rmdir
  cifs: Use CREATE_OPTION_EXCLUSIVE when doing SMB1 rmdir on NT server

 fs/smb/client/cifsglob.h     |  11 +-
 fs/smb/client/cifspdu.h      |   5 +
 fs/smb/client/cifsproto.h    |   6 +-
 fs/smb/client/cifssmb.c      | 228 +++++++++++++----------
 fs/smb/client/inode.c        | 346 ++++++++++++++++-------------------
 fs/smb/client/netmisc.c      |   2 +-
 fs/smb/client/reparse.c      |  75 +++-----
 fs/smb/client/smb1ops.c      |  84 +++++++--
 fs/smb/client/smb2glob.h     |   1 +
 fs/smb/client/smb2inode.c    | 264 +++++++++++++++++++++++---
 fs/smb/client/smb2maperror.c |   2 +-
 fs/smb/client/smb2ops.c      |  24 +++
 fs/smb/client/smb2pdu.c      |  51 +++++-
 fs/smb/client/smb2proto.h    |   6 +
 14 files changed, 704 insertions(+), 401 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/35] cifs: Fix and improve cifs_is_path_accessible() function
  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 ` 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
                   ` (34 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Do not call SMBQueryInformation() command for path with SMB wildcard
characters on non-UNICODE connection because server expands wildcards.
Function cifs_is_path_accessible() needs to check if the real path exists
and must not expand wildcard characters.

Do not dynamically allocate memory for small FILE_ALL_INFO structure and
instead allocate it on the stack. This structure is allocated on stack by
all other functions.

When CAP_NT_SMBS was not negotiated then do not issue CIFSSMBQPathInfo()
command. This command returns failure by non-NT Win9x SMB servers, so there
is no need try it. The purpose of cifs_is_path_accessible() function is
just to check if the path is accessible, so SMBQueryInformation() for old
servers is enough.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb1ops.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 893a1ea8c000..1772f30419a9 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -521,21 +521,27 @@ static int
 cifs_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
 			struct cifs_sb_info *cifs_sb, const char *full_path)
 {
-	int rc;
-	FILE_ALL_INFO *file_info;
+	int rc = -EOPNOTSUPP;
+	FILE_ALL_INFO file_info;
 
-	file_info = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
-	if (file_info == NULL)
-		return -ENOMEM;
+	if (tcon->ses->capabilities & CAP_NT_SMBS)
+		rc = CIFSSMBQPathInfo(xid, tcon, full_path, &file_info,
+				      0 /* not legacy */, cifs_sb->local_nls,
+				      cifs_remap(cifs_sb));
 
-	rc = CIFSSMBQPathInfo(xid, tcon, full_path, file_info,
-			      0 /* not legacy */, cifs_sb->local_nls,
-			      cifs_remap(cifs_sb));
+	/*
+	 * Non-UNICODE variant of fallback functions below expands wildcards,
+	 * so they cannot be used for querying paths with wildcard characters.
+	 * Therefore for such paths returns -ENOENT as they cannot exist.
+	 */
+	if ((rc == -EOPNOTSUPP || rc == -EINVAL) &&
+	    !(tcon->ses->capabilities & CAP_UNICODE) &&
+	    strpbrk(full_path, "*?\"><"))
+		rc = -ENOENT;
 
 	if (rc == -EOPNOTSUPP || rc == -EINVAL)
-		rc = SMBQueryInformation(xid, tcon, full_path, file_info,
+		rc = SMBQueryInformation(xid, tcon, full_path, &file_info,
 				cifs_sb->local_nls, cifs_remap(cifs_sb));
-	kfree(file_info);
 	return rc;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/35] cifs: Allow fallback code in smb_set_file_info() also for directories
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 03/35] cifs: Add fallback code path for cifs_mkdir_setinfo() Pali Rohár
                   ` (33 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

On NT systems, it is possible to do SMB open call also for directories.
Open argument CREATE_NOT_DIR disallows opening directories. So in fallback
code path in smb_set_file_info() remove CREATE_NOT_DIR restriction to allow
it also for directories.

Similar fallback is implemented also in CIFSSMBSetPathInfoFB() function and
this function already allows to call operation for directories.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb1ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 1772f30419a9..cc4feebbdd11 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -980,7 +980,7 @@ smb_set_file_info(struct inode *inode, const char *full_path,
 		.tcon = tcon,
 		.cifs_sb = cifs_sb,
 		.desired_access = SYNCHRONIZE | FILE_WRITE_ATTRIBUTES,
-		.create_options = cifs_create_options(cifs_sb, CREATE_NOT_DIR),
+		.create_options = cifs_create_options(cifs_sb, 0),
 		.disposition = FILE_OPEN,
 		.path = full_path,
 		.fid = &fid,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/35] cifs: Add fallback code path for cifs_mkdir_setinfo()
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 04/35] cifs: Remove code for querying FILE_INFO_STANDARD via CIFSSMBQPathInfo() Pali Rohár
                   ` (32 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Use SMBSetInformation() as a fallback function (when CIFSSMBSetPathInfo()
fails) which can set attribudes on the directory, including changing
read-only attribute.

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

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index cc4feebbdd11..de415b9945ee 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -824,6 +824,11 @@ cifs_mkdir_setinfo(struct inode *inode, const char *full_path,
 	info.Attributes = cpu_to_le32(dosattrs);
 	rc = CIFSSMBSetPathInfo(xid, tcon, full_path, &info, cifs_sb->local_nls,
 				cifs_sb);
+	if (rc == -EOPNOTSUPP || rc == -EINVAL)
+		rc = SMBSetInformation(xid, tcon, full_path,
+				       info.Attributes,
+				       0 /* do not change write time */,
+				       cifs_sb->local_nls, cifs_sb);
 	if (rc == 0)
 		cifsInode->cifsAttrs = dosattrs;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/35] cifs: Remove code for querying FILE_INFO_STANDARD via CIFSSMBQPathInfo()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (2 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 05/35] cifs: Remove CIFSSMBSetPathInfoFB() fallback function Pali Rohár
                   ` (31 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Querying FILE_INFO_STANDARD structure via SMB_INFO_STANDARD level over
TRANS2_QUERY_PATH_INFORMATION or TRANS2_QUERY_FILE_INFORMATION command
(implemented in CIFSSMBQPathInfo() when called with argument legacy=true)
is mostly unusable.

Win9x SMB server returns over those commands the FILE_INFO_STANDARD
structure with swapped TIME and DATE fields, compared with [MS-CIFS] spec
and Samba server implementation. Therefore this command cannot be used
unless we know against which server implementation we are connected.

There are already two fallback mechanisms for querying information about
path which are working correctly against Samba, NT and Win9x servers:
CIFSFindFirst() and SMBQueryInformation() commands.

So remove TRANS2_QUERY_PATH_INFORMATION/SMB_INFO_STANDARD code from
CIFSSMBQPathInfo() function, when the function is called with legacy=true.

Note that there is no use of CIFSSMBQPathInfo(legacy=true) anymore.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsproto.h |  1 -
 fs/smb/client/cifssmb.c   | 22 +++-------------------
 fs/smb/client/smb1ops.c   |  4 ++--
 3 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index c34c533b2efa..f467b24fd984 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -368,7 +368,6 @@ extern int CIFSSMBQFileInfo(const unsigned int xid, struct cifs_tcon *tcon,
 			u16 netfid, FILE_ALL_INFO *pFindData);
 extern int CIFSSMBQPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 			    const char *search_Name, FILE_ALL_INFO *data,
-			    int legacy /* whether to use old info level */,
 			    const struct nls_table *nls_codepage, int remap);
 extern int SMBQueryInformation(const unsigned int xid, struct cifs_tcon *tcon,
 			       const char *search_name, FILE_ALL_INFO *data,
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index d20766f664c4..42ab901a08e7 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -3856,7 +3856,6 @@ CIFSSMBQFileInfo(const unsigned int xid, struct cifs_tcon *tcon,
 int
 CIFSSMBQPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 		 const char *search_name, FILE_ALL_INFO *data,
-		 int legacy /* old style infolevel */,
 		 const struct nls_table *nls_codepage, int remap)
 {
 	/* level 263 SMB_QUERY_FILE_ALL_INFO */
@@ -3904,10 +3903,7 @@ CIFSSMBQPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 	byte_count = params + 1 /* pad */ ;
 	pSMB->TotalParameterCount = cpu_to_le16(params);
 	pSMB->ParameterCount = pSMB->TotalParameterCount;
-	if (legacy)
-		pSMB->InformationLevel = cpu_to_le16(SMB_INFO_STANDARD);
-	else
-		pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
+	pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
 	pSMB->Reserved4 = 0;
 	inc_rfc1001_len(pSMB, byte_count);
 	pSMB->ByteCount = cpu_to_le16(byte_count);
@@ -3921,25 +3917,13 @@ CIFSSMBQPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 
 		if (rc) /* BB add auto retry on EOPNOTSUPP? */
 			rc = -EIO;
-		else if (!legacy && get_bcc(&pSMBr->hdr) < 40)
+		else if (get_bcc(&pSMBr->hdr) < 40)
 			rc = -EIO;	/* bad smb */
-		else if (legacy && get_bcc(&pSMBr->hdr) < 24)
-			rc = -EIO;  /* 24 or 26 expected but we do not read
-					last field */
 		else if (data) {
 			int size;
 			__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
 
-			/*
-			 * On legacy responses we do not read the last field,
-			 * EAsize, fortunately since it varies by subdialect and
-			 * also note it differs on Set vs Get, ie two bytes or 4
-			 * bytes depending but we don't care here.
-			 */
-			if (legacy)
-				size = sizeof(FILE_INFO_STANDARD);
-			else
-				size = sizeof(FILE_ALL_INFO);
+			size = sizeof(FILE_ALL_INFO);
 			memcpy((char *) data, (char *) &pSMBr->hdr.Protocol +
 			       data_offset, size);
 		} else
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index de415b9945ee..9dca458dbc96 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -526,7 +526,7 @@ cifs_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
 
 	if (tcon->ses->capabilities & CAP_NT_SMBS)
 		rc = CIFSSMBQPathInfo(xid, tcon, full_path, &file_info,
-				      0 /* not legacy */, cifs_sb->local_nls,
+				      cifs_sb->local_nls,
 				      cifs_remap(cifs_sb));
 
 	/*
@@ -571,7 +571,7 @@ static int cifs_query_path_info(const unsigned int xid,
 	 * do not even use CIFSSMBQPathInfo() or CIFSSMBQFileInfo() function.
 	 */
 	if (tcon->ses->capabilities & CAP_NT_SMBS)
-		rc = CIFSSMBQPathInfo(xid, tcon, full_path, &fi, 0 /* not legacy */,
+		rc = CIFSSMBQPathInfo(xid, tcon, full_path, &fi,
 				      cifs_sb->local_nls, cifs_remap(cifs_sb));
 
 	/*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/35] cifs: Remove CIFSSMBSetPathInfoFB() fallback function
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (3 preceding siblings ...)
  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 ` 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
                   ` (30 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

This fallback function CIFSSMBSetPathInfoFB() is called only from
CIFSSMBSetPathInfo() function. CIFSSMBSetPathInfo() is used in
smb_set_file_info() which contains all required fallback code, including
fallback via filehandle.

So the CIFSSMBSetPathInfoFB() is just code duplication, which is not needed
anymore. Therefore remove it.

This change depends on other changes which are extending
cifs_mkdir_setinfo() and smb_set_file_info() functions:
"Fix changing times and read-only attr over SMB1 smb_set_file_info()
function" and "Add fallback code path for cifs_mkdir_setinfo()"

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 42ab901a08e7..0d773860fd6c 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -5518,38 +5518,6 @@ CIFSSMBSetFileDisposition(const unsigned int xid, struct cifs_tcon *tcon,
 	return rc;
 }
 
-static int
-CIFSSMBSetPathInfoFB(const unsigned int xid, struct cifs_tcon *tcon,
-		     const char *fileName, const FILE_BASIC_INFO *data,
-		     const struct nls_table *nls_codepage,
-		     struct cifs_sb_info *cifs_sb)
-{
-	int oplock = 0;
-	struct cifs_open_parms oparms;
-	struct cifs_fid fid;
-	int rc;
-
-	oparms = (struct cifs_open_parms) {
-		.tcon = tcon,
-		.cifs_sb = cifs_sb,
-		.desired_access = GENERIC_WRITE,
-		.create_options = cifs_create_options(cifs_sb, 0),
-		.disposition = FILE_OPEN,
-		.path = fileName,
-		.fid = &fid,
-	};
-
-	rc = CIFS_open(xid, &oparms, &oplock, NULL);
-	if (rc)
-		goto out;
-
-	rc = CIFSSMBSetFileInfo(xid, tcon, data, fid.netfid, current->tgid);
-	CIFSSMBClose(xid, tcon, fid.netfid);
-out:
-
-	return rc;
-}
-
 int
 CIFSSMBSetPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 		   const char *fileName, const FILE_BASIC_INFO *data,
@@ -5626,10 +5594,6 @@ CIFSSMBSetPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 	if (rc == -EAGAIN)
 		goto SetTimesRetry;
 
-	if (rc == -EOPNOTSUPP)
-		return CIFSSMBSetPathInfoFB(xid, tcon, fileName, data,
-					    nls_codepage, cifs_sb);
-
 	return rc;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/35] cifs: Remove cifs_backup_query_path_info() and replace it by cifs_query_path_info()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (4 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 05/35] cifs: Remove CIFSSMBSetPathInfoFB() fallback function Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 07/35] cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY Pali Rohár
                   ` (29 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Response handling of cifs_backup_query_path_info() function in
cifs_get_fattr() is broken and can cause buffer overflows because
cifs_backup_query_path_info() prepares request with different info levels
but the response parser in cifs_get_fattr() always expects response
structure FILE_DIRECTORY_INFO.

Code which queries file/dir attributes via CIFSFindFirst() is already
implemented in cifs_query_path_info() function, so extend it for
backup_cred(), which is the only missing functionality compared to
cifs_backup_query_path_info().

With this change the cifs_query_path_info() would do everything which is
open-coded in cifs_set_fattr_ino() and cifs_backup_query_path_info()
functions for SMB1. So remove that SMB1 code from cifs_set_fattr_ino() and
also remove whole cifs_backup_query_path_info() function.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c   | 97 -----------------------------------------
 fs/smb/client/smb1ops.c |  7 ++-
 2 files changed, 5 insertions(+), 99 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 75be4b46bc6f..cd06598eacbd 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1046,61 +1046,6 @@ static __u64 simple_hashstr(const char *str)
 	return hash;
 }
 
-#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
-/**
- * cifs_backup_query_path_info - SMB1 fallback code to get ino
- *
- * Fallback code to get file metadata when we don't have access to
- * full_path (EACCES) and have backup creds.
- *
- * @xid:	transaction id used to identify original request in logs
- * @tcon:	information about the server share we have mounted
- * @sb:	the superblock stores info such as disk space available
- * @full_path:	name of the file we are getting the metadata for
- * @resp_buf:	will be set to cifs resp buf and needs to be freed with
- * 		cifs_buf_release() when done with @data
- * @data:	will be set to search info result buffer
- */
-static int
-cifs_backup_query_path_info(int xid,
-			    struct cifs_tcon *tcon,
-			    struct super_block *sb,
-			    const char *full_path,
-			    void **resp_buf,
-			    FILE_ALL_INFO **data)
-{
-	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
-	struct cifs_search_info info = {0};
-	u16 flags;
-	int rc;
-
-	*resp_buf = NULL;
-	info.endOfSearch = false;
-	if (tcon->unix_ext)
-		info.info_level = SMB_FIND_FILE_UNIX;
-	else if ((tcon->ses->capabilities &
-		  tcon->ses->server->vals->cap_nt_find) == 0)
-		info.info_level = SMB_FIND_FILE_INFO_STANDARD;
-	else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
-		info.info_level = SMB_FIND_FILE_ID_FULL_DIR_INFO;
-	else /* no srvino useful for fallback to some netapp */
-		info.info_level = SMB_FIND_FILE_DIRECTORY_INFO;
-
-	flags = CIFS_SEARCH_CLOSE_ALWAYS |
-		CIFS_SEARCH_CLOSE_AT_END |
-		CIFS_SEARCH_BACKUP_SEARCH;
-
-	rc = CIFSFindFirst(xid, tcon, full_path,
-			   cifs_sb, NULL, flags, &info, false);
-	if (rc)
-		return rc;
-
-	*resp_buf = (void *)info.ntwrk_buf_start;
-	*data = (FILE_ALL_INFO *)info.srch_entries_start;
-	return 0;
-}
-#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
-
 static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_block *sb,
 			       struct inode **inode, const char *full_path,
 			       struct cifs_open_info_data *data, struct cifs_fattr *fattr)
@@ -1314,45 +1259,6 @@ static int cifs_get_fattr(struct cifs_open_info_data *data,
 		cifs_create_junction_fattr(fattr, sb);
 		rc = 0;
 		break;
-	case -EACCES:
-#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
-		/*
-		 * perm errors, try again with backup flags if possible
-		 *
-		 * For SMB2 and later the backup intent flag
-		 * is already sent if needed on open and there
-		 * is no path based FindFirst operation to use
-		 * to retry with
-		 */
-		if (backup_cred(cifs_sb) && is_smb1_server(server)) {
-			/* for easier reading */
-			FILE_ALL_INFO *fi;
-			FILE_DIRECTORY_INFO *fdi;
-			SEARCH_ID_FULL_DIR_INFO *si;
-
-			rc = cifs_backup_query_path_info(xid, tcon, sb,
-							 full_path,
-							 &smb1_backup_rsp_buf,
-							 &fi);
-			if (rc)
-				goto out;
-
-			move_cifs_info_to_smb2(&data->fi, fi);
-			fdi = (FILE_DIRECTORY_INFO *)fi;
-			si = (SEARCH_ID_FULL_DIR_INFO *)fi;
-
-			cifs_dir_info_to_fattr(fattr, fdi, cifs_sb);
-			fattr->cf_uniqueid = le64_to_cpu(si->UniqueId);
-			/* uniqueid set, skip get inum step */
-			goto handle_mnt_opt;
-		} else {
-			/* nothing we can do, bail out */
-			goto out;
-		}
-#else
-		goto out;
-#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
-		break;
 	default:
 		cifs_dbg(FYI, "%s: unhandled err rc %d\n", __func__, rc);
 		goto out;
@@ -1367,9 +1273,6 @@ static int cifs_get_fattr(struct cifs_open_info_data *data,
 	/*
 	 * 4. Tweak fattr based on mount options
 	 */
-#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
-handle_mnt_opt:
-#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
 	/* query for SFU type info if supported and needed */
 	if ((fattr->cf_cifsattrs & ATTR_SYSTEM) &&
 	    (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL)) {
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 9dca458dbc96..55ab8a5b150c 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -584,15 +584,18 @@ static int cifs_query_path_info(const unsigned int xid,
 	/*
 	 * Then fallback to CIFSFindFirst() which works also with non-NT servers
 	 * but does not does not provide NumberOfLinks.
+	 * Can be used with backup intent flag to overcome -EACCES error.
 	 */
-	if ((rc == -EOPNOTSUPP || rc == -EINVAL) &&
+	if ((rc == -EOPNOTSUPP || rc == -EINVAL ||
+	     (backup_cred(cifs_sb) && rc == -EACCES)) &&
 	    !non_unicode_wildcard) {
 		if (!(tcon->ses->capabilities & tcon->ses->server->vals->cap_nt_find))
 			search_info.info_level = SMB_FIND_FILE_INFO_STANDARD;
 		else
 			search_info.info_level = SMB_FIND_FILE_FULL_DIRECTORY_INFO;
 		rc = CIFSFindFirst(xid, tcon, full_path, cifs_sb, NULL,
-				   CIFS_SEARCH_CLOSE_ALWAYS | CIFS_SEARCH_CLOSE_AT_END,
+				   CIFS_SEARCH_CLOSE_ALWAYS | CIFS_SEARCH_CLOSE_AT_END |
+				    (backup_cred(cifs_sb) ? CIFS_SEARCH_BACKUP_SEARCH : 0),
 				   &search_info, false);
 		if (rc == 0) {
 			if (!(tcon->ses->capabilities & tcon->ses->server->vals->cap_nt_find)) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/35] cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (5 preceding siblings ...)
  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 ` 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
                   ` (28 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

STATUS_DELETE_PENDING error is returned when trying to open a file which is
in delete pending state. Linux SMB client currently translates this error
to -ENOENT. So Linux application trying to open a file which still exists
will receive -ENOENT error. This is confusing as -ENONET means that
directory entry does not exist.

File on SMB server can be in delete pending state for an indefinite long
period. Moreover it does not have to final state before the real deleting,
as any SMB client who still have opened handle to such file can revert file
from delete pending state back to normal state. And therefore client can
cancel any scheduled file removal.

So change translation of STATUS_DELETE_PENDING error to -EBUSY. -EBUSY is
used also for STATUS_SHARING_VIOLATION error which is similar case, when
opening a file was disallowed by server due to concurrent usage.

For SMB1, STATUS_DELETE_PENDING is translated to ERRDOS+ERRbadshare which
is then translated to -EBUSY. In the same way is STATUS_SHARING_VIOLATION
translated to -EBUSY.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/netmisc.c      | 2 +-
 fs/smb/client/smb2maperror.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/netmisc.c b/fs/smb/client/netmisc.c
index 9ec20601cee2..4fb265525ea4 100644
--- a/fs/smb/client/netmisc.c
+++ b/fs/smb/client/netmisc.c
@@ -302,7 +302,7 @@ static const struct {
 	ERRHRD, ERRgeneral, NT_STATUS_EA_CORRUPT_ERROR}, {
 	ERRDOS, ERRlock, NT_STATUS_FILE_LOCK_CONFLICT}, {
 	ERRDOS, ERRlock, NT_STATUS_LOCK_NOT_GRANTED}, {
-	ERRDOS, ERRbadfile, NT_STATUS_DELETE_PENDING}, {
+	ERRDOS, ERRbadshare, NT_STATUS_DELETE_PENDING}, {
 	ERRDOS, ERRunsup, NT_STATUS_CTL_FILE_NOT_SUPPORTED}, {
 	ERRHRD, ERRgeneral, NT_STATUS_UNKNOWN_REVISION}, {
 	ERRHRD, ERRgeneral, NT_STATUS_REVISION_MISMATCH}, {
diff --git a/fs/smb/client/smb2maperror.c b/fs/smb/client/smb2maperror.c
index 12c2b868789f..6d381f26c5cd 100644
--- a/fs/smb/client/smb2maperror.c
+++ b/fs/smb/client/smb2maperror.c
@@ -368,7 +368,7 @@ static const struct status_to_posix_error smb2_error_map_table[] = {
 	{STATUS_EA_CORRUPT_ERROR, -EIO, "STATUS_EA_CORRUPT_ERROR"},
 	{STATUS_FILE_LOCK_CONFLICT, -EACCES, "STATUS_FILE_LOCK_CONFLICT"},
 	{STATUS_LOCK_NOT_GRANTED, -EACCES, "STATUS_LOCK_NOT_GRANTED"},
-	{STATUS_DELETE_PENDING, -ENOENT, "STATUS_DELETE_PENDING"},
+	{STATUS_DELETE_PENDING, -EBUSY, "STATUS_DELETE_PENDING"},
 	{STATUS_CTL_FILE_NOT_SUPPORTED, -ENOSYS,
 	"STATUS_CTL_FILE_NOT_SUPPORTED"},
 	{STATUS_UNKNOWN_REVISION, -EIO, "STATUS_UNKNOWN_REVISION"},
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/35] cifs: Improve SMB2+ stat() to work also for paths in DELETE_PENDING state
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (6 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 09/35] cifs: Improve SMB1 " Pali Rohár
                   ` (27 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Paths in DELETE_PENDING state cannot be opened at all. So standard way of
querying path attributes for this case is not possible.

There is an alternative way how to query limited information about file
over SMB2+ dialects without opening file itself. It is by opening the
parent directory, querying specific child with filled search filter and
asking for attributes for that child.

Implement this fallback when standard case in smb2_query_path_info fails
with STATUS_DELETE_PENDING error and stat was asked for path which is not
top level one (because top level does not have parent directory at all).

Depends on "cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY".

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsglob.h  |   1 +
 fs/smb/client/smb2glob.h  |   1 +
 fs/smb/client/smb2inode.c | 177 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 176 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index e6830ab3a546..0ecf4988664e 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -2337,6 +2337,7 @@ struct smb2_compound_vars {
 	struct smb_rqst rqst[MAX_COMPOUND];
 	struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
 	struct kvec qi_iov;
+	struct kvec qd_iov[SMB2_QUERY_DIRECTORY_IOV_SIZE];
 	struct kvec io_iov[SMB2_IOCTL_IOV_SIZE];
 	struct kvec si_iov[SMB2_SET_INFO_IOV_SIZE];
 	struct kvec close_iov;
diff --git a/fs/smb/client/smb2glob.h b/fs/smb/client/smb2glob.h
index 224495322a05..1cb219605e75 100644
--- a/fs/smb/client/smb2glob.h
+++ b/fs/smb/client/smb2glob.h
@@ -39,6 +39,7 @@ enum smb2_compound_ops {
 	SMB2_OP_GET_REPARSE,
 	SMB2_OP_QUERY_WSL_EA,
 	SMB2_OP_OPEN_QUERY,
+	SMB2_OP_QUERY_DIRECTORY,
 };
 
 /* Used when constructing chained read requests. */
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 2a0316c514e4..460e75614ef1 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -176,6 +176,9 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 			    struct kvec *out_iov, int *out_buftype, struct dentry *dentry)
 {
 
+	bool has_cifs_mount_server_inum = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM;
+	struct smb2_query_directory_req *qd_rqst = NULL;
+	struct smb2_query_directory_rsp *qd_rsp = NULL;
 	struct smb2_create_rsp *create_rsp = NULL;
 	struct smb2_query_info_rsp *qi_rsp = NULL;
 	struct smb2_compound_vars *vars = NULL;
@@ -344,6 +347,41 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 			trace_smb3_posix_query_info_compound_enter(xid, tcon->tid,
 								   ses->Suid, full_path);
 			break;
+		case SMB2_OP_QUERY_DIRECTORY:
+			rqst[num_rqst].rq_iov = &vars->qd_iov[0];
+			rqst[num_rqst].rq_nvec = SMB2_QUERY_DIRECTORY_IOV_SIZE;
+
+			rc = SMB2_query_directory_init(xid,
+						       tcon,
+						       server,
+						       &rqst[num_rqst],
+						       cfile ?
+							cfile->fid.persistent_fid : COMPOUND_FID,
+						       cfile ?
+							cfile->fid.volatile_fid : COMPOUND_FID,
+						       0,
+						       has_cifs_mount_server_inum ?
+							SMB_FIND_FILE_ID_FULL_DIR_INFO :
+							SMB_FIND_FILE_FULL_DIRECTORY_INFO);
+			if (!rc) {
+				/*
+				 * Change the default search wildcard pattern '*'
+				 * to the requested file name stored in in_iov[i]
+				 * and request for only one single entry.
+				 */
+				qd_rqst = rqst[num_rqst].rq_iov[0].iov_base;
+				qd_rqst->Flags |= SMB2_RETURN_SINGLE_ENTRY;
+				qd_rqst->FileNameLength = cpu_to_le16(in_iov[i].iov_len);
+				rqst[num_rqst].rq_iov[1] = in_iov[i];
+			}
+			if (!rc && (!cfile || num_rqst > 1)) {
+				smb2_set_next_command(tcon, &rqst[num_rqst]);
+				smb2_set_related(&rqst[num_rqst]);
+			} else if (rc) {
+				goto finished;
+			}
+			num_rqst++;
+			break;
 		case SMB2_OP_DELETE:
 			trace_smb3_delete_enter(xid, tcon->tid, ses->Suid, full_path);
 			break;
@@ -730,6 +768,64 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 				trace_smb3_posix_query_info_compound_done(xid, tcon->tid,
 									  ses->Suid);
 			break;
+		case SMB2_OP_QUERY_DIRECTORY:
+			if (rc == 0) {
+				qd_rsp = (struct smb2_query_directory_rsp *)
+					rsp_iov[i + 1].iov_base;
+				rc = smb2_validate_iov(le16_to_cpu(qd_rsp->OutputBufferOffset),
+						       le32_to_cpu(qd_rsp->OutputBufferLength),
+						       &rsp_iov[i + 1],
+						       has_cifs_mount_server_inum ?
+							sizeof(SEARCH_ID_FULL_DIR_INFO) :
+							sizeof(FILE_FULL_DIRECTORY_INFO));
+			}
+			if (rc == 0) {
+				/*
+				 * Both SEARCH_ID_FULL_DIR_INFO and FILE_FULL_DIRECTORY_INFO
+				 * have same member offsets except the UniqueId and FileName.
+				 */
+				SEARCH_ID_FULL_DIR_INFO *si =
+					(SEARCH_ID_FULL_DIR_INFO *)qd_rsp->Buffer;
+
+				idata = in_iov[i + 1].iov_base;
+				idata->fi.CreationTime = si->CreationTime;
+				idata->fi.LastAccessTime = si->LastAccessTime;
+				idata->fi.LastWriteTime = si->LastWriteTime;
+				idata->fi.ChangeTime = si->ChangeTime;
+				idata->fi.Attributes = si->ExtFileAttributes;
+				idata->fi.AllocationSize = si->AllocationSize;
+				idata->fi.EndOfFile = si->EndOfFile;
+				idata->fi.EASize = si->EaSize;
+				idata->fi.Directory =
+					!!(le32_to_cpu(si->ExtFileAttributes) & ATTR_DIRECTORY);
+				/*
+				 * UniqueId is present only in struct SEARCH_ID_FULL_DIR_INFO.
+				 * It is not present in struct FILE_FULL_DIRECTORY_INFO.
+				 * struct SEARCH_ID_FULL_DIR_INFO was requested only when
+				 * CIFS_MOUNT_SERVER_INUM is set.
+				 */
+				if (has_cifs_mount_server_inum)
+					idata->fi.IndexNumber = si->UniqueId;
+				/*
+				 * Do not change idata->fi.NumberOfLinks to correctly
+				 * trigger the CIFS_FATTR_UNKNOWN_NLINK flag.
+				 */
+				/*
+				 * Do not change idata->fi.DeletePending as we do not know if
+				 * the entry is in the delete pending state. SMB2 QUERY_DIRECTORY
+				 * at any level does not provide this information.
+				 */
+			}
+			SMB2_query_directory_free(&rqst[num_rqst++]);
+			if (rc)
+				trace_smb3_query_dir_err(xid,
+					cfile ? cfile->fid.persistent_fid : COMPOUND_FID,
+					tcon->tid, ses->Suid, 0, 0, rc);
+			else
+				trace_smb3_query_dir_done(xid,
+					cfile ? cfile->fid.persistent_fid : COMPOUND_FID,
+					tcon->tid, ses->Suid, 0, 0);
+			break;
 		case SMB2_OP_DELETE:
 			if (rc)
 				trace_smb3_delete_err(xid, tcon->tid, ses->Suid, rc);
@@ -1090,9 +1186,9 @@ int smb2_query_path_info(const unsigned int xid,
 		break;
 	case -EREMOTE:
 		break;
-	default:
-		if (hdr->Status != STATUS_OBJECT_NAME_INVALID)
-			break;
+	}
+
+	if (hdr->Status == STATUS_OBJECT_NAME_INVALID) {
 		rc2 = cifs_inval_name_dfs_link_error(xid, tcon, cifs_sb,
 						     full_path, &islink);
 		if (rc2) {
@@ -1101,6 +1197,81 @@ int smb2_query_path_info(const unsigned int xid,
 		}
 		if (islink)
 			rc = -EREMOTE;
+	} else if (hdr->Status == STATUS_DELETE_PENDING && full_path[0]) {
+		/*
+		 * If SMB2 OPEN/CREATE fails with STATUS_DELETE_PENDING error,
+		 * it means that the path is in delete pending state and it is
+		 * not possible to open it until some other client clears delete
+		 * pending state or all other clients close all opened handles
+		 * to that path.
+		 *
+		 * There is an alternative way how to query limited information
+		 * about path which is in delete pending state still suitable
+		 * for the stat() syscall. It is by opening the parent directory,
+		 * querying specific child with filled search filer and asking
+		 * for attributes for that child.
+		 */
+
+		char *parent_path;
+		const char *basename;
+		__le16 *basename_utf16;
+		int basename_utf16_len;
+		struct cifsFileInfo *parent_cfile;
+
+		basename = strrchr(full_path, CIFS_DIR_SEP(cifs_sb));
+		if (basename) {
+			parent_path = kstrndup(full_path, basename - full_path, GFP_KERNEL);
+			basename++;
+		} else {
+			parent_path = kstrdup("", GFP_KERNEL);
+			basename = full_path;
+		}
+
+		if (!parent_path) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
+		basename_utf16 = cifs_convert_path_to_utf16(basename, cifs_sb);
+		if (!basename_utf16) {
+			kfree(parent_path);
+			rc = -ENOMEM;
+			goto out;
+		}
+
+		basename_utf16_len = 2 * UniStrnlen((wchar_t *)basename_utf16, PATH_MAX);
+
+retry_query_directory:
+		num_cmds = 1;
+		cmds[0] = SMB2_OP_QUERY_DIRECTORY;
+		in_iov[0].iov_base = basename_utf16;
+		in_iov[0].iov_len = basename_utf16_len;
+		in_iov[1].iov_base = data;
+		in_iov[1].iov_len = sizeof(*data);
+		oparms = CIFS_OPARMS(cifs_sb, tcon, parent_path, FILE_READ_DATA,
+				     FILE_OPEN, CREATE_NOT_FILE, ACL_NO_MODE);
+		cifs_get_readable_path(tcon, parent_path, &parent_cfile);
+		free_rsp_iov(out_iov, out_buftype, ARRAY_SIZE(out_iov));
+		rc = smb2_compound_op(xid, tcon, cifs_sb, parent_path,
+				      &oparms, in_iov, cmds, num_cmds,
+				      parent_cfile, out_iov, out_buftype, NULL);
+		if (rc == -EOPNOTSUPP && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
+			/*
+			 * If querying of server inode numbers is not supported
+			 * but is enabled, then disable it and try again.
+			 */
+			cifs_autodisable_serverino(cifs_sb);
+			goto retry_query_directory;
+		}
+
+		kfree(parent_path);
+		kfree(basename_utf16);
+
+		hdr = out_iov[0].iov_base;
+		if (!hdr || out_buftype[0] == CIFS_NO_BUFFER)
+			goto out;
+
+		data->fi.DeletePending = 1; /* This is code path for STATUS_DELETE_PENDING. */
 	}
 
 out:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/35] cifs: Improve SMB1 stat() to work also for paths in DELETE_PENDING state
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (7 preceding siblings ...)
  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-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 10/35] cifs: Improve detect_directory_symlink_target() function Pali Rohár
                   ` (26 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Windows NT SMB server may return -EBUSY (STATUS_DELETE_PENDING) from
CIFSSMBQPathInfo() function for files which are in DELETE_PENDING state.
When this happens, it is still possible to use CIFSFindFirst() fallback.
So allow to use CIFSFindFirst() fallback also for -EBUSY error.

This change fixes stat() to work also against Windows Server 2022 for files
in DELETE_PENDING state.

Depends on "cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY".

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb1ops.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 55ab8a5b150c..176bc2a211bf 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -583,10 +583,11 @@ static int cifs_query_path_info(const unsigned int xid,
 
 	/*
 	 * Then fallback to CIFSFindFirst() which works also with non-NT servers
-	 * but does not does not provide NumberOfLinks.
+	 * but does not does not provide NumberOfLinks. Also it works for files
+	 * in DELETE_PENDING state (CIFSSMBQPathInfo() returns -EBUSY for them).
 	 * Can be used with backup intent flag to overcome -EACCES error.
 	 */
-	if ((rc == -EOPNOTSUPP || rc == -EINVAL ||
+	if ((rc == -EOPNOTSUPP || rc == -EINVAL || rc == -EBUSY ||
 	     (backup_cred(cifs_sb) && rc == -EACCES)) &&
 	    !non_unicode_wildcard) {
 		if (!(tcon->ses->capabilities & tcon->ses->server->vals->cap_nt_find))
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/35] cifs: Improve detect_directory_symlink_target() function
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (8 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 09/35] cifs: Improve SMB1 " Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 11/35] cifs: Fix random name construction for cifs_rename_pending_delete() Pali Rohár
                   ` (25 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/35] cifs: Fix random name construction for cifs_rename_pending_delete()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (9 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 10/35] cifs: Improve detect_directory_symlink_target() function Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 12/35] cifs: Fix DELETE comments in cifs_rename_pending_delete() Pali Rohár
                   ` (24 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Currently the random name for SMB1 silly rename used in
cifs_rename_pending_delete() is generated as:

    cifs<mid>

This is not very unique and can cause conflicts. Also it does not match the
comment which says something different.

Change the way how it is generated and use the algorithm from nfs client
and construct random name as:

    .smb<inodenum><counter>

Where counter is global counter incremented for each request and generated
name is checked if it really does not exist before it is used. NFS client
uses same pattern but instead of ".smb" it has ".nfs".

Move random name construction code from CIFSSMBRenameOpenFile() function to
cifs_rename_pending_delete() function as it is the place where silly rename
is implemented.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c | 12 +-----------
 fs/smb/client/inode.c   | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 0d773860fd6c..8d9f6f28c17e 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -2320,7 +2320,6 @@ int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *pTcon,
 	struct smb_com_transaction2_sfi_rsp *pSMBr = NULL;
 	struct set_file_rename *rename_info;
 	char *data_offset;
-	char dummy_string[30];
 	int rc = 0;
 	int bytes_returned = 0;
 	int len_of_str;
@@ -2354,21 +2353,12 @@ int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *pTcon,
 	pSMB->TotalParameterCount = pSMB->ParameterCount;
 	pSMB->ParameterOffset = cpu_to_le16(param_offset);
 	pSMB->DataOffset = cpu_to_le16(offset);
-	/* construct random name ".cifs_tmp<inodenum><mid>" */
 	rename_info->overwrite = cpu_to_le32(1);
 	rename_info->root_fid  = 0;
 	/* unicode only call */
-	if (target_name == NULL) {
-		sprintf(dummy_string, "cifs%x", pSMB->hdr.Mid);
-		len_of_str =
-			cifsConvertToUTF16((__le16 *)rename_info->target_name,
-					dummy_string, 24, nls_codepage, remap);
-	} else {
-		len_of_str =
-			cifsConvertToUTF16((__le16 *)rename_info->target_name,
+	len_of_str = cifsConvertToUTF16((__le16 *)rename_info->target_name,
 					target_name, PATH_MAX, nls_codepage,
 					remap);
-	}
 	rename_info->target_name_len = cpu_to_le32(2 * len_of_str);
 	count = sizeof(struct set_file_rename) + (2 * len_of_str);
 	byte_count += count;
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index cd06598eacbd..a37dfd50f33d 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -6,6 +6,7 @@
  *
  */
 #include <linux/fs.h>
+#include <linux/namei.h>
 #include <linux/stat.h>
 #include <linux/slab.h>
 #include <linux/pagemap.h>
@@ -1689,6 +1690,13 @@ cifs_set_file_info(struct inode *inode, struct iattr *attrs, unsigned int xid,
  * and rename it to a random name that hopefully won't conflict with
  * anything else.
  */
+#define SILLYNAME_PREFIX ".smb"
+#define SILLYNAME_PREFIX_LEN ((unsigned int)sizeof(SILLYNAME_PREFIX) - 1)
+#define SILLYNAME_FILEID_LEN ((unsigned int)sizeof(u64) << 1)
+#define SILLYNAME_COUNTER_LEN ((unsigned int)sizeof(unsigned int) << 1)
+#define SILLYNAME_LEN (SILLYNAME_PREFIX_LEN + \
+		SILLYNAME_FILEID_LEN + \
+		SILLYNAME_COUNTER_LEN)
 int
 cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 			   const unsigned int xid)
@@ -1704,12 +1712,40 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	struct cifs_tcon *tcon;
 	__u32 dosattr, origattr;
 	FILE_BASIC_INFO *info_buf = NULL;
+	unsigned char sillyname[SILLYNAME_LEN + 1];
+	int sillyname_len;
 
 	tlink = cifs_sb_tlink(cifs_sb);
 	if (IS_ERR(tlink))
 		return PTR_ERR(tlink);
 	tcon = tlink_tcon(tlink);
 
+	/* construct random name ".smb<inodenum><counter>" */
+	while (true) {
+		static unsigned int sillycounter; /* globally unique */
+		bool sillyname_available = false;
+		struct dentry *sdentry = NULL;
+
+		sillycounter++;
+		sillyname_len = scnprintf(sillyname, sizeof(sillyname),
+				 SILLYNAME_PREFIX "%0*llx%0*x",
+				 SILLYNAME_FILEID_LEN, (unsigned long long)cifsInode->uniqueid,
+				 SILLYNAME_COUNTER_LEN, sillycounter);
+		sdentry = lookup_noperm(&QSTR(sillyname), dentry->d_parent);
+		if (IS_ERR(sdentry)) {
+			rc = -EBUSY;
+			goto out;
+		}
+
+		if (d_inode(sdentry) == NULL) /* need negative lookup */
+			sillyname_available = true;
+
+		dput(sdentry);
+
+		if (sillyname_available)
+			break;
+	}
+
 	/*
 	 * We cannot rename the file if the server doesn't support
 	 * CAP_INFOLEVEL_PASSTHRU
@@ -1761,7 +1797,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	}
 
 	/* rename the file */
-	rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, NULL,
+	rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
 				   cifs_sb->local_nls,
 				   cifs_remap(cifs_sb));
 	if (rc != 0) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/35] cifs: Fix DELETE comments in cifs_rename_pending_delete()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (10 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 13/35] cifs: Avoid dynamic memory allocation of FILE_BASIC_INFO buffer " Pali Rohár
                   ` (23 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

DELETE_ON_CLOSE is a flag which is being set during the opening a file and
DELETE_PENDING flag is being set by SetFileDisposition() on the already
opened file. Those two flags are different and have different meaning.

There in the function cifs_rename_pending_delete() is being set
DELETE_PENDING flag on the already opened file.

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index a37dfd50f33d..88d1d657cfb0 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1686,7 +1686,7 @@ cifs_set_file_info(struct inode *inode, struct iattr *attrs, unsigned int xid,
 
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 /*
- * Open the given file (if it isn't already), set the DELETE_ON_CLOSE bit
+ * Open the given file (if it isn't already), set the DELETE_PENDING bit
  * and rename it to a random name that hopefully won't conflict with
  * anything else.
  */
@@ -1805,7 +1805,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		goto undo_setattr;
 	}
 
-	/* try to set DELETE_ON_CLOSE */
+	/* try to set DELETE_PENDING */
 	if (!test_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags)) {
 		rc = CIFSSMBSetFileDisposition(xid, tcon, true, fid.netfid,
 					       current->tgid);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/35] cifs: Avoid dynamic memory allocation of FILE_BASIC_INFO buffer in cifs_rename_pending_delete()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (11 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 14/35] cifs: Extend CIFSSMBRenameOpenFile() function for overwrite parameter Pali Rohár
                   ` (22 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Put FILE_BASIC_INFO buffer on the stack as it is not too large.
This simplify error handling in cifs_rename_pending_delete().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 88d1d657cfb0..2889fa6625af 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1711,7 +1711,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	struct tcon_link *tlink;
 	struct cifs_tcon *tcon;
 	__u32 dosattr, origattr;
-	FILE_BASIC_INFO *info_buf = NULL;
+	FILE_BASIC_INFO info_buf = {};
 	unsigned char sillyname[SILLYNAME_LEN + 1];
 	int sillyname_len;
 
@@ -1780,13 +1780,8 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 
 	/* set ATTR_HIDDEN and clear ATTR_READONLY, but only if needed */
 	if (dosattr != origattr) {
-		info_buf = kzalloc(sizeof(*info_buf), GFP_KERNEL);
-		if (info_buf == NULL) {
-			rc = -ENOMEM;
-			goto out_close;
-		}
-		info_buf->Attributes = cpu_to_le32(dosattr);
-		rc = CIFSSMBSetFileInfo(xid, tcon, info_buf, fid.netfid,
+		info_buf.Attributes = cpu_to_le32(dosattr);
+		rc = CIFSSMBSetFileInfo(xid, tcon, &info_buf, fid.netfid,
 					current->tgid);
 		/* although we would like to mark the file hidden
  		   if that fails we will still try to rename it */
@@ -1829,7 +1824,6 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 out_close:
 	CIFSSMBClose(xid, tcon, fid.netfid);
 out:
-	kfree(info_buf);
 	cifs_put_tlink(tlink);
 	return rc;
 
@@ -1843,8 +1837,8 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 				cifs_sb->local_nls, cifs_remap(cifs_sb));
 undo_setattr:
 	if (dosattr != origattr) {
-		info_buf->Attributes = cpu_to_le32(origattr);
-		if (!CIFSSMBSetFileInfo(xid, tcon, info_buf, fid.netfid,
+		info_buf.Attributes = cpu_to_le32(origattr);
+		if (!CIFSSMBSetFileInfo(xid, tcon, &info_buf, fid.netfid,
 					current->tgid))
 			cifsInode->cifsAttrs = origattr;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/35] cifs: Extend CIFSSMBRenameOpenFile() function for overwrite parameter
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (12 preceding siblings ...)
  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 ` 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
                   ` (21 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Currently CIFSSMBRenameOpenFile() function always overwrite the target
location. This new overwrite parameter allows to specify if the rename
should fail when the target location exists. This new parameter would be
used in follow up changes.

No functional change.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsproto.h | 2 +-
 fs/smb/client/cifssmb.c   | 4 ++--
 fs/smb/client/inode.c     | 3 +++
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index f467b24fd984..f248b18f1cf3 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -470,7 +470,7 @@ int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon,
 		  const char *from_name, const char *to_name,
 		  struct cifs_sb_info *cifs_sb);
 extern int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *tcon,
-				 int netfid, const char *target_name,
+				 int netfid, const char *target_name, bool overwrite,
 				 const struct nls_table *nls_codepage,
 				 int remap_special_chars);
 int CIFSCreateHardLink(const unsigned int xid,
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 8d9f6f28c17e..f12bc0f4d0c1 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -2313,7 +2313,7 @@ int CIFSSMBRename(const unsigned int xid, struct cifs_tcon *tcon,
 }
 
 int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *pTcon,
-		int netfid, const char *target_name,
+		int netfid, const char *target_name, bool overwrite,
 		const struct nls_table *nls_codepage, int remap)
 {
 	struct smb_com_transaction2_sfi_req *pSMB  = NULL;
@@ -2353,7 +2353,7 @@ int CIFSSMBRenameOpenFile(const unsigned int xid, struct cifs_tcon *pTcon,
 	pSMB->TotalParameterCount = pSMB->ParameterCount;
 	pSMB->ParameterOffset = cpu_to_le16(param_offset);
 	pSMB->DataOffset = cpu_to_le16(offset);
-	rename_info->overwrite = cpu_to_le32(1);
+	rename_info->overwrite = cpu_to_le32(overwrite);
 	rename_info->root_fid  = 0;
 	/* unicode only call */
 	len_of_str = cifsConvertToUTF16((__le16 *)rename_info->target_name,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 2889fa6625af..be8e5e5ca6cd 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1793,6 +1793,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 
 	/* rename the file */
 	rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
+				   true /* overwrite */,
 				   cifs_sb->local_nls,
 				   cifs_remap(cifs_sb));
 	if (rc != 0) {
@@ -1834,6 +1835,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	 */
 undo_rename:
 	CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, dentry->d_name.name,
+				true /* overwrite */,
 				cifs_sb->local_nls, cifs_remap(cifs_sb));
 undo_setattr:
 	if (dosattr != origattr) {
@@ -2374,6 +2376,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry,
 	if (rc == 0) {
 		rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid,
 				(const char *) to_dentry->d_name.name,
+				true /* overwrite */,
 				cifs_sb->local_nls, cifs_remap(cifs_sb));
 		CIFSSMBClose(xid, tcon, fid.netfid);
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/35] cifs: Do not try to overwrite existing sillyname in cifs_rename_pending_delete()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (13 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 14/35] cifs: Extend CIFSSMBRenameOpenFile() function for overwrite parameter Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 16/35] cifs: Add comments for DeletePending assignments in open functions Pali Rohár
                   ` (20 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

If the target location for newly generated sillyname file already exists
then do not try to rename file to that new target name. It would either
fail (because file is still in use) or it will unexpectedly remove
additional file which was not requested.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index be8e5e5ca6cd..89d1b82ac55c 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1793,7 +1793,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 
 	/* rename the file */
 	rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
-				   true /* overwrite */,
+				   false /* overwrite */,
 				   cifs_sb->local_nls,
 				   cifs_remap(cifs_sb));
 	if (rc != 0) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/35] cifs: Add comments for DeletePending assignments in open functions
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (14 preceding siblings ...)
  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 ` 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
                   ` (19 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

On more places is set DeletePending member to 0. Add comments why is 0 the
correct value. Paths in DELETE_PENDING state cannot be opened by new calls.
So if the newly issued open for that path succeed then it means that the
path cannot be in DELETE_PENDING state.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c   | 4 ++--
 fs/smb/client/smb2inode.c | 2 +-
 fs/smb/client/smb2pdu.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index f12bc0f4d0c1..6dabff82f6ae 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1163,7 +1163,7 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 				cpu_to_le64(le32_to_cpu(pSMBr->EndOfFile));
 			pfile_info->EndOfFile = pfile_info->AllocationSize;
 			pfile_info->NumberOfLinks = cpu_to_le32(1);
-			pfile_info->DeletePending = 0;
+			pfile_info->DeletePending = 0; /* successful open = not delete pending */
 		}
 	}
 
@@ -1288,7 +1288,7 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
 		buf->AllocationSize = rsp->AllocationSize;
 		buf->EndOfFile = rsp->EndOfFile;
 		buf->NumberOfLinks = cpu_to_le32(1);
-		buf->DeletePending = 0;
+		buf->DeletePending = 0; /* successful open = not delete pending */
 	}
 
 	cifs_buf_release(req);
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 460e75614ef1..6c9da150a402 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -698,7 +698,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 		idata->fi.EndOfFile = create_rsp->EndofFile;
 		if (le32_to_cpu(idata->fi.NumberOfLinks) == 0)
 			idata->fi.NumberOfLinks = cpu_to_le32(1); /* dummy value */
-		idata->fi.DeletePending = 0;
+		idata->fi.DeletePending = 0; /* successful open = not delete pending */
 		idata->fi.Directory = !!(le32_to_cpu(create_rsp->FileAttributes) & ATTR_DIRECTORY);
 
 		/* smb2_parse_contexts() fills idata->fi.IndexNumber */
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 2df93a75e3b8..58800490142e 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3277,7 +3277,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
 		buf->EndOfFile = rsp->EndofFile;
 		buf->Attributes = rsp->FileAttributes;
 		buf->NumberOfLinks = cpu_to_le32(1);
-		buf->DeletePending = 0;
+		buf->DeletePending = 0; /* successful open = not delete pending */
 	}
 
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 17/35] cifs: Use NT_STATUS_DELETE_PENDING for filling fi.DeletePending in cifs_query_path_info()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (15 preceding siblings ...)
  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 ` 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
                   ` (18 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Function CIFSSMBQPathInfo() returns NT_STATUS_DELETE_PENDING status code
when the path is in the delete pending state. So use this information when
filling the fi.DeletePending member in cifs_query_path_info() function.

Depends on "cifs: Change translation of STATUS_DELETE_PENDING to -EBUSY".

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c | 6 ++++++
 fs/smb/client/smb1ops.c | 6 +++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 6dabff82f6ae..2427752bc224 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -35,6 +35,7 @@
 #include "cifs_debug.h"
 #include "fscache.h"
 #include "smbdirect.h"
+#include "nterr.h"
 #ifdef CONFIG_CIFS_DFS_UPCALL
 #include "dfs_cache.h"
 #endif
@@ -3902,6 +3903,11 @@ CIFSSMBQPathInfo(const unsigned int xid, struct cifs_tcon *tcon,
 			 (struct smb_hdr *) pSMBr, &bytes_returned, 0);
 	if (rc) {
 		cifs_dbg(FYI, "Send error in QPathInfo = %d\n", rc);
+		/* Fill at least the DeletePending for -EBUSY error code */
+		if (rc == -EBUSY && data)
+			data->DeletePending =
+			  (pSMBr->hdr.Flags2 & SMBFLG2_ERR_STATUS) &&
+			  pSMBr->hdr.Status.CifsError == cpu_to_le32(NT_STATUS_DELETE_PENDING);
 	} else {		/* decode response */
 		rc = validate_t2((struct smb_t2_rsp *)pSMBr);
 
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 176bc2a211bf..618470db6444 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -628,7 +628,11 @@ static int cifs_query_path_info(const unsigned int xid,
 				fi.EASize = di->EaSize;
 			}
 			fi.NumberOfLinks = cpu_to_le32(1);
-			fi.DeletePending = 0;
+			/*
+			 * Do not change fi.DeletePending as it is set by the above
+			 * CIFSSMBQPathInfo() call even on error. By default it is
+			 * initialized to zero (false).
+			 */
 			fi.Directory = !!(le32_to_cpu(fi.Attributes) & ATTR_DIRECTORY);
 			cifs_buf_release(search_info.ntwrk_buf_start);
 		} else if (!full_path[0]) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 18/35] cifs: Do not set NumberOfLinks to 1 from open or query calls
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (16 preceding siblings ...)
  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 ` 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
                   ` (17 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Current code already interprets zero value as unknown number of links and
sets the CIFS_FATTR_UNKNOWN_NLINK flag to handle this situation.

Setting NumberOfLinks to 1 prevents the CIFS_FATTR_UNKNOWN_NLINK indicator.
So set the NumberOfLinks to 0 when it is unknown.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c   | 4 ++--
 fs/smb/client/smb1ops.c   | 2 +-
 fs/smb/client/smb2inode.c | 6 ++++--
 fs/smb/client/smb2pdu.c   | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 2427752bc224..2a83fbc65395 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1163,7 +1163,7 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 			pfile_info->AllocationSize =
 				cpu_to_le64(le32_to_cpu(pSMBr->EndOfFile));
 			pfile_info->EndOfFile = pfile_info->AllocationSize;
-			pfile_info->NumberOfLinks = cpu_to_le32(1);
+			pfile_info->NumberOfLinks = cpu_to_le32(0); /* CIFS_FATTR_UNKNOWN_NLINK */
 			pfile_info->DeletePending = 0; /* successful open = not delete pending */
 		}
 	}
@@ -1288,7 +1288,7 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
 		/* the file_info buf is endian converted by caller */
 		buf->AllocationSize = rsp->AllocationSize;
 		buf->EndOfFile = rsp->EndOfFile;
-		buf->NumberOfLinks = cpu_to_le32(1);
+		buf->NumberOfLinks = cpu_to_le32(0); /* trigger CIFS_FATTR_UNKNOWN_NLINK */
 		buf->DeletePending = 0; /* successful open = not delete pending */
 	}
 
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 618470db6444..6e928e90d72b 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -627,7 +627,7 @@ static int cifs_query_path_info(const unsigned int xid,
 				fi.EndOfFile = di->EndOfFile;
 				fi.EASize = di->EaSize;
 			}
-			fi.NumberOfLinks = cpu_to_le32(1);
+			fi.NumberOfLinks = cpu_to_le32(0); /* trigger CIFS_FATTR_UNKNOWN_NLINK */
 			/*
 			 * Do not change fi.DeletePending as it is set by the above
 			 * CIFSSMBQPathInfo() call even on error. By default it is
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 6c9da150a402..c8b0e9b2438f 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -696,8 +696,10 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 		idata->fi.Attributes = create_rsp->FileAttributes;
 		idata->fi.AllocationSize = create_rsp->AllocationSize;
 		idata->fi.EndOfFile = create_rsp->EndofFile;
-		if (le32_to_cpu(idata->fi.NumberOfLinks) == 0)
-			idata->fi.NumberOfLinks = cpu_to_le32(1); /* dummy value */
+		/*
+		 * Do not change idata->fi.NumberOfLinks to correctly
+		 * trigger the CIFS_FATTR_UNKNOWN_NLINK flag.
+		 */
 		idata->fi.DeletePending = 0; /* successful open = not delete pending */
 		idata->fi.Directory = !!(le32_to_cpu(create_rsp->FileAttributes) & ATTR_DIRECTORY);
 
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 58800490142e..bf588ec99618 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3276,7 +3276,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
 		buf->AllocationSize = rsp->AllocationSize;
 		buf->EndOfFile = rsp->EndofFile;
 		buf->Attributes = rsp->FileAttributes;
-		buf->NumberOfLinks = cpu_to_le32(1);
+		buf->NumberOfLinks = cpu_to_le32(0); /* trigger CIFS_FATTR_UNKNOWN_NLINK */
 		buf->DeletePending = 0; /* successful open = not delete pending */
 	}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 19/35] cifs: Fix cifs_rename_pending_delete() for files with more hardlinks
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (17 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 20/35] cifs: Fix permission logic in cifs_rename_pending_delete() Pali Rohár
                   ` (16 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Do not set ATTR_HIDDEN when the file has more links. These attributes are
shared across all links, they are not direntry specific. So changing
attribute on one hardlink affects all other hardlinks.

The cifs_rename_pending_delete() function called from unlink() should
affect only the path passed to unlink(), and not other links.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 89d1b82ac55c..4658af632098 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1769,16 +1769,26 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	if (rc != 0)
 		goto out;
 
-	origattr = cifsInode->cifsAttrs;
-	if (origattr == 0)
-		origattr |= ATTR_NORMAL;
+	origattr = cifsInode->cifsAttrs & ~ATTR_NORMAL;
 
+	/* clear ATTR_READONLY, needed for opening file with DELETE access */
 	dosattr = origattr & ~ATTR_READONLY;
+
+	/*
+	 * Set ATTR_HIDDEN to hide the file, but only if this is not a hardlink
+	 * because all hardlinked directory entries shares same attribues and
+	 * we do not want to mark all hardlinked entries as hidden.
+	 */
+	if (inode->i_nlink <= 1)
+		dosattr |= ATTR_HIDDEN;
+
+	/* clearing all attributes is done via ATTR_NORMAL value */
+	if (origattr == 0)
+		origattr |= ATTR_NORMAL;
 	if (dosattr == 0)
 		dosattr |= ATTR_NORMAL;
-	dosattr |= ATTR_HIDDEN;
 
-	/* set ATTR_HIDDEN and clear ATTR_READONLY, but only if needed */
+	/* change dosattr, but only if needed */
 	if (dosattr != origattr) {
 		info_buf.Attributes = cpu_to_le32(dosattr);
 		rc = CIFSSMBSetFileInfo(xid, tcon, &info_buf, fid.netfid,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 20/35] cifs: Fix permission logic in cifs_rename_pending_delete()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (18 preceding siblings ...)
  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 ` 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
                   ` (15 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Opening file with DELETE access does not have to be allowed by server for
file with ATTR_READONLY attribute set. So the current logic for clearing
the ATTR_READONLY attribute in cifs_rename_pending_delete() can fail as it
tries to open file with access mask DELETE | FILE_WRITE_ATTRIBUTES.

Fix the permission logic. First clear the ATTR_READONLY attribute and then
the open file with DELETE access. As for the RENAME and SET_DELETE_PENDING
operations is not required the FILE_WRITE_ATTRIBUTES access, do not pass it
into open. This allows to call cifs_rename_pending_delete() also on the
files with ACL permissions which disallows file modification.

For changing attributes use the set_file_info() callback function which
already handles the situation when the caller wants to clear the
ATTR_READONLY flag. Note that the set_file_info() callback also updates the
cifsInode->cifsAttrs member, so remove explicit assignment here.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 48 ++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 4658af632098..63ab233517f2 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1755,20 +1755,6 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		goto out;
 	}
 
-	oparms = (struct cifs_open_parms) {
-		.tcon = tcon,
-		.cifs_sb = cifs_sb,
-		.desired_access = DELETE | FILE_WRITE_ATTRIBUTES,
-		.create_options = cifs_create_options(cifs_sb, CREATE_NOT_DIR),
-		.disposition = FILE_OPEN,
-		.path = full_path,
-		.fid = &fid,
-	};
-
-	rc = CIFS_open(xid, &oparms, &oplock, NULL);
-	if (rc != 0)
-		goto out;
-
 	origattr = cifsInode->cifsAttrs & ~ATTR_NORMAL;
 
 	/* clear ATTR_READONLY, needed for opening file with DELETE access */
@@ -1791,16 +1777,27 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	/* change dosattr, but only if needed */
 	if (dosattr != origattr) {
 		info_buf.Attributes = cpu_to_le32(dosattr);
-		rc = CIFSSMBSetFileInfo(xid, tcon, &info_buf, fid.netfid,
-					current->tgid);
+		rc = tcon->ses->server->ops->set_file_info(inode, full_path, &info_buf, xid);
 		/* although we would like to mark the file hidden
  		   if that fails we will still try to rename it */
-		if (!rc)
-			cifsInode->cifsAttrs = dosattr;
-		else
+		if (rc)
 			dosattr = origattr; /* since not able to change them */
 	}
 
+	oparms = (struct cifs_open_parms) {
+		.tcon = tcon,
+		.cifs_sb = cifs_sb,
+		.desired_access = DELETE,
+		.create_options = cifs_create_options(cifs_sb, CREATE_NOT_DIR),
+		.disposition = FILE_OPEN,
+		.path = full_path,
+		.fid = &fid,
+	};
+
+	rc = CIFS_open(xid, &oparms, &oplock, NULL);
+	if (rc != 0)
+		goto undo_setattr;
+
 	/* rename the file */
 	rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
 				   false /* overwrite */,
@@ -1808,7 +1805,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 				   cifs_remap(cifs_sb));
 	if (rc != 0) {
 		rc = -EBUSY;
-		goto undo_setattr;
+		goto undo_close;
 	}
 
 	/* try to set DELETE_PENDING */
@@ -1832,8 +1829,8 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		set_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags);
 	}
 
-out_close:
 	CIFSSMBClose(xid, tcon, fid.netfid);
+
 out:
 	cifs_put_tlink(tlink);
 	return rc;
@@ -1847,15 +1844,14 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, dentry->d_name.name,
 				true /* overwrite */,
 				cifs_sb->local_nls, cifs_remap(cifs_sb));
+undo_close:
+	CIFSSMBClose(xid, tcon, fid.netfid);
 undo_setattr:
 	if (dosattr != origattr) {
 		info_buf.Attributes = cpu_to_le32(origattr);
-		if (!CIFSSMBSetFileInfo(xid, tcon, &info_buf, fid.netfid,
-					current->tgid))
-			cifsInode->cifsAttrs = origattr;
+		tcon->ses->server->ops->set_file_info(inode, full_path, &info_buf, xid);
 	}
-
-	goto out_close;
+	goto out;
 }
 #endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 21/35] cifs: Propagate error code from CIFSSMBSetFileInfo() to cifs_rename_pending_delete()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (19 preceding siblings ...)
  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 ` 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
                   ` (14 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Function CIFSSMBSetFileInfo() setting the file to the delete pending state,
the core part of the cifs_rename_pending_delete() functionality.

So do not mask error code from CIFSSMBSetFileInfo() function and properly
propagate it to the caller of cifs_rename_pending_delete().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 63ab233517f2..b0526787138d 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1822,10 +1822,8 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		 */
 		if (rc == -ENOENT)
 			rc = 0;
-		else if (rc != 0) {
-			rc = -EBUSY;
+		else if (rc != 0)
 			goto undo_rename;
-		}
 		set_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags);
 	}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 22/35] cifs: Improve cifs_rename_pending_delete() to work without the PASSTHRU support
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (20 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 23/35] cifs: Fix SMBLegacyOpen() function Pali Rohár
                   ` (13 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

If PASSTHRU support is not supported by SMB server (which is currently
required for renaming the opened file) then rename the file via path before
opening it. This does not require PASSTHRU support on SMB server.

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

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index b0526787138d..9a5504a3406d 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1711,9 +1711,13 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	struct tcon_link *tlink;
 	struct cifs_tcon *tcon;
 	__u32 dosattr, origattr;
+	char *sillyname_full_path = NULL;
+	bool can_rename_opened_file = true;
 	FILE_BASIC_INFO info_buf = {};
 	unsigned char sillyname[SILLYNAME_LEN + 1];
 	int sillyname_len;
+	const char *dirpath_end;
+	size_t dirpath_len;
 
 	tlink = cifs_sb_tlink(cifs_sb);
 	if (IS_ERR(tlink))
@@ -1747,13 +1751,26 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	}
 
 	/*
-	 * We cannot rename the file if the server doesn't support
-	 * CAP_INFOLEVEL_PASSTHRU
+	 * We cannot rename the opened file if the SMB1 server doesn't
+	 * support CAP_INFOLEVEL_PASSTHRU. But we can rename file via path.
 	 */
-	if (!(tcon->ses->capabilities & CAP_INFOLEVEL_PASSTHRU)) {
-		rc = -EBUSY;
+	if (!(tcon->ses->capabilities & CAP_INFOLEVEL_PASSTHRU))
+		can_rename_opened_file = false;
+
+	dirpath_end = strrchr(full_path, CIFS_DIR_SEP(cifs_sb));
+	if (!dirpath_end) {
+		rc = -EINVAL;
+		goto out;
+	}
+	dirpath_len = dirpath_end + 1 - full_path;
+
+	sillyname_full_path = kzalloc(dirpath_len + sillyname_len + 1, GFP_KERNEL);
+	if (!sillyname_full_path) {
+		rc = -ENOMEM;
 		goto out;
 	}
+	memcpy(sillyname_full_path, full_path, dirpath_len);
+	memcpy(sillyname_full_path + dirpath_len, sillyname, sillyname_len);
 
 	origattr = cifsInode->cifsAttrs & ~ATTR_NORMAL;
 
@@ -1784,6 +1801,16 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 			dosattr = origattr; /* since not able to change them */
 	}
 
+	/* rename the file path (before open if server does not support renaming opened file) */
+	if (!can_rename_opened_file) {
+		rc = tcon->ses->server->ops->rename(xid, tcon, dentry, full_path,
+						    sillyname_full_path, cifs_sb);
+		if (rc != 0) {
+			rc = -EBUSY;
+			goto undo_setattr;
+		}
+	}
+
 	oparms = (struct cifs_open_parms) {
 		.tcon = tcon,
 		.cifs_sb = cifs_sb,
@@ -1796,16 +1823,18 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 
 	rc = CIFS_open(xid, &oparms, &oplock, NULL);
 	if (rc != 0)
-		goto undo_setattr;
+		goto undo_rename_path;
 
-	/* rename the file */
-	rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
+	/* rename the opened file (if it was not already renamed before the open) */
+	if (can_rename_opened_file) {
+		rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
 				   false /* overwrite */,
 				   cifs_sb->local_nls,
 				   cifs_remap(cifs_sb));
-	if (rc != 0) {
-		rc = -EBUSY;
-		goto undo_close;
+		if (rc != 0) {
+			rc = -EBUSY;
+			goto undo_close;
+		}
 	}
 
 	/* try to set DELETE_PENDING */
@@ -1823,13 +1852,14 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		if (rc == -ENOENT)
 			rc = 0;
 		else if (rc != 0)
-			goto undo_rename;
+			goto undo_rename_opened_file;
 		set_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags);
 	}
 
 	CIFSSMBClose(xid, tcon, fid.netfid);
 
 out:
+	kfree(sillyname_full_path);
 	cifs_put_tlink(tlink);
 	return rc;
 
@@ -1838,12 +1868,19 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	 * dealing with errors here since we can't do anything about
 	 * them anyway.
 	 */
-undo_rename:
-	CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, dentry->d_name.name,
+undo_rename_opened_file:
+	if (can_rename_opened_file)
+		CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, dentry->d_name.name,
 				true /* overwrite */,
 				cifs_sb->local_nls, cifs_remap(cifs_sb));
 undo_close:
 	CIFSSMBClose(xid, tcon, fid.netfid);
+undo_rename_path:
+	if (!can_rename_opened_file)
+		CIFSSMBRename(xid, tcon, dentry,
+				sillyname_full_path,
+				full_path,
+				cifs_sb);
 undo_setattr:
 	if (dosattr != origattr) {
 		info_buf.Attributes = cpu_to_le32(origattr);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 23/35] cifs: Fix SMBLegacyOpen() function
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (21 preceding siblings ...)
  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 ` 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
                   ` (12 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Function SMBLegacyOpen() contains many FIXME comments, commented code and
missing handling of different flags.

Fix the FIXME comments, add missing handling of different flags and use
named macros as constants instead of magic numbers.

This is done according to MS-CIFS spec.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifspdu.h |  4 +++
 fs/smb/client/cifssmb.c | 73 +++++++++++++++++++++++++++--------------
 fs/smb/client/smb1ops.c |  4 ++-
 3 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index d9cf7db0ac35..cb1f99d22dd7 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -137,6 +137,10 @@
  * Flags on SMB open
  */
 #define SMBOPEN_WRITE_THROUGH 0x4000
+#define SMBOPEN_DO_NOT_CACHE  0x1000
+#define SMBOPEN_RANDOM_ACCESS 0x0200
+#define SMBOPEN_SEQUENTIAL    0x0100
+#define SMBOPEN_DENY_COMPAT   0x0000
 #define SMBOPEN_DENY_ALL      0x0010
 #define SMBOPEN_DENY_WRITE    0x0020
 #define SMBOPEN_DENY_READ     0x0030
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 2a83fbc65395..c09713ebdc7c 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1103,32 +1103,41 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 		count = 0;      /* no pad */
 		name_len = copy_path_name(pSMB->fileName, fileName);
 	}
-	if (*pOplock & REQ_OPLOCK)
-		pSMB->OpenFlags = cpu_to_le16(REQ_OPLOCK);
-	else if (*pOplock & REQ_BATCHOPLOCK)
+
+	if (*pOplock & REQ_BATCHOPLOCK)
 		pSMB->OpenFlags = cpu_to_le16(REQ_BATCHOPLOCK);
+	else if (*pOplock & REQ_OPLOCK)
+		pSMB->OpenFlags = cpu_to_le16(REQ_OPLOCK);
+
+	if (pfile_info)
+		pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
 
-	pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
 	pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
-	pSMB->Mode |= cpu_to_le16(0x40); /* deny none */
+	pSMB->Mode |= cpu_to_le16(SMBOPEN_DENY_NONE);
+
+	if (create_options & CREATE_WRITE_THROUGH)
+		pSMB->Mode |= cpu_to_le16(SMBOPEN_WRITE_THROUGH);
+
+	if (create_options & CREATE_NO_BUFFER)
+		pSMB->Mode |= cpu_to_le16(SMBOPEN_DO_NOT_CACHE);
+
+	if (create_options & CREATE_RANDOM_ACCESS)
+		pSMB->Mode |= cpu_to_le16(SMBOPEN_RANDOM_ACCESS);
+	else if (create_options & CREATE_SEQUENTIAL)
+		pSMB->Mode |= cpu_to_le16(SMBOPEN_SEQUENTIAL);
+
 	/* set file as system file if special file such as fifo,
 	 * socket, char or block and server expecting SFU style and
 	   no Unix extensions */
-
 	if (create_options & CREATE_OPTION_SPECIAL)
 		pSMB->FileAttributes = cpu_to_le16(ATTR_SYSTEM);
-	else /* BB FIXME BB */
-		pSMB->FileAttributes = cpu_to_le16(0/*ATTR_NORMAL*/);
+	else
+		pSMB->FileAttributes = cpu_to_le16(0);
 
 	if (create_options & CREATE_OPTION_READONLY)
 		pSMB->FileAttributes |= cpu_to_le16(ATTR_READONLY);
 
-	/* BB FIXME BB */
-/*	pSMB->CreateOptions = cpu_to_le32(create_options &
-						 CREATE_OPTIONS_MASK); */
-	/* BB FIXME END BB */
-
-	pSMB->Sattr = cpu_to_le16(ATTR_HIDDEN | ATTR_SYSTEM | ATTR_DIRECTORY);
+	pSMB->Sattr = cpu_to_le16(ATTR_READONLY | ATTR_HIDDEN | ATTR_SYSTEM | ATTR_ARCHIVE);
 	pSMB->OpenFunction = cpu_to_le16(convert_disposition(openDisposition));
 	count += name_len;
 	inc_rfc1001_len(pSMB, count);
@@ -1139,24 +1148,38 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 	cifs_stats_inc(&tcon->stats.cifs_stats.num_opens);
 	if (rc) {
 		cifs_dbg(FYI, "Error in Open = %d\n", rc);
+	} else if (pSMBr->hdr.WordCount != 15) {
+		rc = -EIO;
 	} else {
-	/* BB verify if wct == 15 */
-
-/*		*pOplock = pSMBr->OplockLevel; */ /* BB take from action field*/
+		if (!(pSMBr->Action & 0x8000))
+			*pOplock = OPLOCK_NONE;
+		else if (*pOplock & REQ_BATCHOPLOCK)
+			*pOplock = OPLOCK_BATCH;
+		else if (*pOplock & REQ_OPLOCK)
+			*pOplock = OPLOCK_EXCLUSIVE;
+		else
+			*pOplock = OPLOCK_NONE;
 
 		*netfid = pSMBr->Fid;   /* cifs fid stays in le */
+
 		/* Let caller know file was created so we can set the mode. */
 		/* Do we care about the CreateAction in any other cases? */
-	/* BB FIXME BB */
-/*		if (cpu_to_le32(FILE_CREATE) == pSMBr->CreateAction)
-			*pOplock |= CIFS_CREATE_ACTION; */
-	/* BB FIXME END */
+		if ((pSMBr->Action & 0x0003) == 2)
+			*pOplock |= CIFS_CREATE_ACTION;
 
 		if (pfile_info) {
-			pfile_info->CreationTime = 0; /* BB convert CreateTime*/
-			pfile_info->LastAccessTime = 0; /* BB fixme */
-			pfile_info->LastWriteTime = 0; /* BB fixme */
-			pfile_info->ChangeTime = 0;  /* BB fixme */
+			struct timespec64 ts;
+			__u32 time = le32_to_cpu(pSMBr->LastWriteTime);
+
+			ts.tv_nsec = 0;
+			ts.tv_sec = time;
+			pfile_info->LastWriteTime = cpu_to_le64(cifs_UnixTimeToNT(ts));
+			pfile_info->ChangeTime = pfile_info->LastWriteTime;
+			if (*pOplock & CIFS_CREATE_ACTION)
+				pfile_info->CreationTime = pfile_info->LastWriteTime;
+			else
+				pfile_info->CreationTime = 0;
+			pfile_info->LastAccessTime = 0;
 			pfile_info->Attributes =
 				cpu_to_le32(le16_to_cpu(pSMBr->FileAttributes));
 			/* the file_info buf is endian converted by caller */
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 6e928e90d72b..73d3dc83faa6 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -853,7 +853,9 @@ static int cifs_open_file(const unsigned int xid, struct cifs_open_parms *oparms
 				   oparms->disposition,
 				   oparms->desired_access,
 				   oparms->create_options,
-				   &oparms->fid->netfid, oplock, &fi,
+				   &oparms->fid->netfid,
+				   oplock,
+				   data ? &fi : NULL,
 				   oparms->cifs_sb->local_nls,
 				   cifs_remap(oparms->cifs_sb));
 	else
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 24/35] cifs: Add a new callback set_file_disp() for setting file disposition (delete pending state)
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (22 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 23/35] cifs: Fix SMBLegacyOpen() function Pali Rohár
@ 2025-08-31 12:35 ` 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
                   ` (11 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Implement it for all SMB dialects. It will be used by follow up changes.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsglob.h  |  3 +++
 fs/smb/client/smb1ops.c   |  8 ++++++++
 fs/smb/client/smb2ops.c   | 11 +++++++++++
 fs/smb/client/smb2pdu.c   | 13 +++++++++++++
 fs/smb/client/smb2proto.h |  2 ++
 5 files changed, 37 insertions(+)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 0ecf4988664e..7162b9120198 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -426,6 +426,9 @@ struct smb_version_operations {
 	/* set attributes */
 	int (*set_file_info)(struct inode *, const char *, FILE_BASIC_INFO *,
 			     const unsigned int);
+	/* set file disposition (delete pending state) */
+	int (*set_file_disp)(const unsigned int xid, struct cifs_tcon *tcon,
+			     struct cifs_fid *fid, bool delete_pending);
 	int (*set_compression)(const unsigned int, struct cifs_tcon *,
 			       struct cifsFileInfo *);
 	/* check if we can send an echo or nor */
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 73d3dc83faa6..e37104d3c5d7 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1079,6 +1079,13 @@ smb_set_file_info(struct inode *inode, const char *full_path,
 	return rc;
 }
 
+static int
+cifs_set_file_disp(const unsigned int xid, struct cifs_tcon *tcon,
+		   struct cifs_fid *fid, bool delete_pending)
+{
+	return CIFSSMBSetFileDisposition(xid, tcon, delete_pending, fid->netfid, current->tgid);
+}
+
 static int
 cifs_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 		   struct cifsFileInfo *cfile)
@@ -1391,6 +1398,7 @@ struct smb_version_operations smb1_operations = {
 	.set_path_size = CIFSSMBSetEOF,
 	.set_file_size = CIFSSMBSetFileSize,
 	.set_file_info = smb_set_file_info,
+	.set_file_disp = cifs_set_file_disp,
 	.set_compression = cifs_set_compression,
 	.echo = CIFSSMBEcho,
 	.mkdir = CIFSSMBMkDir,
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index ad8947434b71..530e66fa4671 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1515,6 +1515,13 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
 	return rc;
 }
 
+static int
+smb2_set_file_disp(const unsigned int xid, struct cifs_tcon *tcon,
+		   struct cifs_fid *fid, bool delete_pending)
+{
+	return SMB2_set_disp(xid, tcon, fid->persistent_fid, fid->volatile_fid, delete_pending);
+}
+
 static int
 SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
 		     u64 persistent_fid, u64 volatile_fid,
@@ -5310,6 +5317,7 @@ struct smb_version_operations smb20_operations = {
 	.set_path_size = smb2_set_path_size,
 	.set_file_size = smb2_set_file_size,
 	.set_file_info = smb2_set_file_info,
+	.set_file_disp = smb2_set_file_disp,
 	.set_compression = smb2_set_compression,
 	.mkdir = smb2_mkdir,
 	.mkdir_setinfo = smb2_mkdir_setinfo,
@@ -5413,6 +5421,7 @@ struct smb_version_operations smb21_operations = {
 	.set_path_size = smb2_set_path_size,
 	.set_file_size = smb2_set_file_size,
 	.set_file_info = smb2_set_file_info,
+	.set_file_disp = smb2_set_file_disp,
 	.set_compression = smb2_set_compression,
 	.mkdir = smb2_mkdir,
 	.mkdir_setinfo = smb2_mkdir_setinfo,
@@ -5520,6 +5529,7 @@ struct smb_version_operations smb30_operations = {
 	.set_path_size = smb2_set_path_size,
 	.set_file_size = smb2_set_file_size,
 	.set_file_info = smb2_set_file_info,
+	.set_file_disp = smb2_set_file_disp,
 	.set_compression = smb2_set_compression,
 	.mkdir = smb2_mkdir,
 	.mkdir_setinfo = smb2_mkdir_setinfo,
@@ -5635,6 +5645,7 @@ struct smb_version_operations smb311_operations = {
 	.set_path_size = smb2_set_path_size,
 	.set_file_size = smb2_set_file_size,
 	.set_file_info = smb2_set_file_info,
+	.set_file_disp = smb2_set_file_disp,
 	.set_compression = smb2_set_compression,
 	.mkdir = smb2_mkdir,
 	.mkdir_setinfo = smb2_mkdir_setinfo,
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index bf588ec99618..e05ddd446467 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -5755,6 +5755,19 @@ SMB2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
 		0, 1, (void **)&buf, &len);
 }
 
+int
+SMB2_set_disp(const unsigned int xid, struct cifs_tcon *tcon,
+	      u64 persistent_fid, u64 volatile_fid, bool delete_pending)
+{
+	__u8 disp = delete_pending;
+	__u8 *buf = &disp;
+	unsigned int len = sizeof(disp);
+
+	return send_set_info(xid, tcon, persistent_fid, volatile_fid,
+			     current->tgid, FILE_DISPOSITION_INFORMATION,
+			     SMB2_O_INFO_FILE, 0, 1, (void **)&buf, &len);
+}
+
 int
 SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
 		  const u64 persistent_fid, const u64 volatile_fid,
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index 6e805ece6a7b..d78ea3a6a5fb 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -249,6 +249,8 @@ extern int SMB2_set_acl(const unsigned int xid, struct cifs_tcon *tcon,
 extern int SMB2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
 		       u64 persistent_fid, u64 volatile_fid,
 		       struct smb2_file_full_ea_info *buf, int len);
+extern int SMB2_set_disp(const unsigned int xid, struct cifs_tcon *tcon,
+			 u64 persistent_fid, u64 volatile_fid, bool delete_pending);
 extern int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 				u64 persistent_fid, u64 volatile_fid);
 extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 25/35] cifs: Add a new callback rename_opened_file() for renaming an opened file
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (23 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 26/35] cifs: Add SMB2+ support into cifs_rename_pending_delete() function Pali Rohár
                   ` (10 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Implement it for all SMB dialects. It will be used by follow up changes.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsglob.h  |  4 ++++
 fs/smb/client/smb1ops.c   | 18 ++++++++++++++++++
 fs/smb/client/smb2ops.c   | 13 +++++++++++++
 fs/smb/client/smb2pdu.c   | 28 ++++++++++++++++++++++++++++
 fs/smb/client/smb2proto.h |  4 ++++
 5 files changed, 67 insertions(+)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 7162b9120198..ec5608924ce7 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -462,6 +462,10 @@ struct smb_version_operations {
 		      struct dentry *source_dentry,
 		      const char *from_name, const char *to_name,
 		      struct cifs_sb_info *cifs_sb);
+	/* send rename request for opened file */
+	int (*rename_opened_file)(const unsigned int xid, struct cifs_tcon *tcon,
+				  struct cifs_fid *fid, const char *new_full_path,
+				  bool overwrite, struct cifs_sb_info *cifs_sb);
 	/* send create hardlink request */
 	int (*create_hardlink)(const unsigned int xid,
 			       struct cifs_tcon *tcon,
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index e37104d3c5d7..26798db5c00b 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1086,6 +1086,23 @@ cifs_set_file_disp(const unsigned int xid, struct cifs_tcon *tcon,
 	return CIFSSMBSetFileDisposition(xid, tcon, delete_pending, fid->netfid, current->tgid);
 }
 
+static int
+cifs_rename_opened_file(const unsigned int xid, struct cifs_tcon *tcon,
+			struct cifs_fid *fid, const char *new_full_path,
+			bool overwrite, struct cifs_sb_info *cifs_sb)
+{
+	const char *name;
+
+	/* CIFSSMBRenameOpenFile() requires just new basename of the file */
+	name = strrchr(new_full_path, CIFS_DIR_SEP(cifs_sb));
+	if (name)
+		name++;
+	else
+		name = new_full_path;
+	return CIFSSMBRenameOpenFile(xid, tcon, fid->netfid, name, overwrite,
+				     cifs_sb->local_nls, cifs_remap(cifs_sb));
+}
+
 static int
 cifs_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 		   struct cifsFileInfo *cfile)
@@ -1407,6 +1424,7 @@ struct smb_version_operations smb1_operations = {
 	.unlink = CIFSSMBDelFile,
 	.rename_pending_delete = cifs_rename_pending_delete,
 	.rename = CIFSSMBRename,
+	.rename_opened_file = cifs_rename_opened_file,
 	.create_hardlink = CIFSCreateHardLink,
 	.query_symlink = cifs_query_symlink,
 	.get_reparse_point_buffer = cifs_get_reparse_point_buffer,
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 530e66fa4671..0ba15af86582 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1522,6 +1522,15 @@ smb2_set_file_disp(const unsigned int xid, struct cifs_tcon *tcon,
 	return SMB2_set_disp(xid, tcon, fid->persistent_fid, fid->volatile_fid, delete_pending);
 }
 
+static int
+smb2_rename_opened_file(const unsigned int xid, struct cifs_tcon *tcon,
+			struct cifs_fid *fid, const char *new_full_path,
+			bool overwrite, struct cifs_sb_info *cifs_sb)
+{
+	return SMB2_set_full_path(xid, tcon, fid->persistent_fid, fid->volatile_fid,
+				  new_full_path, overwrite, cifs_sb);
+}
+
 static int
 SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
 		     u64 persistent_fid, u64 volatile_fid,
@@ -5324,6 +5333,7 @@ struct smb_version_operations smb20_operations = {
 	.rmdir = smb2_rmdir,
 	.unlink = smb2_unlink,
 	.rename = smb2_rename_path,
+	.rename_opened_file = smb2_rename_opened_file,
 	.create_hardlink = smb2_create_hardlink,
 	.get_reparse_point_buffer = smb2_get_reparse_point_buffer,
 	.query_mf_symlink = smb3_query_mf_symlink,
@@ -5428,6 +5438,7 @@ struct smb_version_operations smb21_operations = {
 	.rmdir = smb2_rmdir,
 	.unlink = smb2_unlink,
 	.rename = smb2_rename_path,
+	.rename_opened_file = smb2_rename_opened_file,
 	.create_hardlink = smb2_create_hardlink,
 	.get_reparse_point_buffer = smb2_get_reparse_point_buffer,
 	.query_mf_symlink = smb3_query_mf_symlink,
@@ -5536,6 +5547,7 @@ struct smb_version_operations smb30_operations = {
 	.rmdir = smb2_rmdir,
 	.unlink = smb2_unlink,
 	.rename = smb2_rename_path,
+	.rename_opened_file = smb2_rename_opened_file,
 	.create_hardlink = smb2_create_hardlink,
 	.get_reparse_point_buffer = smb2_get_reparse_point_buffer,
 	.query_mf_symlink = smb3_query_mf_symlink,
@@ -5653,6 +5665,7 @@ struct smb_version_operations smb311_operations = {
 	.rmdir = smb2_rmdir,
 	.unlink = smb2_unlink,
 	.rename = smb2_rename_path,
+	.rename_opened_file = smb2_rename_opened_file,
 	.create_hardlink = smb2_create_hardlink,
 	.get_reparse_point_buffer = smb2_get_reparse_point_buffer,
 	.query_mf_symlink = smb3_query_mf_symlink,
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index e05ddd446467..a6c69b01ec6b 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -5768,6 +5768,34 @@ SMB2_set_disp(const unsigned int xid, struct cifs_tcon *tcon,
 			     SMB2_O_INFO_FILE, 0, 1, (void **)&buf, &len);
 }
 
+int
+SMB2_set_full_path(const unsigned int xid, struct cifs_tcon *tcon,
+		   u64 persistent_fid, u64 volatile_fid, const char *new_full_path,
+		   bool overwrite, struct cifs_sb_info *cifs_sb)
+{
+	struct smb2_file_rename_info rename_info = {};
+	unsigned int size[2];
+	void *data[2];
+	int rc;
+
+	data[1] = cifs_convert_path_to_utf16(new_full_path, cifs_sb);
+	if (!data[1])
+		return -ENOMEM;
+	size[1] = 2 * UniStrnlen((wchar_t *)data[1], PATH_MAX);
+
+	rename_info.ReplaceIfExists = overwrite;
+	rename_info.RootDirectory = 0;
+	rename_info.FileNameLength = cpu_to_le32(size[1]);
+	data[0] = &rename_info;
+	size[0] = sizeof(rename_info);
+
+	rc = send_set_info(xid, tcon, persistent_fid, volatile_fid,
+			   current->tgid, FILE_RENAME_INFORMATION,
+			   SMB2_O_INFO_FILE, 0, 2, data, size);
+	kfree(data[1]);
+	return rc;
+}
+
 int
 SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
 		  const u64 persistent_fid, const u64 volatile_fid,
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index d78ea3a6a5fb..7c300dd9ea4e 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -251,6 +251,10 @@ extern int SMB2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
 		       struct smb2_file_full_ea_info *buf, int len);
 extern int SMB2_set_disp(const unsigned int xid, struct cifs_tcon *tcon,
 			 u64 persistent_fid, u64 volatile_fid, bool delete_pending);
+extern int SMB2_set_full_path(const unsigned int xid, struct cifs_tcon *tcon,
+			      u64 persistent_fid, u64 volatile_fid,
+			      const char *name, bool overwrite,
+			      struct cifs_sb_info *cifs_sb);
 extern int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 				u64 persistent_fid, u64 volatile_fid);
 extern int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 26/35] cifs: Add SMB2+ support into cifs_rename_pending_delete() function.
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (24 preceding siblings ...)
  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 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 27/35] cifs: Move SMB1 usage of CIFSPOSIXDelFile() from inode.c to cifssmb.c Pali Rohár
                   ` (9 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Currently the cifs_rename_pending_delete() function calls directly the SMB1
functions and therefore cannot be used by SMB2+ dialects.

Change cifs_rename_pending_delete() code to use tcon->ses->server->ops->
callbacks instead of direct SMB1 functions. This allows to use this
function also by SMB2 and SMB3 dialects.

Mark the function cifs_rename_pending_delete() as static in inode.c and
calls it directly. As it is now dialect neutral, remove it from struct
smb_version_operations callback list.

This change allows to use silly rename in SMB2+ by follow up changes.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifsglob.h  |  3 ---
 fs/smb/client/cifsproto.h |  3 ---
 fs/smb/client/inode.c     | 56 +++++++++++++++++----------------------
 fs/smb/client/smb1ops.c   |  1 -
 4 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index ec5608924ce7..592a4faa3440 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -453,9 +453,6 @@ struct smb_version_operations {
 	/* unlink file */
 	int (*unlink)(const unsigned int, struct cifs_tcon *, const char *,
 		      struct cifs_sb_info *, struct dentry *);
-	/* open, rename and delete file */
-	int (*rename_pending_delete)(const char *, struct dentry *,
-				     const unsigned int);
 	/* send rename request */
 	int (*rename)(const unsigned int xid,
 		      struct cifs_tcon *tcon,
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index f248b18f1cf3..a29662a4d83d 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -242,9 +242,6 @@ extern int cifs_get_inode_info_unix(struct inode **pinode,
 			struct super_block *sb, unsigned int xid);
 extern int cifs_set_file_info(struct inode *inode, struct iattr *attrs,
 			      unsigned int xid, const char *full_path, __u32 dosattr);
-extern int cifs_rename_pending_delete(const char *full_path,
-				      struct dentry *dentry,
-				      const unsigned int xid);
 extern int sid_to_id(struct cifs_sb_info *cifs_sb, struct smb_sid *psid,
 				struct cifs_fattr *fattr, uint sidtype);
 extern int cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb,
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 9a5504a3406d..c3f101d10488 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1684,7 +1684,6 @@ cifs_set_file_info(struct inode *inode, struct iattr *attrs, unsigned int xid,
 	return server->ops->set_file_info(inode, full_path, &info_buf, xid);
 }
 
-#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 /*
  * Open the given file (if it isn't already), set the DELETE_PENDING bit
  * and rename it to a random name that hopefully won't conflict with
@@ -1697,9 +1696,11 @@ cifs_set_file_info(struct inode *inode, struct iattr *attrs, unsigned int xid,
 #define SILLYNAME_LEN (SILLYNAME_PREFIX_LEN + \
 		SILLYNAME_FILEID_LEN + \
 		SILLYNAME_COUNTER_LEN)
-int
-cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
-			   const unsigned int xid)
+static int
+cifs_rename_pending_delete(const unsigned int xid,
+			   struct cifs_tcon *tcon,
+			   const char *full_path,
+			   struct dentry *dentry)
 {
 	int oplock = 0;
 	int rc;
@@ -1708,8 +1709,6 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	struct inode *inode = d_inode(dentry);
 	struct cifsInodeInfo *cifsInode = CIFS_I(inode);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
-	struct tcon_link *tlink;
-	struct cifs_tcon *tcon;
 	__u32 dosattr, origattr;
 	char *sillyname_full_path = NULL;
 	bool can_rename_opened_file = true;
@@ -1719,11 +1718,6 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	const char *dirpath_end;
 	size_t dirpath_len;
 
-	tlink = cifs_sb_tlink(cifs_sb);
-	if (IS_ERR(tlink))
-		return PTR_ERR(tlink);
-	tcon = tlink_tcon(tlink);
-
 	/* construct random name ".smb<inodenum><counter>" */
 	while (true) {
 		static unsigned int sillycounter; /* globally unique */
@@ -1753,8 +1747,10 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	/*
 	 * We cannot rename the opened file if the SMB1 server doesn't
 	 * support CAP_INFOLEVEL_PASSTHRU. But we can rename file via path.
+	 * SMB2+ always supports renaming of the opened file.
 	 */
-	if (!(tcon->ses->capabilities & CAP_INFOLEVEL_PASSTHRU))
+	if (tcon->ses->server->vals->protocol_id == SMB10_PROT_ID &&
+	    !(tcon->ses->capabilities & CAP_INFOLEVEL_PASSTHRU))
 		can_rename_opened_file = false;
 
 	dirpath_end = strrchr(full_path, CIFS_DIR_SEP(cifs_sb));
@@ -1821,16 +1817,16 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		.fid = &fid,
 	};
 
-	rc = CIFS_open(xid, &oparms, &oplock, NULL);
+	rc = tcon->ses->server->ops->open(xid, &oparms, &oplock, NULL);
 	if (rc != 0)
 		goto undo_rename_path;
 
 	/* rename the opened file (if it was not already renamed before the open) */
 	if (can_rename_opened_file) {
-		rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, sillyname,
+		rc = tcon->ses->server->ops->rename_opened_file(
+				   xid, tcon, &fid, sillyname_full_path,
 				   false /* overwrite */,
-				   cifs_sb->local_nls,
-				   cifs_remap(cifs_sb));
+				   cifs_sb);
 		if (rc != 0) {
 			rc = -EBUSY;
 			goto undo_close;
@@ -1839,8 +1835,7 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 
 	/* try to set DELETE_PENDING */
 	if (!test_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags)) {
-		rc = CIFSSMBSetFileDisposition(xid, tcon, true, fid.netfid,
-					       current->tgid);
+		rc = tcon->ses->server->ops->set_file_disp(xid, tcon, &fid, true);
 		/*
 		 * some samba versions return -ENOENT when we try to set the
 		 * file disposition here. Likely a samba bug, but work around
@@ -1856,11 +1851,10 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 		set_bit(CIFS_INO_DELETE_PENDING, &cifsInode->flags);
 	}
 
-	CIFSSMBClose(xid, tcon, fid.netfid);
+	tcon->ses->server->ops->close(xid, tcon, &fid);
 
 out:
 	kfree(sillyname_full_path);
-	cifs_put_tlink(tlink);
 	return rc;
 
 	/*
@@ -1870,14 +1864,16 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	 */
 undo_rename_opened_file:
 	if (can_rename_opened_file)
-		CIFSSMBRenameOpenFile(xid, tcon, fid.netfid, dentry->d_name.name,
+		tcon->ses->server->ops->rename_opened_file(
+				xid, tcon, &fid, full_path,
 				true /* overwrite */,
-				cifs_sb->local_nls, cifs_remap(cifs_sb));
+				cifs_sb);
 undo_close:
-	CIFSSMBClose(xid, tcon, fid.netfid);
+	tcon->ses->server->ops->close(xid, tcon, &fid);
 undo_rename_path:
 	if (!can_rename_opened_file)
-		CIFSSMBRename(xid, tcon, dentry,
+		tcon->ses->server->ops->rename(
+				xid, tcon, dentry,
 				sillyname_full_path,
 				full_path,
 				cifs_sb);
@@ -1888,7 +1884,6 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	}
 	goto out;
 }
-#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
 
 /* copied from fs/nfs/dir.c with small changes */
 static void
@@ -1981,13 +1976,10 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
 	} else if (rc == -ENOENT) {
 		d_drop(dentry);
 	} else if (rc == -EBUSY) {
-		if (server->ops->rename_pending_delete) {
-			rc = server->ops->rename_pending_delete(full_path,
-								dentry, xid);
-			if (rc == 0) {
-				cifs_mark_open_handles_for_deleted_file(inode, full_path);
-				cifs_drop_nlink(inode);
-			}
+		rc = cifs_rename_pending_delete(xid, tcon, full_path, dentry);
+		if (rc == 0) {
+			cifs_mark_open_handles_for_deleted_file(inode, full_path);
+			cifs_drop_nlink(inode);
 		}
 	} else if ((rc == -EACCES) && (dosattr == 0) && inode) {
 		attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 26798db5c00b..746dd9aa6b1b 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -1422,7 +1422,6 @@ struct smb_version_operations smb1_operations = {
 	.mkdir_setinfo = cifs_mkdir_setinfo,
 	.rmdir = CIFSSMBRmDir,
 	.unlink = CIFSSMBDelFile,
-	.rename_pending_delete = cifs_rename_pending_delete,
 	.rename = CIFSSMBRename,
 	.rename_opened_file = cifs_rename_opened_file,
 	.create_hardlink = CIFSCreateHardLink,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 27/35] cifs: Move SMB1 usage of CIFSPOSIXDelFile() from inode.c to cifssmb.c
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (25 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 26/35] cifs: Add SMB2+ support into cifs_rename_pending_delete() function Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 28/35] cifs: Fix smb2_unlink() to fail on directory Pali Rohár
                   ` (8 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Special case of unlinking file via SMB1 UNIX extension is currently in the
dialect agnostic function cifs_unlink() and hidden under the #ifdef
CONFIG_CIFS_ALLOW_INSECURE_LEGACY.

Cleanup the code and move this functionality into the SMB1 ->unlink()
callback, which removes one #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
code block from inode.c

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifssmb.c | 12 ++++++++++++
 fs/smb/client/inode.c   | 20 +++-----------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index c09713ebdc7c..3a0452479a69 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -768,6 +768,18 @@ CIFSSMBDelFile(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 	int name_len;
 	int remap = cifs_remap(cifs_sb);
 
+	/* If UNIX extensions are available then use UNIX UNLINK call. */
+	if (cap_unix(tcon->ses) &&
+	    (le64_to_cpu(tcon->fsUnixInfo.Capability) & CIFS_UNIX_POSIX_PATH_OPS_CAP)) {
+		rc = CIFSPOSIXDelFile(xid, tcon, name,
+				      SMB_POSIX_UNLINK_FILE_TARGET,
+				      cifs_sb->local_nls,
+				      cifs_remap(cifs_sb));
+		cifs_dbg(FYI, "posix del rc %d\n", rc);
+		if (rc == 0 || rc == -ENOENT)
+			return rc;
+	}
+
 DelFileRetry:
 	rc = smb_init(SMB_COM_DELETE, 1, tcon, (void **) &pSMB,
 		      (void **) &pSMBr);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index c3f101d10488..545964cac9cd 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1947,27 +1947,13 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
 
 	netfs_wait_for_outstanding_io(inode);
 	cifs_close_deferred_file_under_dentry(tcon, full_path);
-#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
-	if (cap_unix(tcon->ses) && (CIFS_UNIX_POSIX_PATH_OPS_CAP &
-				le64_to_cpu(tcon->fsUnixInfo.Capability))) {
-		rc = CIFSPOSIXDelFile(xid, tcon, full_path,
-			SMB_POSIX_UNLINK_FILE_TARGET, cifs_sb->local_nls,
-			cifs_remap(cifs_sb));
-		cifs_dbg(FYI, "posix del rc %d\n", rc);
-		if ((rc == 0) || (rc == -ENOENT))
-			goto psx_del_no_retry;
-	}
-#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */
 
 retry_std_delete:
-	if (!server->ops->unlink) {
+	if (!server->ops->unlink)
 		rc = -ENOSYS;
-		goto psx_del_no_retry;
-	}
-
-	rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry);
+	else
+		rc = server->ops->unlink(xid, tcon, full_path, cifs_sb, dentry);
 
-psx_del_no_retry:
 	if (!rc) {
 		if (inode) {
 			cifs_mark_open_handles_for_deleted_file(inode, full_path);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 28/35] cifs: Fix smb2_unlink() to fail on directory
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (26 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 27/35] cifs: Move SMB1 usage of CIFSPOSIXDelFile() from inode.c to cifssmb.c Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 29/35] cifs: Fix smb2_rmdir() on reparse point Pali Rohár
                   ` (7 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

unlink() should fail on the directory with ENOTDIR error code.
Flag CREATE_NOT_DIR handles that.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index c8b0e9b2438f..c69293fcf26c 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1348,7 +1348,7 @@ smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 
 	oparms = CIFS_OPARMS(cifs_sb, tcon, name,
 			     DELETE, FILE_OPEN,
-			     CREATE_DELETE_ON_CLOSE | OPEN_REPARSE_POINT,
+			     CREATE_DELETE_ON_CLOSE | CREATE_NOT_DIR | OPEN_REPARSE_POINT,
 			     ACL_NO_MODE);
 	int rc = smb2_compound_op(xid, tcon, cifs_sb, name, &oparms,
 				  NULL, &(int){SMB2_OP_DELETE}, 1,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 29/35] cifs: Fix smb2_rmdir() on reparse point
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (27 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 28/35] cifs: Fix smb2_unlink() to fail on directory Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 30/35] cifs: Simplify SMB2_OP_DELETE API usage Pali Rohár
                   ` (6 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

rmdir() should remove the directory node specified by the path and not the
path where reparse point can point. Open flag OPEN_REPARSE_POINT cause to
remove directory node itself.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index c69293fcf26c..59d73e01ccd2 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1333,7 +1333,7 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 
 	drop_cached_dir_by_name(xid, tcon, name, cifs_sb);
 	oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE,
-			     FILE_OPEN, CREATE_NOT_FILE, ACL_NO_MODE);
+			     FILE_OPEN, CREATE_NOT_FILE | OPEN_REPARSE_POINT, ACL_NO_MODE);
 	return smb2_compound_op(xid, tcon, cifs_sb,
 				name, &oparms, NULL,
 				&(int){SMB2_OP_RMDIR}, 1,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 30/35] cifs: Simplify SMB2_OP_DELETE API usage
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (28 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 29/35] cifs: Fix smb2_rmdir() on reparse point Pali Rohár
@ 2025-08-31 12:35 ` 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
                   ` (5 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Currently the caller of SMB2_OP_DELETE operation has to specify also the
CREATE_DELETE_ON_CLOSE flag. Simplify API usage and let the callee
smb2_compound_op() to automatically set this flag when issuing the
SMB2_OP_DELETE operation.

No functional change.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2inode.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 59d73e01ccd2..a3b1ed53a860 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -255,6 +255,13 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 	vars->oparms = *oparms;
 	vars->oparms.fid = &fid;
 
+	for (i = 0; i < num_cmds; i++) {
+		if (cmds[i] == SMB2_OP_DELETE) {
+			vars->oparms.create_options |= CREATE_DELETE_ON_CLOSE;
+			break;
+		}
+	}
+
 	rqst[num_rqst].rq_iov = &vars->open_iov[0];
 	rqst[num_rqst].rq_nvec = SMB2_CREATE_IOV_SIZE;
 	rc = SMB2_open_init(tcon, server,
@@ -1348,7 +1355,7 @@ smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 
 	oparms = CIFS_OPARMS(cifs_sb, tcon, name,
 			     DELETE, FILE_OPEN,
-			     CREATE_DELETE_ON_CLOSE | CREATE_NOT_DIR | OPEN_REPARSE_POINT,
+			     CREATE_NOT_DIR | OPEN_REPARSE_POINT,
 			     ACL_NO_MODE);
 	int rc = smb2_compound_op(xid, tcon, cifs_sb, name, &oparms,
 				  NULL, &(int){SMB2_OP_DELETE}, 1,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 31/35] cifs: Deduplicate smb2_unlink() and smb2_rmdir() into one common function
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (29 preceding siblings ...)
  2025-08-31 12:35 ` [PATCH 30/35] cifs: Simplify SMB2_OP_DELETE API usage Pali Rohár
@ 2025-08-31 12:35 ` Pali Rohár
  2025-08-31 12:35 ` [PATCH 32/35] cifs: Use cifs_rename_pending_delete() fallback also for rmdir() Pali Rohár
                   ` (4 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

These two functions smb2_unlink() and smb2_rmdir() share lot of common
logic. Deduplicate them into one common static function smb2_remove().

No functional change. All logic and handling stay same.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/smb2inode.c | 57 +++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index a3b1ed53a860..0dd4a77dfb64 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1332,43 +1332,54 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name,
 		cifs_i->cifsAttrs = dosattrs;
 }
 
-int
-smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
-	   struct cifs_sb_info *cifs_sb)
+static int
+smb2_remove(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
+	    struct cifs_sb_info *cifs_sb, struct dentry *dentry, bool is_dir)
 {
 	struct cifs_open_parms oparms;
+	int op_flags;
+	int op;
+	int rc;
 
-	drop_cached_dir_by_name(xid, tcon, name, cifs_sb);
-	oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE,
-			     FILE_OPEN, CREATE_NOT_FILE | OPEN_REPARSE_POINT, ACL_NO_MODE);
-	return smb2_compound_op(xid, tcon, cifs_sb,
-				name, &oparms, NULL,
-				&(int){SMB2_OP_RMDIR}, 1,
-				NULL, NULL, NULL, NULL);
-}
-
-int
-smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
-	    struct cifs_sb_info *cifs_sb, struct dentry *dentry)
-{
-	struct cifs_open_parms oparms;
+	if (is_dir)
+		drop_cached_dir_by_name(xid, tcon, name, cifs_sb);
 
+	if (is_dir) {
+		op = SMB2_OP_RMDIR;
+		op_flags = CREATE_NOT_FILE;
+	} else {
+		op = SMB2_OP_DELETE;
+		op_flags = CREATE_NOT_DIR;
+	}
 	oparms = CIFS_OPARMS(cifs_sb, tcon, name,
 			     DELETE, FILE_OPEN,
-			     CREATE_NOT_DIR | OPEN_REPARSE_POINT,
+			     OPEN_REPARSE_POINT | op_flags,
 			     ACL_NO_MODE);
-	int rc = smb2_compound_op(xid, tcon, cifs_sb, name, &oparms,
-				  NULL, &(int){SMB2_OP_DELETE}, 1,
-				  NULL, NULL, NULL, dentry);
-	if (rc == -EINVAL) {
+	rc = smb2_compound_op(xid, tcon, cifs_sb, name, &oparms,
+			      NULL, &op, 1, NULL, NULL, NULL, dentry);
+	if (rc == -EINVAL && dentry) {
 		cifs_dbg(FYI, "invalid lease key, resending request without lease");
 		rc = smb2_compound_op(xid, tcon, cifs_sb, name, &oparms,
-				      NULL, &(int){SMB2_OP_DELETE}, 1,
+				      NULL, &op, 1,
 				      NULL, NULL, NULL, NULL);
 	}
 	return rc;
 }
 
+int
+smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
+	   struct cifs_sb_info *cifs_sb)
+{
+	return smb2_remove(xid, tcon, name, cifs_sb, NULL, true /* is_dir */);
+}
+
+int
+smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
+	    struct cifs_sb_info *cifs_sb, struct dentry *dentry)
+{
+	return smb2_remove(xid, tcon, name, cifs_sb, dentry, false /* is_dir */);
+}
+
 static int smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon,
 			      const char *from_name, const char *to_name,
 			      struct cifs_sb_info *cifs_sb,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 32/35] cifs: Use cifs_rename_pending_delete() fallback also for rmdir()
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (30 preceding siblings ...)
  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 ` 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
                   ` (3 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

When ->rmdir() callback fails on the -EBUSY error then use the silly rename
functionality fallback. Removing of directories has exactly same problem
with DELETE_PENDING state as removal of the files.

Empty directory which is opened on Windows server will stay in
DELETE_PENDING state until the last user on server does not close it.
Directory itself is not immediately unlinked at remove call, like file.

Note that currently the ->rmdir() callbacks do not return -EBUSY error.
This will be implemented in follow up changes.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/inode.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 545964cac9cd..abe3108e4963 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1700,6 +1700,7 @@ static int
 cifs_rename_pending_delete(const unsigned int xid,
 			   struct cifs_tcon *tcon,
 			   const char *full_path,
+			   bool is_dir,
 			   struct dentry *dentry)
 {
 	int oplock = 0;
@@ -1811,7 +1812,8 @@ cifs_rename_pending_delete(const unsigned int xid,
 		.tcon = tcon,
 		.cifs_sb = cifs_sb,
 		.desired_access = DELETE,
-		.create_options = cifs_create_options(cifs_sb, CREATE_NOT_DIR),
+		.create_options = cifs_create_options(cifs_sb,
+					is_dir ? CREATE_NOT_FILE : CREATE_NOT_DIR),
 		.disposition = FILE_OPEN,
 		.path = full_path,
 		.fid = &fid,
@@ -1962,7 +1964,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
 	} else if (rc == -ENOENT) {
 		d_drop(dentry);
 	} else if (rc == -EBUSY) {
-		rc = cifs_rename_pending_delete(xid, tcon, full_path, dentry);
+		rc = cifs_rename_pending_delete(xid, tcon, full_path, false /* is_dir */, dentry);
 		if (rc == 0) {
 			cifs_mark_open_handles_for_deleted_file(inode, full_path);
 			cifs_drop_nlink(inode);
@@ -2298,6 +2300,9 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)
 	}
 
 	rc = server->ops->rmdir(xid, tcon, full_path, cifs_sb);
+	if (rc == -EBUSY)
+		rc = cifs_rename_pending_delete(xid, tcon, full_path, true /* is_dir */, direntry);
+
 	cifs_put_tlink(tlink);
 
 	if (!rc) {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 33/35] cifs: Add a new open flag CREATE_OPTION_EXCLUSIVE to open with deny all shared reservation
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (31 preceding siblings ...)
  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 ` 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
                   ` (2 subsequent siblings)
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:36 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

This is just an internal flag for cifs.ko code, not exported to userspace.
It allows cifs.ko code to take an exclusive open with deny all shared
reservation. It is going to be used by the upcoming fixes for the unlink()
and rmdir() silly rename support.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 fs/smb/client/cifspdu.h |  1 +
 fs/smb/client/cifssmb.c | 12 ++++++++++--
 fs/smb/client/smb2pdu.c |  6 +++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index cb1f99d22dd7..90b0b82e23b1 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -395,6 +395,7 @@
 #define CREATE_OPTIONS_MASK     0x007FFFFF
 #define CREATE_OPTION_READONLY	0x10000000
 #define CREATE_OPTION_SPECIAL   0x20000000   /* system. NB not sent over wire */
+#define CREATE_OPTION_EXCLUSIVE 0x40000000   /* exclusive open, NB not set over wire */
 
 /* ImpersonationLevel flags */
 #define SECURITY_ANONYMOUS      0
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 3a0452479a69..37bc0541bc21 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1125,7 +1125,11 @@ SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 		pSMB->OpenFlags |= cpu_to_le16(REQ_MORE_INFO);
 
 	pSMB->Mode = cpu_to_le16(access_flags_to_smbopen_mode(access_flags));
-	pSMB->Mode |= cpu_to_le16(SMBOPEN_DENY_NONE);
+
+	if (create_options & CREATE_OPTION_EXCLUSIVE)
+		pSMB->Mode |= cpu_to_le16(SMBOPEN_DENY_ALL);
+	else
+		pSMB->Mode |= cpu_to_le16(SMBOPEN_DENY_NONE);
 
 	if (create_options & CREATE_WRITE_THROUGH)
 		pSMB->Mode |= cpu_to_le16(SMBOPEN_WRITE_THROUGH);
@@ -1281,7 +1285,11 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
 	if (create_options & CREATE_OPTION_READONLY)
 		req->FileAttributes |= cpu_to_le32(ATTR_READONLY);
 
-	req->ShareAccess = cpu_to_le32(FILE_SHARE_ALL);
+	if (create_options & CREATE_OPTION_EXCLUSIVE)
+		req->ShareAccess = cpu_to_le32(FILE_NO_SHARE);
+	else
+		req->ShareAccess = cpu_to_le32(FILE_SHARE_ALL);
+
 	req->CreateDisposition = cpu_to_le32(disposition);
 	req->CreateOptions = cpu_to_le32(create_options & CREATE_OPTIONS_MASK);
 
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index a6c69b01ec6b..8315213d699e 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3034,7 +3034,11 @@ SMB2_open_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 	req->DesiredAccess = cpu_to_le32(oparms->desired_access);
 	/* File attributes ignored on open (used in create though) */
 	req->FileAttributes = cpu_to_le32(file_attributes);
-	req->ShareAccess = FILE_SHARE_ALL_LE;
+
+	if (oparms->create_options & CREATE_OPTION_EXCLUSIVE)
+		req->ShareAccess = cpu_to_le32(FILE_NO_SHARE);
+	else
+		req->ShareAccess = cpu_to_le32(FILE_SHARE_ALL);
 
 	req->CreateDisposition = cpu_to_le32(oparms->disposition);
 	req->CreateOptions = cpu_to_le32(oparms->create_options & CREATE_OPTIONS_MASK);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 34/35] cifs: Use CREATE_OPTION_EXCLUSIVE when opening file/dir for SMB2+ non-POSIX unlink/rmdir
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (32 preceding siblings ...)
  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 ` 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
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:36 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Using of CREATE_OPTION_EXCLUSIVE against non-POSIX SMB2+ server ensures
that the smb2_remove() function either success and removes the directory
entry or it returns an error that file or directory is in use by other SMB
client and silly rename is required to use.

POSIX-based SMB2+ servers do not have this problems as they should unlink
the directory entry immediately and not transition them into delete pending
state.

This allows the cifs_unlink() and cifs_rmdir() functions against non-POSIX
servers to detect these failures via -EBUSY error from smb2_unlink() and
smb2_rmdir() calls and fallbacks to cifs_rename_pending_delete() which
implements silly rename.

This is the final change which enables the silly rename functionality for
the unlink and rmdir calls in SMB2+ dialects on mounted exports from
Windows servers.

With this change Linux unlink() and rmdir() syscalls called on SMB2+ mounts
from Windows servers cause that on success the path would not exist anymore
and new file or directory with that path can be created.

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

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 0dd4a77dfb64..727349ed76b9 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1351,6 +1351,23 @@ smb2_remove(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 		op = SMB2_OP_DELETE;
 		op_flags = CREATE_NOT_DIR;
 	}
+
+	/*
+	 * CREATE_OPTION_EXCLUSIVE ensures exclusive access to the path.
+	 * If some other client has that path opened then our open fails.
+	 * So together with remove operation it cause that either the path
+	 * is immediately unlinked or the command fails with -EBUSY.
+	 * It should not let the path in the delete pending state.
+	 *
+	 * When using POSIX extensions then we do not need any exclusive
+	 * access to the file or directory.
+	 * In this case the path is unlinked immediately even if it is opened
+	 * by other client. Unlink fails only in case path is directory and
+	 * that directory is not empty.
+	 */
+	if (!tcon->posix_extensions)
+		op_flags |= CREATE_OPTION_EXCLUSIVE;
+
 	oparms = CIFS_OPARMS(cifs_sb, tcon, name,
 			     DELETE, FILE_OPEN,
 			     OPEN_REPARSE_POINT | op_flags,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 35/35] cifs: Use CREATE_OPTION_EXCLUSIVE when doing SMB1 rmdir on NT server
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (33 preceding siblings ...)
  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 ` Pali Rohár
  2025-09-01  7:55 ` [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Stefan Metzmacher
  35 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-08-31 12:36 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, ronnie sahlberg; +Cc: linux-cifs, linux-kernel

Windows NT servers just set the DELETE_PENDING flag when executing the SMB1
SMB_COM_DELETE_DIRECTORY command. This is opposite of the SMB_COM_DELETE
command (can be used only on files) which completely removes the file and
not just transition it into DELETE_PENDING state.

This means that the SMB1 rmdir against Windows NT servers has same issues
as SMB2+ rmdir and silly rename needs to be used. As in SMB2+ rmdir, use
the CREATE_OPTION_EXCLUSIVE logic for issuing SMB1 rmdir when communicating
with NT-based SMB1 server.

With this change Linux rmdir() syscall called on SMB1 mounts from Windows
NT servers cause that on success the path would not exist anymore and new
file or directory with that path can be created.

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

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 37bc0541bc21..31638b71ee0c 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -823,8 +823,53 @@ CIFSSMBRmDir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 	int bytes_returned;
 	int name_len;
 	int remap = cifs_remap(cifs_sb);
+	struct cifs_open_parms oparms;
+	struct cifs_fid fid;
+	int oplock;
 
 	cifs_dbg(FYI, "In CIFSSMBRmDir\n");
+
+	/*
+	 * Do not send SMB_COM_DELETE_DIRECTORY to NT servers. NT servers just
+	 * sets the DELETE PENDING state on the directory and in case that
+	 * directory is opened by some other client, it stay in this state and
+	 * direntry stay present in the parent directory.
+	 *
+	 * So for NT servers use NT OPEN in exclusive mode. It fails when some
+	 * other SMB client has the directory opened, and it triggers the
+	 * sillyrename code path. After successful NT OPEN in exclusive mode,
+	 * sets the DELETE PENDING state and close the directory.
+	 *
+	 * Servers with UNIX extensions should support SMB_COM_DELETE_DIRECTORY
+	 * with correct UNIX semantics, so use this NT OPEN + DELETE PENDING
+	 * only against non-UNIX NT servers.
+	 */
+	if ((tcon->ses->capabilities & CAP_NT_SMBS) &&
+	    !(cap_unix(tcon->ses) &&
+	      (le64_to_cpu(tcon->fsUnixInfo.Capability) & CIFS_UNIX_POSIX_PATH_OPS_CAP))) {
+		oparms = CIFS_OPARMS(cifs_sb, tcon, name, DELETE, FILE_OPEN,
+				     CREATE_OPTION_EXCLUSIVE | CREATE_NOT_FILE | OPEN_REPARSE_POINT,
+				     ACL_NO_MODE);
+		oparms.fid = &fid;
+		oplock = 0;
+		rc = CIFS_open(xid, &oparms, &oplock, NULL);
+		if (rc)
+			return rc;
+		rc = CIFSSMBSetFileDisposition(xid, tcon, true, fid.netfid, current->tgid);
+		/*
+		 * some samba versions return -ENOENT when we try to set the
+		 * file disposition here. Likely a samba bug, but work around
+		 * it for now. This means that some cifsXXX files may hang
+		 * around after they shouldn't.
+		 *
+		 * BB: remove this hack after more servers have the fix
+		 */
+		if (rc == -ENOENT)
+			rc = 0;
+		CIFSSMBClose(xid, tcon, fid.netfid);
+		return rc;
+	}
+
 RmDirRetry:
 	rc = smb_init(SMB_COM_DELETE_DIRECTORY, 0, tcon, (void **) &pSMB,
 		      (void **) &pSMBr);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers
  2025-08-31 12:35 [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers Pali Rohár
                   ` (34 preceding siblings ...)
  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 ` Stefan Metzmacher
  2025-09-01 17:02   ` Pali Rohár
  35 siblings, 1 reply; 40+ messages in thread
From: Stefan Metzmacher @ 2025-09-01  7:55 UTC (permalink / raw)
  To: Pali Rohár, Steve French, Paulo Alcantara, ronnie sahlberg,
	Ralph Böhme
  Cc: linux-cifs, linux-kernel

Hi Pali,

> This patch series improves Linux rmdir() and unlink() syscalls called on
> SMB mounts exported from Windows SMB servers which do not implement
> POSIX semantics of the file and directory removal.
> 
> This patch series should have no impact and no function change when
> communicating with the POSIX SMB servers, as they should implement
> proper rmdir and unlink logic.

Please note that even servers implementing posix/unix extensions,
may also have windows clients connected operating on the same files/directories.
And in that case even posix clients will see the windows behaviour
of DELETE_PENDING for set disposition or on rename
NT_STATUS_ACCESS_DENIED or NT_STATUS_DIRECTORY_NOT_EMPTY.

> When issuing remove path command against non-POSIX / Windows SMB server,
> it let the directory entry which is being removed in the directory until
> all users / clients close all handles / references to that path.
> 
> POSIX requires from rmdir() and unlink() syscalls that after successful
> call, the requested path / directory entry is released and allows to
> create a new file or directory with that name. This is currently not
> working against non-POSIX / Windows SMB servers.
> 
> To workaround this problem fix and improve existing cifs silly rename
> code and extend it also to SMB2 and SMB3 dialects when communicating
> with Windows SMB servers. Silly rename is applied only when it is
> necessary (when some other client has opened file or directory).
> If no other client has the file / dir open then silly rename is not
> used.

If I 'git grep -i silly fs/smb/client' there's no hit, can you
please explain what code do you mean with silly rename?

Thanks!
metze

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers
  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
  0 siblings, 1 reply; 40+ messages in thread
From: Pali Rohár @ 2025-09-01 17:02 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: Steve French, Paulo Alcantara, ronnie sahlberg, Ralph Böhme,
	linux-cifs, linux-kernel

Hello!

On Monday 01 September 2025 09:55:45 Stefan Metzmacher wrote:
> Hi Pali,
> 
> > This patch series improves Linux rmdir() and unlink() syscalls called on
> > SMB mounts exported from Windows SMB servers which do not implement
> > POSIX semantics of the file and directory removal.
> > 
> > This patch series should have no impact and no function change when
> > communicating with the POSIX SMB servers, as they should implement
> > proper rmdir and unlink logic.
> 
> Please note that even servers implementing posix/unix extensions,
> may also have windows clients connected operating on the same files/directories.
> And in that case even posix clients will see the windows behaviour
> of DELETE_PENDING for set disposition or on rename
> NT_STATUS_ACCESS_DENIED or NT_STATUS_DIRECTORY_NOT_EMPTY.

Ok. So does it mean that the issue described here applies also for POSIX
SMB server?

If yes, then I would propose to first fix the problem with
Windows/non-POSIX SMB server and then with others. So it is not too big.

> > When issuing remove path command against non-POSIX / Windows SMB server,
> > it let the directory entry which is being removed in the directory until
> > all users / clients close all handles / references to that path.
> > 
> > POSIX requires from rmdir() and unlink() syscalls that after successful
> > call, the requested path / directory entry is released and allows to
> > create a new file or directory with that name. This is currently not
> > working against non-POSIX / Windows SMB servers.
> > 
> > To workaround this problem fix and improve existing cifs silly rename
> > code and extend it also to SMB2 and SMB3 dialects when communicating
> > with Windows SMB servers. Silly rename is applied only when it is
> > necessary (when some other client has opened file or directory).
> > If no other client has the file / dir open then silly rename is not
> > used.
> 
> If I 'git grep -i silly fs/smb/client' there's no hit, can you
> please explain what code do you mean with silly rename?

Currently (without this patch series) it is CIFSSMBRenameOpenFile()
function when called with NULL as 3rd argument.

Cleanup is done in PATCH 11/35, where are more details.

Originally the "Silly rename" is the term used by NFS client, when it
does rename instead of unlink when the path is in use.
I reused this term.


So for SMB this "silly rename" means:
- open path with DELETE access and get its handle
- rename path (via opened handle) to some unique (auto generated) name
- set delete pending state on the path (via opened handle)
- close handle

(plus some stuff around to remove READ_ONLY attr which may disallow to
open path with DELETE ACCESS)

So above silly rename means that the original path is not occupied
anymore (thanks to rename) and the original file / dir is removed after
all clients / users release handles (thanks to set delete pending).

It is clear now clear? Or do you need to explain some other steps?
Sometimes some parts are too obvious for me and I unintentionally omit
description for something which is important. And seems that this is
such case. So it is my mistake, I should have explain it better.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers
  2025-09-01 17:02   ` Pali Rohár
@ 2025-09-02 15:17     ` Stefan Metzmacher
  2025-09-02 16:30       ` Pali Rohár
  0 siblings, 1 reply; 40+ messages in thread
From: Stefan Metzmacher @ 2025-09-02 15:17 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Steve French, Paulo Alcantara, ronnie sahlberg, Ralph Böhme,
	linux-cifs, linux-kernel

Hi Pali,

>>> This patch series improves Linux rmdir() and unlink() syscalls called on
>>> SMB mounts exported from Windows SMB servers which do not implement
>>> POSIX semantics of the file and directory removal.
>>>
>>> This patch series should have no impact and no function change when
>>> communicating with the POSIX SMB servers, as they should implement
>>> proper rmdir and unlink logic.
>>
>> Please note that even servers implementing posix/unix extensions,
>> may also have windows clients connected operating on the same files/directories.
>> And in that case even posix clients will see the windows behaviour
>> of DELETE_PENDING for set disposition or on rename
>> NT_STATUS_ACCESS_DENIED or NT_STATUS_DIRECTORY_NOT_EMPTY.
> 
> Ok. So does it mean that the issue described here applies also for POSIX
> SMB server?

I guess so.

> If yes, then I would propose to first fix the problem with
> Windows/non-POSIX SMB server and then with others. So it is not too big.

That's up to Steve. But isn't it just a matter of removing the
if statement that checks for posix?

>>> When issuing remove path command against non-POSIX / Windows SMB server,
>>> it let the directory entry which is being removed in the directory until
>>> all users / clients close all handles / references to that path.
>>>
>>> POSIX requires from rmdir() and unlink() syscalls that after successful
>>> call, the requested path / directory entry is released and allows to
>>> create a new file or directory with that name. This is currently not
>>> working against non-POSIX / Windows SMB servers.
>>>
>>> To workaround this problem fix and improve existing cifs silly rename
>>> code and extend it also to SMB2 and SMB3 dialects when communicating
>>> with Windows SMB servers. Silly rename is applied only when it is
>>> necessary (when some other client has opened file or directory).
>>> If no other client has the file / dir open then silly rename is not
>>> used.
>>
>> If I 'git grep -i silly fs/smb/client' there's no hit, can you
>> please explain what code do you mean with silly rename?
> 
> Currently (without this patch series) it is CIFSSMBRenameOpenFile()
> function when called with NULL as 3rd argument.
> 
> Cleanup is done in PATCH 11/35, where are more details.
> 
> Originally the "Silly rename" is the term used by NFS client, when it
> does rename instead of unlink when the path is in use.
> I reused this term.
> 
> 
> So for SMB this "silly rename" means:
> - open path with DELETE access and get its handle
> - rename path (via opened handle) to some unique (auto generated) name
> - set delete pending state on the path (via opened handle)
> - close handle
> 
> (plus some stuff around to remove READ_ONLY attr which may disallow to
> open path with DELETE ACCESS)
> 
> So above silly rename means that the original path is not occupied
> anymore (thanks to rename) and the original file / dir is removed after
> all clients / users release handles (thanks to set delete pending).
> 
> It is clear now clear? Or do you need to explain some other steps?
> Sometimes some parts are too obvious for me and I unintentionally omit
> description for something which is important. And seems that this is
> such case. So it is my mistake, I should have explain it better.

I think I understand what it tries to do, thanks for explaining.

I was just wondering why the rename on a busy handle would work
while delete won't work. I'd guess the chances are high that both fail.

Do you have network captures showing the old and new behavior
that's often easier to understand than looking at patches alone.

metze

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows SMB servers
  2025-09-02 15:17     ` Stefan Metzmacher
@ 2025-09-02 16:30       ` Pali Rohár
  0 siblings, 0 replies; 40+ messages in thread
From: Pali Rohár @ 2025-09-02 16:30 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: Steve French, Paulo Alcantara, ronnie sahlberg, Ralph Böhme,
	linux-cifs, linux-kernel

Hello!

On Tuesday 02 September 2025 17:17:14 Stefan Metzmacher wrote:
> Hi Pali,
> 
> > > > This patch series improves Linux rmdir() and unlink() syscalls called on
> > > > SMB mounts exported from Windows SMB servers which do not implement
> > > > POSIX semantics of the file and directory removal.
> > > > 
> > > > This patch series should have no impact and no function change when
> > > > communicating with the POSIX SMB servers, as they should implement
> > > > proper rmdir and unlink logic.
> > > 
> > > Please note that even servers implementing posix/unix extensions,
> > > may also have windows clients connected operating on the same files/directories.
> > > And in that case even posix clients will see the windows behaviour
> > > of DELETE_PENDING for set disposition or on rename
> > > NT_STATUS_ACCESS_DENIED or NT_STATUS_DIRECTORY_NOT_EMPTY.
> > 
> > Ok. So does it mean that the issue described here applies also for POSIX
> > SMB server?
> 
> I guess so.
> 
> > If yes, then I would propose to first fix the problem with
> > Windows/non-POSIX SMB server and then with others. So it is not too big.
> 
> That's up to Steve. But isn't it just a matter of removing the
> if statement that checks for posix?

I can modify that if statement, no problem. I just did not wanted to
touch POSIX code path as my focus is on the Windows code path.
I do not know all those case which POSIX paths against POSIX server may
trigger, so it was safer for me to let POSIX as is.

> > > > When issuing remove path command against non-POSIX / Windows SMB server,
> > > > it let the directory entry which is being removed in the directory until
> > > > all users / clients close all handles / references to that path.
> > > > 
> > > > POSIX requires from rmdir() and unlink() syscalls that after successful
> > > > call, the requested path / directory entry is released and allows to
> > > > create a new file or directory with that name. This is currently not
> > > > working against non-POSIX / Windows SMB servers.
> > > > 
> > > > To workaround this problem fix and improve existing cifs silly rename
> > > > code and extend it also to SMB2 and SMB3 dialects when communicating
> > > > with Windows SMB servers. Silly rename is applied only when it is
> > > > necessary (when some other client has opened file or directory).
> > > > If no other client has the file / dir open then silly rename is not
> > > > used.
> > > 
> > > If I 'git grep -i silly fs/smb/client' there's no hit, can you
> > > please explain what code do you mean with silly rename?
> > 
> > Currently (without this patch series) it is CIFSSMBRenameOpenFile()
> > function when called with NULL as 3rd argument.
> > 
> > Cleanup is done in PATCH 11/35, where are more details.
> > 
> > Originally the "Silly rename" is the term used by NFS client, when it
> > does rename instead of unlink when the path is in use.
> > I reused this term.
> > 
> > 
> > So for SMB this "silly rename" means:
> > - open path with DELETE access and get its handle
> > - rename path (via opened handle) to some unique (auto generated) name
> > - set delete pending state on the path (via opened handle)
> > - close handle
> > 
> > (plus some stuff around to remove READ_ONLY attr which may disallow to
> > open path with DELETE ACCESS)
> > 
> > So above silly rename means that the original path is not occupied
> > anymore (thanks to rename) and the original file / dir is removed after
> > all clients / users release handles (thanks to set delete pending).
> > 
> > It is clear now clear? Or do you need to explain some other steps?
> > Sometimes some parts are too obvious for me and I unintentionally omit
> > description for something which is important. And seems that this is
> > such case. So it is my mistake, I should have explain it better.
> 
> I think I understand what it tries to do, thanks for explaining.
> 
> I was just wondering why the rename on a busy handle would work
> while delete won't work. I'd guess the chances are high that both fail.

Both "set delete pending" and "rename" operations are working (if open
pass). Just "set delete pending" does not unlink file/dir immediately
but rather wait until path is not busy anymore. "rename" on the other
hand is executed immediately.

So we can rename the in-use/busy file on Windows server, but we cannot
remove it immediately. We can only "schedule" its removal on the Windows
server. So combination of "rename" + "schedule removal" is what are
patches doing.

In case open fails (e.g. due to conflicting DENY shared reservations or
because path is already in delete pending state) then obviously it is
not possible to continue with either rename or set delete pending
operation, both then fails.

> Do you have network captures showing the old and new behavior
> that's often easier to understand than looking at patches alone.
> 
> metze

I do not have them right now, but I can run test scenario and capture
them, this is not problem. Test case is pretty straightforward.

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2025-09-02 16:30 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-08-31 12:35 ` [PATCH 09/35] cifs: Improve SMB1 " Pali Rohár
2025-08-31 12:35 ` [PATCH 10/35] cifs: Improve detect_directory_symlink_target() function Pali Rohár
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-08-31 12:35 ` [PATCH 27/35] cifs: Move SMB1 usage of CIFSPOSIXDelFile() from inode.c to cifssmb.c Pali Rohár
2025-08-31 12:35 ` [PATCH 28/35] cifs: Fix smb2_unlink() to fail on directory Pali Rohár
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).