cifs: pass instantiated filp back after open call

The current scheme of sticking open files on a list and assuming that
cifs_open will scoop them off of it is broken and leads to "Busy
inodes after umount..." errors at unmount time.

The problem is that there is no guarantee that cifs_open will always
be called after a ->lookup or ->create operation. If there are
permissions or other problems, then it's quite likely that it *won't*
be called.

Fix this by fully instantiating the filp whenever the file is created
and pass that filp back to the VFS. If there is a problem, the VFS
can clean up the references.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-and-Tested-by: Suresh Jayaraman <sjayaraman@suse.de>
This commit is contained in:
Jeff Layton 2010-06-16 13:40:16 -04:00
parent 2422f676fb
commit 6ca9f3bae8
2 changed files with 29 additions and 50 deletions

View File

@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <linux/namei.h>
#include <linux/mount.h>
#include <linux/file.h>
#include "cifsfs.h"
#include "cifspdu.h"
#include "cifsglob.h"
@ -184,6 +185,8 @@ cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle,
}
write_unlock(&GlobalSMBSeslock);
file->private_data = pCifsFile;
return pCifsFile;
}
@ -463,14 +466,22 @@ cifs_create_set_dentry:
if (newinode && nd && (nd->flags & LOOKUP_OPEN)) {
struct cifsFileInfo *pfile_info;
/*
* cifs_fill_filedata() takes care of setting cifsFileInfo
* pointer to file->private_data.
*/
pfile_info = cifs_new_fileinfo(newinode, fileHandle, NULL,
struct file *filp;
filp = lookup_instantiate_filp(nd, direntry, generic_file_open);
if (IS_ERR(filp)) {
rc = PTR_ERR(filp);
CIFSSMBClose(xid, tcon, fileHandle);
goto cifs_create_out;
}
pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp,
nd->path.mnt, oflags);
if (pfile_info == NULL)
if (pfile_info == NULL) {
fput(filp);
CIFSSMBClose(xid, tcon, fileHandle);
rc = -ENOMEM;
}
} else {
CIFSSMBClose(xid, tcon, fileHandle);
}
@ -717,15 +728,23 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
direntry->d_op = &cifs_dentry_ops;
d_add(direntry, newInode);
if (posix_open) {
cfile = cifs_new_fileinfo(newInode, fileHandle, NULL,
filp = lookup_instantiate_filp(nd, direntry,
generic_file_open);
if (IS_ERR(filp)) {
rc = PTR_ERR(filp);
CIFSSMBClose(xid, pTcon, fileHandle);
goto lookup_out;
}
cfile = cifs_new_fileinfo(newInode, fileHandle, filp,
nd->path.mnt,
nd->intent.open.flags);
if (cfile == NULL) {
fput(filp);
CIFSSMBClose(xid, pTcon, fileHandle);
rc = -ENOMEM;
goto lookup_out;
}
filp = lookup_instantiate_filp(nd, direntry, NULL);
}
/* since paths are not looked up by component - the parent
directories are presumed to be good here */

View File

@ -162,38 +162,6 @@ psx_client_can_cache:
return 0;
}
static struct cifsFileInfo *
cifs_fill_filedata(struct file *file)
{
struct list_head *tmp;
struct cifsFileInfo *pCifsFile = NULL;
struct cifsInodeInfo *pCifsInode = NULL;
/* search inode for this file and fill in file->private_data */
pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
read_lock(&GlobalSMBSeslock);
list_for_each(tmp, &pCifsInode->openFileList) {
pCifsFile = list_entry(tmp, struct cifsFileInfo, flist);
if ((pCifsFile->pfile == NULL) &&
(pCifsFile->pid == current->tgid)) {
/* mode set in cifs_create */
/* needed for writepage */
pCifsFile->pfile = file;
file->private_data = pCifsFile;
break;
}
}
read_unlock(&GlobalSMBSeslock);
if (file->private_data != NULL) {
return pCifsFile;
} else if ((file->f_flags & O_CREAT) && (file->f_flags & O_EXCL))
cERROR(1, "could not find file instance for "
"new file %p", file);
return NULL;
}
/* all arguments to this function must be checked for validity in caller */
static inline int cifs_open_inode_helper(struct inode *inode, struct file *file,
struct cifsInodeInfo *pCifsInode, struct cifsFileInfo *pCifsFile,
@ -256,7 +224,7 @@ int cifs_open(struct inode *inode, struct file *file)
__u32 oplock;
struct cifs_sb_info *cifs_sb;
struct cifsTconInfo *tcon;
struct cifsFileInfo *pCifsFile;
struct cifsFileInfo *pCifsFile = NULL;
struct cifsInodeInfo *pCifsInode;
char *full_path = NULL;
int desiredAccess;
@ -270,12 +238,6 @@ int cifs_open(struct inode *inode, struct file *file)
tcon = cifs_sb->tcon;
pCifsInode = CIFS_I(file->f_path.dentry->d_inode);
pCifsFile = cifs_fill_filedata(file);
if (pCifsFile) {
rc = 0;
FreeXid(xid);
return rc;
}
full_path = build_path_from_dentry(file->f_path.dentry);
if (full_path == NULL) {
@ -315,7 +277,6 @@ int cifs_open(struct inode *inode, struct file *file)
rc = -ENOMEM;
goto out;
}
file->private_data = pCifsFile;
cifs_posix_open_inode_helper(inode, file, pCifsInode,
oplock, netfid);
@ -401,8 +362,7 @@ int cifs_open(struct inode *inode, struct file *file)
pCifsFile = cifs_new_fileinfo(inode, netfid, file, file->f_path.mnt,
file->f_flags);
file->private_data = pCifsFile;
if (file->private_data == NULL) {
if (pCifsFile == NULL) {
rc = -ENOMEM;
goto out;
}