Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] cifs: create a helper to find a writeable handle py path name
@ 2019-08-27  1:35 Ronnie Sahlberg
  2019-08-27  1:35 ` [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root Ronnie Sahlberg
  0 siblings, 1 reply; 5+ messages in thread
From: Ronnie Sahlberg @ 2019-08-27  1:35 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French

rename() takes a path for old_file and in SMB2 we used to just create
a compound for create(old_path)/rename/close().
If we already have a writable handle we can avoid the create() and close()
altogether and just use the existing handle.

For this situation, as we avoid doing the create()
we also avoid triggering an oplock break for the existing handle.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsproto.h |  2 ++
 fs/cifs/file.c      | 35 ++++++++++++++++++++++
 fs/cifs/smb2inode.c | 83 +++++++++++++++++++++++++++++++++++++----------------
 3 files changed, 96 insertions(+), 24 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 592a6cea2b79..be206744407c 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -137,6 +137,8 @@ extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, bool);
 extern int cifs_get_writable_file(struct cifsInodeInfo *cifs_inode,
 				  bool fsuid_only,
 				  struct cifsFileInfo **ret_file);
+extern int cifs_get_writable_path(struct cifs_tcon *tcon, const char *name,
+				  struct cifsFileInfo **ret_file);
 extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool);
 extern unsigned int smbCalcSize(void *buf, struct TCP_Server_Info *server);
 extern int decode_negTokenInit(unsigned char *security_blob, int length,
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 97090693d182..b67f17b06bcd 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1980,6 +1980,41 @@ find_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only)
 	return cfile;
 }
 
+int
+cifs_get_writable_path(struct cifs_tcon *tcon, const char *name,
+		       struct cifsFileInfo **ret_file)
+{
+	struct list_head *tmp;
+	struct cifsFileInfo *cfile;
+	struct cifsInodeInfo *cinode;
+	char *full_path;
+
+	*ret_file = NULL;
+
+	spin_lock(&tcon->open_file_lock);
+	list_for_each(tmp, &tcon->openFileList) {
+		cfile = list_entry(tmp, struct cifsFileInfo,
+			     tlist);
+		full_path = build_path_from_dentry(cfile->dentry);
+		if (full_path == NULL) {
+			spin_unlock(&tcon->open_file_lock);
+			return -ENOMEM;
+		}
+		if (strcmp(full_path, name)) {
+			kfree(full_path);
+			continue;
+		}
+		kfree(full_path);
+
+		cinode = CIFS_I(d_inode(cfile->dentry));
+		spin_unlock(&tcon->open_file_lock);
+		return cifs_get_writable_file(cinode, 0, ret_file);
+	}
+
+	spin_unlock(&tcon->open_file_lock);
+	return -ENOENT;
+}
+
 static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
 {
 	struct address_space *mapping = page->mapping;
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index d8d9cdfa30b6..b4dbbf7dfa2d 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -51,7 +51,8 @@ static int
 smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 		 struct cifs_sb_info *cifs_sb, const char *full_path,
 		 __u32 desired_access, __u32 create_disposition,
-		 __u32 create_options, void *ptr, int command)
+		 __u32 create_options, void *ptr, int command,
+		 struct cifsFileInfo *cfile)
 {
 	int rc;
 	__le16 *utf16_path = NULL;
@@ -83,6 +84,10 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 	resp_buftype[0] = resp_buftype[1] = resp_buftype[2] = CIFS_NO_BUFFER;
 	memset(rsp_iov, 0, sizeof(rsp_iov));
 
+	/* We already have a handle so we can skip the open */
+	if (cfile)
+		goto after_open;
+
 	/* Open */
 	utf16_path = cifs_convert_path_to_utf16(full_path, cifs_sb);
 	if (!utf16_path)
@@ -106,7 +111,9 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 	if (rc)
 		goto finished;
 
-	smb2_set_next_command(tcon, &rqst[num_rqst++]);
+	smb2_set_next_command(tcon, &rqst[num_rqst]);
+ after_open:
+	num_rqst++;
 
 	/* Operation */
 	switch (command) {
@@ -210,14 +217,24 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 		size[1] = len + 2 /* null */;
 		data[1] = (__le16 *)ptr;
 
-		rc = SMB2_set_info_init(tcon, &rqst[num_rqst], COMPOUND_FID,
-					COMPOUND_FID, current->tgid,
-					FILE_RENAME_INFORMATION,
+		if (cfile) {
+			rc = SMB2_set_info_init(tcon, &rqst[num_rqst],
+						cfile->fid.persistent_fid,
+						cfile->fid.volatile_fid,
+					current->tgid, FILE_RENAME_INFORMATION,
 					SMB2_O_INFO_FILE, 0, data, size);
+			cifsFileInfo_put(cfile);
+		} else {
+			rc = SMB2_set_info_init(tcon, &rqst[num_rqst],
+					COMPOUND_FID, COMPOUND_FID,
+					current->tgid, FILE_RENAME_INFORMATION,
+					SMB2_O_INFO_FILE, 0, data, size);
+			smb2_set_next_command(tcon, &rqst[num_rqst]);
+			smb2_set_related(&rqst[num_rqst]);
+		}
 		if (rc)
 			goto finished;
-		smb2_set_next_command(tcon, &rqst[num_rqst]);
-		smb2_set_related(&rqst[num_rqst++]);
+		num_rqst++;
 		trace_smb3_rename_enter(xid, ses->Suid, tcon->tid, full_path);
 		break;
 	case SMB2_OP_HARDLINK:
@@ -254,18 +271,29 @@ smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 	if (rc)
 		goto finished;
 
+	/* We already have a handle so we can skip the close */
+	if (cfile)
+		goto after_close;
 	/* Close */
 	memset(&close_iov, 0, sizeof(close_iov));
 	rqst[num_rqst].rq_iov = close_iov;
 	rqst[num_rqst].rq_nvec = 1;
 	rc = SMB2_close_init(tcon, &rqst[num_rqst], COMPOUND_FID,
 			     COMPOUND_FID);
-	smb2_set_related(&rqst[num_rqst++]);
+	smb2_set_related(&rqst[num_rqst]);
 	if (rc)
 		goto finished;
-
-	rc = compound_send_recv(xid, ses, flags, num_rqst, rqst,
-				resp_buftype, rsp_iov);
+ after_close:
+	num_rqst++;
+
+	if (cfile)
+		rc = compound_send_recv(xid, ses, flags, num_rqst - 2,
+					&rqst[1], &resp_buftype[1],
+					&rsp_iov[1]);
+	else
+		rc = compound_send_recv(xid, ses, flags, num_rqst,
+					rqst, resp_buftype,
+					rsp_iov);
 
  finished:
 	SMB2_open_free(&rqst[0]);
@@ -404,7 +432,7 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
 			      FILE_READ_ATTRIBUTES, FILE_OPEN, create_options,
-			      smb2_data, SMB2_OP_QUERY_INFO);
+			      smb2_data, SMB2_OP_QUERY_INFO, NULL);
 	if (rc == -EOPNOTSUPP) {
 		*symlink = true;
 		create_options |= OPEN_REPARSE_POINT;
@@ -413,7 +441,7 @@ smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
 		rc = smb2_compound_op(xid, tcon, cifs_sb, full_path,
 				      FILE_READ_ATTRIBUTES, FILE_OPEN,
 				      create_options, smb2_data,
-				      SMB2_OP_QUERY_INFO);
+				      SMB2_OP_QUERY_INFO, NULL);
 	}
 	if (rc)
 		goto out;
@@ -430,7 +458,7 @@ smb2_mkdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 {
 	return smb2_compound_op(xid, tcon, cifs_sb, name,
 				FILE_WRITE_ATTRIBUTES, FILE_CREATE,
-				CREATE_NOT_FILE, NULL, SMB2_OP_MKDIR);
+				CREATE_NOT_FILE, NULL, SMB2_OP_MKDIR, NULL);
 }
 
 void
@@ -449,7 +477,8 @@ smb2_mkdir_setinfo(struct inode *inode, const char *name,
 	data.Attributes = cpu_to_le32(dosattrs);
 	tmprc = smb2_compound_op(xid, tcon, cifs_sb, name,
 				 FILE_WRITE_ATTRIBUTES, FILE_CREATE,
-				 CREATE_NOT_FILE, &data, SMB2_OP_SET_INFO);
+				 CREATE_NOT_FILE, &data, SMB2_OP_SET_INFO,
+				 NULL);
 	if (tmprc == 0)
 		cifs_i->cifsAttrs = dosattrs;
 }
@@ -460,7 +489,7 @@ smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 {
 	return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN,
 				CREATE_NOT_FILE,
-				NULL, SMB2_OP_RMDIR);
+				NULL, SMB2_OP_RMDIR, NULL);
 }
 
 int
@@ -469,13 +498,14 @@ smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
 {
 	return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN,
 				CREATE_DELETE_ON_CLOSE | OPEN_REPARSE_POINT,
-				NULL, SMB2_OP_DELETE);
+				NULL, SMB2_OP_DELETE, NULL);
 }
 
 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, __u32 access, int command)
+		   struct cifs_sb_info *cifs_sb, __u32 access, int command,
+		   struct cifsFileInfo *cfile)
 {
 	__le16 *smb2_to_name = NULL;
 	int rc;
@@ -486,7 +516,7 @@ smb2_set_path_attr(const unsigned int xid, struct cifs_tcon *tcon,
 		goto smb2_rename_path;
 	}
 	rc = smb2_compound_op(xid, tcon, cifs_sb, from_name, access,
-			      FILE_OPEN, 0, smb2_to_name, command);
+			      FILE_OPEN, 0, smb2_to_name, command, cfile);
 smb2_rename_path:
 	kfree(smb2_to_name);
 	return rc;
@@ -497,8 +527,12 @@ smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon,
 		 const char *from_name, const char *to_name,
 		 struct cifs_sb_info *cifs_sb)
 {
-	return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb,
-				  DELETE, SMB2_OP_RENAME);
+	struct cifsFileInfo *cfile;
+
+	cifs_get_writable_path(tcon, from_name, &cfile);
+
+	return smb2_set_path_attr(xid, tcon, from_name, to_name,
+				  cifs_sb, DELETE, SMB2_OP_RENAME, cfile);
 }
 
 int
@@ -507,7 +541,8 @@ smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon,
 		     struct cifs_sb_info *cifs_sb)
 {
 	return smb2_set_path_attr(xid, tcon, from_name, to_name, cifs_sb,
-				  FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK);
+				  FILE_READ_ATTRIBUTES, SMB2_OP_HARDLINK,
+				  NULL);
 }
 
 int
@@ -519,7 +554,7 @@ smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon,
 
 	return smb2_compound_op(xid, tcon, cifs_sb, full_path,
 				FILE_WRITE_DATA, FILE_OPEN, 0, &eof,
-				SMB2_OP_SET_EOF);
+				SMB2_OP_SET_EOF, NULL);
 }
 
 int
@@ -541,7 +576,7 @@ smb2_set_file_info(struct inode *inode, const char *full_path,
 
 	rc = smb2_compound_op(xid, tlink_tcon(tlink), cifs_sb, full_path,
 			      FILE_WRITE_ATTRIBUTES, FILE_OPEN, 0, buf,
-			      SMB2_OP_SET_INFO);
+			      SMB2_OP_SET_INFO, NULL);
 	cifs_put_tlink(tlink);
 	return rc;
 }
-- 
2.13.6


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

* [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root
  2019-08-27  1:35 [PATCH 1/2] cifs: create a helper to find a writeable handle py path name Ronnie Sahlberg
@ 2019-08-27  1:35 ` Ronnie Sahlberg
  2019-08-27  2:14   ` Steve French
  2019-08-27 23:00   ` Steve French
  0 siblings, 2 replies; 5+ messages in thread
