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: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
Tested-by: Shirish Pargaonkar <shirishp@us.ibm.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>

1 files changed, 11 insertions, 32 deletions

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c34b7f8a217b..fa7beac8b80e 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -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) {