mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Steve French <sfrench@samba.org>,
	Paulo Alcantara <pc@manguebit.com>,
	ronnie sahlberg <ronniesahlberg@gmail.com>
Cc: linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 20/35] cifs: Fix permission logic in cifs_rename_pending_delete()
Date: Sun, 31 Aug 2025 14:35:47 +0200	[thread overview]
Message-ID: <20250831123602.14037-21-pali@kernel.org> (raw)
In-Reply-To: <20250831123602.14037-1-pali@kernel.org>

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


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

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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20250831123602.14037-21-pali@kernel.org \
    --to=pali@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pc@manguebit.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sfrench@samba.org \
    /path/to/YOUR_REPLY

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

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

all inboxes | Powered by JetHome®