Commit 40e75e42 authored by Paulo Alcantara's avatar Paulo Alcantara Committed by Steve French
Browse files

smb: client: fix open handle lookup in cifs_open()



When looking up open handles to be re-used in cifs_open(), calling
cifs_get_{writable,readable}_path() is wrong as it will look up for
the first matching open handle, and if @file->f_flags doesn't match,
it will ignore the remaining open handles in
cifsInodeInfo::openFileList that might potentially match
@file->f_flags.

For writable and readable handles, fix this by calling
__cifs_get_writable_file() and __find_readable_file(), respectively,
with FIND_OPEN_FLAGS set.

With the patch, the following program ends up with two opens instead
of three sent over the wire.

```
  #define _GNU_SOURCE
  #include <unistd.h>
  #include <string.h>
  #include <fcntl.h>

  int main(int argc, char *argv[])
  {
          int fd;

          fd = open("/mnt/1/foo", O_CREAT | O_WRONLY | O_TRUNC, 0664);
          close(fd);
          fd = open("/mnt/1/foo", O_DIRECT | O_WRONLY);
          close(fd);
          fd = open("/mnt/1/foo", O_WRONLY);
          close(fd);
          fd = open("/mnt/1/foo", O_DIRECT | O_WRONLY);
          close(fd);
          return 0;
  }
```

```
$ mount.cifs //srv/share /mnt/1 -o ...
$ gcc test.c && ./a.out
```

Signed-off-by: default avatarPaulo Alcantara (Red Hat) <pc@manguebit.org>
Reviewed-by: default avatarChenXiaoSong <chenxiaosong@kylinos.cn>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent d4c7210d
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -1489,7 +1489,7 @@ struct smb_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
	struct cifsFileInfo *open_file = NULL;

	if (inode)
		open_file = find_readable_file(CIFS_I(inode), true);
		open_file = find_readable_file(CIFS_I(inode), FIND_FSUID_ONLY);
	if (!open_file)
		return get_cifs_acl_by_path(cifs_sb, path, pacllen, info);

+1 −1
Original line number Diff line number Diff line
@@ -1269,7 +1269,7 @@ static int cifs_precopy_set_eof(struct inode *src_inode, struct cifsInodeInfo *s
	struct cifsFileInfo *writeable_srcfile;
	int rc = -EINVAL;

	writeable_srcfile = find_writable_file(src_cifsi, FIND_WR_FSUID_ONLY);
	writeable_srcfile = find_writable_file(src_cifsi, FIND_FSUID_ONLY);
	if (writeable_srcfile) {
		if (src_tcon->ses->server->ops->set_file_size)
			rc = src_tcon->ses->server->ops->set_file_size(
+6 −6
Original line number Diff line number Diff line
@@ -1885,12 +1885,12 @@ static inline bool is_replayable_error(int error)
}


/* cifs_get_writable_file() flags */
enum cifs_writable_file_flags {
	FIND_WR_ANY			= 0U,
	FIND_WR_FSUID_ONLY		= (1U << 0),
	FIND_WR_WITH_DELETE		= (1U << 1),
	FIND_WR_NO_PENDING_DELETE	= (1U << 2),
enum cifs_find_flags {
	FIND_ANY		= 0U,
	FIND_FSUID_ONLY		= (1U << 0),
	FIND_WITH_DELETE	= (1U << 1),
	FIND_NO_PENDING_DELETE	= (1U << 2),
	FIND_OPEN_FLAGS		= (1U << 3),
};

#define   MID_FREE 0
+22 −4
Original line number Diff line number Diff line
@@ -138,12 +138,14 @@ void cifs_write_subrequest_terminated(struct cifs_io_subrequest *wdata,
				      ssize_t result);
struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
					int flags);
int cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, int flags,
int __cifs_get_writable_file(struct cifsInodeInfo *cifs_inode,
			     unsigned int find_flags, unsigned int open_flags,
			     struct cifsFileInfo **ret_file);
int cifs_get_writable_path(struct cifs_tcon *tcon, const char *name, int flags,
			   struct cifsFileInfo **ret_file);
struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
					bool fsuid_only);
struct cifsFileInfo *__find_readable_file(struct cifsInodeInfo *cifs_inode,
					  unsigned int find_flags,
					  unsigned int open_flags);
int cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
			   struct cifsFileInfo **ret_file);
int cifs_get_hardlink_path(struct cifs_tcon *tcon, struct inode *inode,
@@ -596,4 +598,20 @@ static inline void cifs_sg_set_buf(struct sg_table *sgtable,
	}
}