From: Ronnie Sahlberg @ 2019-08-27  1:35 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/connect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 1ed449f4a8ec..c5dc8265b671 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -4232,7 +4232,7 @@ build_unc_path_to_root(const struct smb_vol *vol,
 	unsigned int unc_len = strnlen(vol->UNC, MAX_TREE_SIZE + 1);
 
 	if (unc_len > MAX_TREE_SIZE)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	full_path = kmalloc(unc_len + pplen + 1, GFP_KERNEL);
 	if (full_path == NULL)
-- 
2.13.6


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

* Re: [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root
  2019-08-27  1:35 ` [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root Ronnie Sahlberg
@ 2019-08-27  2:14   ` Steve French
  2019-08-27  2:20     ` ronnie sahlberg
  2019-08-27 23:00   ` Steve French
  1 sibling, 1 reply; 5+ messages in thread
From: Steve French @ 2019-08-27  2:14 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: linux-cifs

Do you have a simple repro for this one that would lead us to want to cc:stable

On Mon, Aug 26, 2019 at 8:36 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/connect.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1ed449f4a8ec..c5dc8265b671 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -4232,7 +4232,7 @@ build_unc_path_to_root(const struct smb_vol *vol,
>         unsigned int unc_len = strnlen(vol->UNC, MAX_TREE_SIZE + 1);
>
>         if (unc_len > MAX_TREE_SIZE)
> -               return -EINVAL;
> +               return ERR_PTR(-EINVAL);
>
>         full_path = kmalloc(unc_len + pplen + 1, GFP_KERNEL);
>         if (full_path == NULL)
> --
> 2.13.6
>


-- 
Thanks,

Steve

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

* Re: [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root
  2019-08-27  2:14   ` Steve French
@ 2019-08-27  2:20     ` ronnie sahlberg
  0 siblings, 0 replies; 5+ messages in thread
From: ronnie sahlberg @ 2019-08-27  2:20 UTC (permalink / raw)
  To: Steve French; +Cc: Ronnie Sahlberg, linux-cifs

On Tue, Aug 27, 2019 at 12:15 PM Steve French <smfrench@gmail.com> wrote:
>
> Do you have a simple repro for this one that would lead us to want to cc:stable

I don't have a reproducer.

>
> On Mon, Aug 26, 2019 at 8:36 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/connect.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 1ed449f4a8ec..c5dc8265b671 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -4232,7 +4232,7 @@ build_unc_path_to_root(const struct smb_vol *vol,
> >         unsigned int unc_len = strnlen(vol->UNC, MAX_TREE_SIZE + 1);
> >
> >         if (unc_len > MAX_TREE_SIZE)
> > -               return -EINVAL;
> > +               return ERR_PTR(-EINVAL);
> >
> >         full_path = kmalloc(unc_len + pplen + 1, GFP_KERNEL);
> >         if (full_path == NULL)
> > --
> > 2.13.6
> >
>
>
> --
> Thanks,
>
> Steve

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

* Re: [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root
  2019-08-27  1:35 ` [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root Ronnie Sahlberg
  2019-08-27  2:14   ` Steve French
@ 2019-08-27 23:00   ` Steve French
  1 sibling, 0 replies; 5+ messages in thread
From: Steve French @ 2019-08-27 23:00 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: linux-cifs

I fixed this in an earlier patch of yours

On Mon, Aug 26, 2019 at 8:36 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/connect.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1ed449f4a8ec..c5dc8265b671 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -4232,7 +4232,7 @@ build_unc_path_to_root(const struct smb_vol *vol,
>         unsigned int unc_len = strnlen(vol->UNC, MAX_TREE_SIZE + 1);
>
>         if (unc_len > MAX_TREE_SIZE)
> -               return -EINVAL;
> +               return ERR_PTR(-EINVAL);
>
>         full_path = kmalloc(unc_len + pplen + 1, GFP_KERNEL);
>         if (full_path == NULL)
> --
> 2.13.6
>


-- 
Thanks,

Steve

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27  1:35 [PATCH 1/2] cifs: create a helper to find a writeable handle py path name Ronnie Sahlberg
2019-08-27  1:35 ` [PATCH 2/2] cifs: fix incorrect error return in build_unc_path_to_root Ronnie Sahlberg
2019-08-27  2:14   ` Steve French
2019-08-27  2:20     ` ronnie sahlberg
2019-08-27 23:00   ` Steve French

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org linux-cifs@archiver.kernel.org
	public-inbox-index linux-cifs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox