Commit 6ab409b5 authored by Dave Kleikamp's avatar Dave Kleikamp Committed by Steve French
Browse files

cifs: Replace wrtPending with a real reference count 



Currently, cifs_close() tries to wait until all I/O is complete and then
frees the file private data.  If I/O does not completely in a reasonable
amount of time it frees the structure anyway, leaving a potential use-
after-free situation.

This patch changes the wrtPending counter to a complete reference count and
lets the last user free the structure.

Signed-off-by: default avatarDave Kleikamp <shaggy@linux.vnet.ibm.com>
Reviewed-by: default avatarJeff Layton <jlayton@redhat.com>
Tested-by: default avatarShirish Pargaonkar <shirishp@us.ibm.com>
Signed-off-by: default avatarSteve French <sfrench@us.ibm.com>
parent 1b49c556

fs/cifs/cifsacl.c

+2 −2
Original line number Diff line number Diff line
@@ -607,7 +607,7 @@ static struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb, 
		return get_cifs_acl_by_path(cifs_sb, path, pacllen);



	pntsd = get_cifs_acl_by_fid(cifs_sb, open_file->netfid, pacllen);

	atomic_dec(&open_file->wrtPending);

	cifsFileInfo_put(open_file);

	return pntsd;

}
@@ -665,7 +665,7 @@ static int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, 
		return set_cifs_acl_by_path(cifs_sb, path, pnntsd, acllen);



	rc = set_cifs_acl_by_fid(cifs_sb, open_file->netfid, pnntsd, acllen);

	atomic_dec(&open_file->wrtPending);

	cifsFileInfo_put(open_file);

	return rc;

}

fs/cifs/cifsglob.h

+14 −1
Original line number Diff line number Diff line
@@ -351,11 +351,24 @@ struct cifsFileInfo { 
	bool closePend:1;	/* file is marked to close */

	bool invalidHandle:1;	/* file closed via session abend */

	bool messageMode:1;	/* for pipes: message vs byte mode */

	atomic_t wrtPending;   /* handle in use - defer close */

	atomic_t count;		/* reference count */

	struct mutex fh_mutex; /* prevents reopen race after dead ses*/

	struct cifs_search_info srch_inf;

};



/* Take a reference on the file private data */

static inline void cifsFileInfo_get(struct cifsFileInfo *cifs_file)

{

	atomic_inc(&cifs_file->count);

}



/* Release a reference on the file private data */

static inline void cifsFileInfo_put(struct cifsFileInfo *cifs_file)

{

	if (atomic_dec_and_test(&cifs_file->count))

		kfree(cifs_file);

}



/*

 * One of these for each file inode

 */

fs/cifs/dir.c