static inline int cifs_get_writable_file(struct cifsInodeInfo *cifs_inode,
					 unsigned int find_flags,
					 struct cifsFileInfo **ret_file)
{
	find_flags &= ~FIND_OPEN_FLAGS;
	return __cifs_get_writable_file(cifs_inode, find_flags, 0, ret_file);
}

static inline struct cifsFileInfo *
find_readable_file(struct cifsInodeInfo *cinode, unsigned int find_flags)
{
	find_flags &= ~FIND_OPEN_FLAGS;
	find_flags |= FIND_NO_PENDING_DELETE;
	return __find_readable_file(cinode, find_flags, 0);
}

#endif			/* _CIFSPROTO_H */
+67 −46
Original line number Diff line number Diff line
@@ -255,7 +255,7 @@ static void cifs_begin_writeback(struct netfs_io_request *wreq)
	struct cifs_io_request *req = container_of(wreq, struct cifs_io_request, rreq);
	int ret;

	ret = cifs_get_writable_file(CIFS_I(wreq->inode), FIND_WR_ANY, &req->cfile);
	ret = cifs_get_writable_file(CIFS_I(wreq->inode), FIND_ANY, &req->cfile);
	if (ret) {
		cifs_dbg(VFS, "No writable handle in writepages ret=%d\n", ret);
		return;
@@ -956,7 +956,7 @@ int cifs_file_flush(const unsigned int xid, struct inode *inode,
		return tcon->ses->server->ops->flush(xid, tcon,
						     &cfile->fid);
	}
	rc = cifs_get_writable_file(CIFS_I(inode), FIND_WR_ANY, &cfile);
	rc = cifs_get_writable_file(CIFS_I(inode), FIND_ANY, &cfile);
	if (!rc) {
		tcon = tlink_tcon(cfile->tlink);
		rc = tcon->ses->server->ops->flush(xid, tcon, &cfile->fid);
@@ -981,7 +981,7 @@ static int cifs_do_truncate(const unsigned int xid, struct dentry *dentry)
		return -ERESTARTSYS;
	mapping_set_error(inode->i_mapping, rc);

	cfile = find_writable_file(cinode, FIND_WR_FSUID_ONLY);
	cfile = find_writable_file(cinode, FIND_FSUID_ONLY);
	rc = cifs_file_flush(xid, inode, cfile);
	if (!rc) {
		if (cfile) {
@@ -1061,32 +1061,29 @@ int cifs_open(struct inode *inode, struct file *file)

	/* Get the cached handle as SMB2 close is deferred */
	if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) {
		rc = cifs_get_writable_path(tcon, full_path,
					    FIND_WR_FSUID_ONLY |
					    FIND_WR_NO_PENDING_DELETE,
					    &cfile);
		rc = __cifs_get_writable_file(CIFS_I(inode),
					      FIND_FSUID_ONLY |
					      FIND_NO_PENDING_DELETE |
					      FIND_OPEN_FLAGS,
					      file->f_flags, &cfile);
	} else {
		rc = cifs_get_readable_path(tcon, full_path, &cfile);
		cfile = __find_readable_file(CIFS_I(inode),
					     FIND_NO_PENDING_DELETE |
					     FIND_OPEN_FLAGS,
					     file->f_flags);
		rc = cfile ? 0 : -ENOENT;
	}
	if (rc == 0) {
		unsigned int oflags = file->f_flags & ~(O_CREAT|O_EXCL|O_TRUNC);
		unsigned int cflags = cfile->f_flags & ~(O_CREAT|O_EXCL|O_TRUNC);

		if (cifs_convert_flags(oflags, 0) == cifs_convert_flags(cflags, 0) &&
		    (oflags & (O_SYNC|O_DIRECT)) == (cflags & (O_SYNC|O_DIRECT))) {
		file->private_data = cfile;
		spin_lock(&CIFS_I(inode)->deferred_lock);
		cifs_del_deferred_close(cfile);
		spin_unlock(&CIFS_I(inode)->deferred_lock);
		goto use_cache;
	}
		_cifsFileInfo_put(cfile, true, false);
	} else {
		/* hard link on the defeered close file */
	/* hard link on the deferred close file */
	rc = cifs_get_hardlink_path(tcon, inode, file);
	if (rc)
		cifs_close_deferred_file(CIFS_I(inode));
	}

	if (server->oplocks)
		oplock = REQ_OPLOCK;
@@ -2512,10 +2509,33 @@ void cifs_write_subrequest_terminated(struct cifs_io_subrequest *wdata, ssize_t
	netfs_write_subrequest_terminated(&wdata->subreq, result);
}

struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
					bool fsuid_only)
static bool open_flags_match(struct cifsInodeInfo *cinode,
			     unsigned int oflags, unsigned int cflags)
{
	struct inode *inode = &cinode->netfs.inode;
	int crw = 0, orw = 0;

	oflags &= ~(O_CREAT | O_EXCL | O_TRUNC);
	cflags &= ~(O_CREAT | O_EXCL | O_TRUNC);

	if (cifs_fscache_enabled(inode)) {
		if (OPEN_FMODE(cflags) & FMODE_WRITE)
			crw = 1;
		if (OPEN_FMODE(oflags) & FMODE_WRITE)
			orw = 1;
	}
	if (cifs_convert_flags(oflags, orw) != cifs_convert_flags(cflags, crw))
		return false;

	return (oflags & (O_SYNC | O_DIRECT)) == (cflags & (O_SYNC | O_DIRECT));
}

struct cifsFileInfo *__find_readable_file(struct cifsInodeInfo *cifs_inode,
					  unsigned int find_flags,
					  unsigned int open_flags)
{
	struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_inode);
	bool fsuid_only = find_flags & FIND_FSUID_ONLY;
	struct cifsFileInfo *open_file = NULL;

	/* only filter by fsuid on multiuser mounts */
@@ -2529,6 +2549,13 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
	list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
		if (fsuid_only && !uid_eq(open_file->uid, current_fsuid()))
			continue;
		if ((find_flags & FIND_NO_PENDING_DELETE) &&
		    open_file->status_file_deleted)
			continue;
		if ((find_flags & FIND_OPEN_FLAGS) &&
		    !open_flags_match(cifs_inode, open_flags,
				      open_file->f_flags))
			continue;
		if (OPEN_FMODE(open_file->f_flags) & FMODE_READ) {
			if ((!open_file->invalidHandle)) {
				/* found a good file */
@@ -2547,8 +2574,8 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
}

/* Return -EBADF if no handle is found and general rc otherwise */
int
cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, int flags,
int __cifs_get_writable_file(struct cifsInodeInfo *cifs_inode,
			     unsigned int find_flags, unsigned int open_flags,
			     struct cifsFileInfo **ret_file)
{
	struct cifsFileInfo *open_file, *inv_file = NULL;
@@ -2556,8 +2583,8 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, int flags,
	bool any_available = false;
	int rc = -EBADF;
	unsigned int refind = 0;
	bool fsuid_only = flags & FIND_WR_FSUID_ONLY;
	bool with_delete = flags & FIND_WR_WITH_DELETE;
	bool fsuid_only = find_flags & FIND_FSUID_ONLY;
	bool with_delete = find_flags & FIND_WITH_DELETE;
	*ret_file = NULL;

	/*
@@ -2591,9 +2618,13 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, int flags,
			continue;
		if (with_delete && !(open_file->fid.access & DELETE))
			continue;
		if ((flags & FIND_WR_NO_PENDING_DELETE) &&
		if ((find_flags & FIND_NO_PENDING_DELETE) &&
		    open_file->status_file_deleted)
			continue;
		if ((find_flags & FIND_OPEN_FLAGS) &&
		    !open_flags_match(cifs_inode, open_flags,
				      open_file->f_flags))
			continue;
		if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
			if (!open_file->invalidHandle) {
				/* found a good writable file */
@@ -2710,17 +2741,7 @@ cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
		cinode = CIFS_I(d_inode(cfile->dentry));
		spin_unlock(&tcon->open_file_lock);
		free_dentry_path(page);
		*ret_file = find_readable_file(cinode, 0);
		if (*ret_file) {
			spin_lock(&cinode->open_file_lock);
			if ((*ret_file)->status_file_deleted) {
				spin_unlock(&cinode->open_file_lock);
				cifsFileInfo_put(*ret_file);
				*ret_file = NULL;
			} else {
				spin_unlock(&cinode->open_file_lock);
			}
		}
		*ret_file = find_readable_file(cinode, FIND_ANY);
		return *ret_file ? 0 : -ENOENT;
	}

@@ -2792,7 +2813,7 @@ int cifs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
		}

		if ((OPEN_FMODE(smbfile->f_flags) & FMODE_WRITE) == 0) {
			smbfile = find_writable_file(CIFS_I(inode), FIND_WR_ANY);
			smbfile = find_writable_file(CIFS_I(inode), FIND_ANY);
			if (smbfile) {
				rc = server->ops->flush(xid, tcon, &smbfile->fid);
				cifsFileInfo_put(smbfile);
Loading