+1 −1
Original line number Diff line number Diff line
@@ -153,7 +153,7 @@ cifs_fill_fileinfo(struct inode *newinode, __u16 fileHandle, 
	mutex_init(&pCifsFile->fh_mutex);

	mutex_init(&pCifsFile->lock_mutex);

	INIT_LIST_HEAD(&pCifsFile->llist);

	atomic_set(&pCifsFile->wrtPending, 0);

	atomic_set(&pCifsFile->count, 1);



	/* set the following in open now

			pCifsFile->pfile = file; */

fs/cifs/file.c

+11 −32
Original line number Diff line number Diff line
@@ -53,11 +53,9 @@ static inline struct cifsFileInfo *cifs_init_private( 
	private_data->pInode = inode;

	private_data->invalidHandle = false;

	private_data->closePend = false;

	/* we have to track num writers to the inode, since writepages

	does not tell us which handle the write is for so there can

	be a close (overlapping with write) of the filehandle that

	cifs_writepages chose to use */

	atomic_set(&private_data->wrtPending, 0);

	/* Initialize reference count to one.  The private data is

	freed on the release of the last reference */

	atomic_set(&private_data->count, 1);



	return private_data;

}
@@ -643,7 +641,7 @@ int cifs_close(struct inode *inode, struct file *file) 
			if (!pTcon->need_reconnect) {

				write_unlock(&GlobalSMBSeslock);

				timeout = 2;

				while ((atomic_read(&pSMBFile->wrtPending) != 0)

				while ((atomic_read(&pSMBFile->count) != 1)

					&& (timeout <= 2048)) {

					/* Give write a better chance to get to

					server ahead of the close.  We do not
@@ -657,8 +655,6 @@ int cifs_close(struct inode *inode, struct file *file) 
					msleep(timeout);

					timeout *= 4;

				}

				if (atomic_read(&pSMBFile->wrtPending))

					cERROR(1, ("close with pending write"));

				if (!pTcon->need_reconnect &&

				    !pSMBFile->invalidHandle)

					rc = CIFSSMBClose(xid, pTcon,
@@ -681,24 +677,7 @@ int cifs_close(struct inode *inode, struct file *file) 
		list_del(&pSMBFile->flist);

		list_del(&pSMBFile->tlist);

		write_unlock(&GlobalSMBSeslock);

		timeout = 10;

		/* We waited above to give the SMBWrite a chance to issue

		   on the wire (so we do not get SMBWrite returning EBADF

		   if writepages is racing with close.  Note that writepages

		   does not specify a file handle, so it is possible for a file

		   to be opened twice, and the application close the "wrong"

		   file handle - in these cases we delay long enough to allow

		   the SMBWrite to get on the wire before the SMB Close.

		   We allow total wait here over 45 seconds, more than

		   oplock break time, and more than enough to allow any write

		   to complete on the server, or to time out on the client */

		while ((atomic_read(&pSMBFile->wrtPending) != 0)

				&& (timeout <= 50000)) {

			cERROR(1, ("writes pending, delay free of handle"));

			msleep(timeout);

			timeout *= 8;

		}

		kfree(file->private_data);

		cifsFileInfo_put(file->private_data);

		file->private_data = NULL;

	} else

		rc = -EBADF;
@@ -1236,7 +1215,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode) 
			if (!open_file->invalidHandle) {

				/* found a good file */

				/* lock it so it will not be closed on us */

				atomic_inc(&open_file->wrtPending);

				cifsFileInfo_get(open_file);

				read_unlock(&GlobalSMBSeslock);

				return open_file;

			} /* else might as well continue, and look for
@@ -1276,7 +1255,7 @@ refind_writable: 
		if (open_file->pfile &&

		    ((open_file->pfile->f_flags & O_RDWR) ||

		     (open_file->pfile->f_flags & O_WRONLY))) {

			atomic_inc(&open_file->wrtPending);

			cifsFileInfo_get(open_file);



			if (!open_file->invalidHandle) {

				/* found a good writable file */
@@ -1293,7 +1272,7 @@ refind_writable: 
				else { /* start over in case this was deleted */

				       /* since the list could be modified */

					read_lock(&GlobalSMBSeslock);

					atomic_dec(&open_file->wrtPending);

					cifsFileInfo_put(open_file);

					goto refind_writable;

				}

			}
@@ -1309,7 +1288,7 @@ refind_writable: 
			read_lock(&GlobalSMBSeslock);

			/* can not use this handle, no write

			   pending on this one after all */

			atomic_dec(&open_file->wrtPending);

			cifsFileInfo_put(open_file);



			if (open_file->closePend) /* list could have changed */

				goto refind_writable;
@@ -1373,7 +1352,7 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to) 
	if (open_file) {

		bytes_written = cifs_write(open_file->pfile, write_data,

					   to-from, &offset);

		atomic_dec(&open_file->wrtPending);

		cifsFileInfo_put(open_file);

		/* Does mm or vfs already set times? */

		inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb);

		if ((bytes_written > 0) && (offset))
@@ -1562,7 +1541,7 @@ retry: 
						   bytes_to_write, offset,

						   &bytes_written, iov, n_iov,

						   long_op);

				atomic_dec(&open_file->wrtPending);

				cifsFileInfo_put(open_file);

				cifs_update_eof(cifsi, offset, bytes_written);



				if (rc || bytes_written < bytes_to_write) {

fs/cifs/inode.c

+3 −3
Original line number Diff line number Diff line
@@ -800,7 +800,7 @@ set_via_filehandle: 
	if (open_file == NULL)

		CIFSSMBClose(xid, pTcon, netfid);

	else

		atomic_dec(&open_file->wrtPending);

		cifsFileInfo_put(open_file);

out:

	return rc;

}
@@ -1635,7 +1635,7 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, 
		__u32 npid = open_file->pid;

		rc = CIFSSMBSetFileSize(xid, pTcon, attrs->ia_size, nfid,

					npid, false);

		atomic_dec(&open_file->wrtPending);

		cifsFileInfo_put(open_file);

		cFYI(1, ("SetFSize for attrs rc = %d", rc));

		if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {

			unsigned int bytes_written;
@@ -1790,7 +1790,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) 
		u16 nfid = open_file->netfid;

		u32 npid = open_file->pid;

		rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid);

		atomic_dec(&open_file->wrtPending);

		cifsFileInfo_put(open_file);

	} else {

		rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,

				    cifs_sb->local_nls,

CRA Git | Maintained and supported by SUSTech CRA and CCSE