From 5f0319a79043457d2555f059fac68c1d840ce381 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 16 Sep 2008 14:05:16 -0400 Subject: [PATCH 01/19] cifs: clean up variables in cifs_unlink Change parameters to cifs_unlink to match the ones used in the generic VFS. Add some local variables to cut down on the amount of struct dereferencing that needs to be done, and eliminate some unneeded NULL pointer checks on the parent directory inode. Finally, rename pTcon to "tcon" to more closely match standard kernel coding style. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsfs.h | 2 +- fs/cifs/inode.c | 90 ++++++++++++++++++++++-------------------------- 2 files changed, 42 insertions(+), 50 deletions(-) diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index 135c965c4137..f7b4a5cd837b 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -41,7 +41,7 @@ extern int cifs_create(struct inode *, struct dentry *, int, struct nameidata *); extern struct dentry *cifs_lookup(struct inode *, struct dentry *, struct nameidata *); -extern int cifs_unlink(struct inode *, struct dentry *); +extern int cifs_unlink(struct inode *dir, struct dentry *dentry); extern int cifs_hardlink(struct dentry *, struct inode *, struct dentry *); extern int cifs_mknod(struct inode *, struct dentry *, int, dev_t); extern int cifs_mkdir(struct inode *, struct dentry *, int); diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 9c548f110102..511c52616794 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -665,40 +665,34 @@ struct inode *cifs_iget(struct super_block *sb, unsigned long ino) return inode; } -int cifs_unlink(struct inode *inode, struct dentry *direntry) +int cifs_unlink(struct inode *dir, struct dentry *dentry) { int rc = 0; int xid; - struct cifs_sb_info *cifs_sb; - struct cifsTconInfo *pTcon; char *full_path = NULL; + struct inode *inode = dentry->d_inode; struct cifsInodeInfo *cifsInode; + struct super_block *sb = dir->i_sb; + struct cifs_sb_info *cifs_sb = CIFS_SB(sb); + struct cifsTconInfo *tcon = cifs_sb->tcon; FILE_BASIC_INFO *pinfo_buf; - cFYI(1, ("cifs_unlink, inode = 0x%p", inode)); + cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry)); xid = GetXid(); - if (inode) - cifs_sb = CIFS_SB(inode->i_sb); - else - cifs_sb = CIFS_SB(direntry->d_sb); - pTcon = cifs_sb->tcon; - - /* Unlink can be called from rename so we can not grab the sem here - since we deadlock otherwise */ -/* mutex_lock(&direntry->d_sb->s_vfs_rename_mutex);*/ - full_path = build_path_from_dentry(direntry); -/* mutex_unlock(&direntry->d_sb->s_vfs_rename_mutex);*/ + /* Unlink can be called from rename so we can not take the + * sb->s_vfs_rename_mutex here */ + full_path = build_path_from_dentry(dentry); if (full_path == NULL) { FreeXid(xid); return -ENOMEM; } - if ((pTcon->ses->capabilities & CAP_UNIX) && + if ((tcon->ses->capabilities & CAP_UNIX) && (CIFS_UNIX_POSIX_PATH_OPS_CAP & - le64_to_cpu(pTcon->fsUnixInfo.Capability))) { - rc = CIFSPOSIXDelFile(xid, pTcon, full_path, + le64_to_cpu(tcon->fsUnixInfo.Capability))) { + rc = CIFSPOSIXDelFile(xid, tcon, full_path, SMB_POSIX_UNLINK_FILE_TARGET, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); cFYI(1, ("posix del rc %d", rc)); @@ -706,31 +700,31 @@ int cifs_unlink(struct inode *inode, struct dentry *direntry) goto psx_del_no_retry; } - rc = CIFSSMBDelFile(xid, pTcon, full_path, cifs_sb->local_nls, + rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); psx_del_no_retry: if (!rc) { - if (direntry->d_inode) - drop_nlink(direntry->d_inode); + if (inode) + drop_nlink(inode); } else if (rc == -ENOENT) { - d_drop(direntry); + d_drop(dentry); } else if (rc == -ETXTBSY) { int oplock = 0; __u16 netfid; - rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_OPEN, DELETE, + rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, DELETE, CREATE_NOT_DIR | CREATE_DELETE_ON_CLOSE, &netfid, &oplock, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); if (rc == 0) { - CIFSSMBRenameOpenFile(xid, pTcon, netfid, NULL, + CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - CIFSSMBClose(xid, pTcon, netfid); - if (direntry->d_inode) - drop_nlink(direntry->d_inode); + CIFSSMBClose(xid, tcon, netfid); + if (inode) + drop_nlink(inode); } } else if (rc == -EACCES) { /* try only if r/o attribute set in local lookup data? */ @@ -738,8 +732,8 @@ psx_del_no_retry: if (pinfo_buf) { /* ATTRS set to normal clears r/o bit */ pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL); - if (!(pTcon->ses->flags & CIFS_SES_NT4)) - rc = CIFSSMBSetPathInfo(xid, pTcon, full_path, + if (!(tcon->ses->flags & CIFS_SES_NT4)) + rc = CIFSSMBSetPathInfo(xid, tcon, full_path, pinfo_buf, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & @@ -750,7 +744,7 @@ psx_del_no_retry: if (rc == -EOPNOTSUPP) { int oplock = 0; __u16 netfid; - /* rc = CIFSSMBSetAttrLegacy(xid, pTcon, + /* rc = CIFSSMBSetAttrLegacy(xid, tcon, full_path, (__u16)ATTR_NORMAL, cifs_sb->local_nls); @@ -761,7 +755,7 @@ psx_del_no_retry: /* BB could scan to see if we already have it open and pass in pid of opener to function */ - rc = CIFSSMBOpen(xid, pTcon, full_path, + rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, 0, &netfid, &oplock, NULL, @@ -769,28 +763,28 @@ psx_del_no_retry: cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); if (rc == 0) { - rc = CIFSSMBSetFileInfo(xid, pTcon, + rc = CIFSSMBSetFileInfo(xid, tcon, pinfo_buf, netfid, current->tgid); - CIFSSMBClose(xid, pTcon, netfid); + CIFSSMBClose(xid, tcon, netfid); } } kfree(pinfo_buf); } if (rc == 0) { - rc = CIFSSMBDelFile(xid, pTcon, full_path, + rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); if (!rc) { - if (direntry->d_inode) - drop_nlink(direntry->d_inode); + if (inode) + drop_nlink(inode); } else if (rc == -ETXTBSY) { int oplock = 0; __u16 netfid; - rc = CIFSSMBOpen(xid, pTcon, full_path, + rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, DELETE, CREATE_NOT_DIR | CREATE_DELETE_ON_CLOSE, @@ -799,30 +793,28 @@ psx_del_no_retry: cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); if (rc == 0) { - CIFSSMBRenameOpenFile(xid, pTcon, + CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - CIFSSMBClose(xid, pTcon, netfid); - if (direntry->d_inode) - drop_nlink(direntry->d_inode); + CIFSSMBClose(xid, tcon, netfid); + if (inode) + drop_nlink(inode); } /* BB if rc = -ETXTBUSY goto the rename logic BB */ } } } - if (direntry->d_inode) { - cifsInode = CIFS_I(direntry->d_inode); + if (inode) { + cifsInode = CIFS_I(inode); cifsInode->time = 0; /* will force revalidate to get info when needed */ - direntry->d_inode->i_ctime = current_fs_time(inode->i_sb); - } - if (inode) { - inode->i_ctime = inode->i_mtime = current_fs_time(inode->i_sb); - cifsInode = CIFS_I(inode); - cifsInode->time = 0; /* force revalidate of dir as well */ + inode->i_ctime = current_fs_time(sb); } + dir->i_ctime = dir->i_mtime = current_fs_time(sb); + cifsInode = CIFS_I(dir); + cifsInode->time = 0; /* force revalidate of dir as well */ kfree(full_path); FreeXid(xid); From 388e57b2759672a3e3ede0d2f7e95124b417b0a3 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 16 Sep 2008 23:50:58 +0000 Subject: [PATCH 02/19] [CIFS] use common code for turning off ATTR_READONLY in cifs_unlink We already have a cifs_set_file_info function that can flip DOS attribute bits. Have cifs_unlink call it to handle turning ATTR_HIDDEN on and ATTR_READONLY off when an unlink attempt returns -EACCES. This also removes a level of indentation from cifs_unlink. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 306 ++++++++++++++++++++++-------------------------- 1 file changed, 139 insertions(+), 167 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 511c52616794..8dbc7c90309c 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -665,6 +665,101 @@ struct inode *cifs_iget(struct super_block *sb, unsigned long ino) return inode; } +static int +cifs_set_file_info(struct inode *inode, struct iattr *attrs, int xid, + char *full_path, __u32 dosattr) +{ + int rc; + int oplock = 0; + __u16 netfid; + __u32 netpid; + bool set_time = false; + struct cifsFileInfo *open_file; + struct cifsInodeInfo *cifsInode = CIFS_I(inode); + struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); + struct cifsTconInfo *pTcon = cifs_sb->tcon; + FILE_BASIC_INFO info_buf; + + if (attrs->ia_valid & ATTR_ATIME) { + set_time = true; + info_buf.LastAccessTime = + cpu_to_le64(cifs_UnixTimeToNT(attrs->ia_atime)); + } else + info_buf.LastAccessTime = 0; + + if (attrs->ia_valid & ATTR_MTIME) { + set_time = true; + info_buf.LastWriteTime = + cpu_to_le64(cifs_UnixTimeToNT(attrs->ia_mtime)); + } else + info_buf.LastWriteTime = 0; + + /* + * Samba throws this field away, but windows may actually use it. + * Do not set ctime unless other time stamps are changed explicitly + * (i.e. by utimes()) since we would then have a mix of client and + * server times. + */ + if (set_time && (attrs->ia_valid & ATTR_CTIME)) { + cFYI(1, ("CIFS - CTIME changed")); + info_buf.ChangeTime = + cpu_to_le64(cifs_UnixTimeToNT(attrs->ia_ctime)); + } else + info_buf.ChangeTime = 0; + + info_buf.CreationTime = 0; /* don't change */ + info_buf.Attributes = cpu_to_le32(dosattr); + + /* + * If the file is already open for write, just use that fileid + */ + open_file = find_writable_file(cifsInode); + if (open_file) { + netfid = open_file->netfid; + netpid = open_file->pid; + goto set_via_filehandle; + } + + /* + * NT4 apparently returns success on this call, but it doesn't + * really work. + */ + if (!(pTcon->ses->flags & CIFS_SES_NT4)) { + rc = CIFSSMBSetPathInfo(xid, pTcon, full_path, + &info_buf, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + if (rc != -EOPNOTSUPP && rc != -EINVAL) + goto out; + } + + cFYI(1, ("calling SetFileInfo since SetPathInfo for " + "times not supported by this server")); + rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_OPEN, + SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, + CREATE_NOT_DIR, &netfid, &oplock, + NULL, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + + if (rc != 0) { + if (rc == -EIO) + rc = -EINVAL; + goto out; + } + + netpid = current->tgid; + +set_via_filehandle: + rc = CIFSSMBSetFileInfo(xid, pTcon, &info_buf, netfid, netpid); + if (open_file == NULL) + CIFSSMBClose(xid, pTcon, netfid); + else + atomic_dec(&open_file->wrtPending); +out: + return rc; +} + int cifs_unlink(struct inode *dir, struct dentry *dentry) { int rc = 0; @@ -675,7 +770,8 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) struct super_block *sb = dir->i_sb; struct cifs_sb_info *cifs_sb = CIFS_SB(sb); struct cifsTconInfo *tcon = cifs_sb->tcon; - FILE_BASIC_INFO *pinfo_buf; + struct iattr *attrs; + __u32 dosattr; cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry)); @@ -728,84 +824,55 @@ psx_del_no_retry: } } else if (rc == -EACCES) { /* try only if r/o attribute set in local lookup data? */ - pinfo_buf = kzalloc(sizeof(FILE_BASIC_INFO), GFP_KERNEL); - if (pinfo_buf) { - /* ATTRS set to normal clears r/o bit */ - pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL); - if (!(tcon->ses->flags & CIFS_SES_NT4)) - rc = CIFSSMBSetPathInfo(xid, tcon, full_path, - pinfo_buf, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - else - rc = -EOPNOTSUPP; - - if (rc == -EOPNOTSUPP) { - int oplock = 0; - __u16 netfid; - /* rc = CIFSSMBSetAttrLegacy(xid, tcon, - full_path, - (__u16)ATTR_NORMAL, - cifs_sb->local_nls); - For some strange reason it seems that NT4 eats the - old setattr call without actually setting the - attributes so on to the third attempted workaround - */ - - /* BB could scan to see if we already have it open - and pass in pid of opener to function */ - rc = CIFSSMBOpen(xid, tcon, full_path, - FILE_OPEN, SYNCHRONIZE | - FILE_WRITE_ATTRIBUTES, 0, - &netfid, &oplock, NULL, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == 0) { - rc = CIFSSMBSetFileInfo(xid, tcon, - pinfo_buf, - netfid, - current->tgid); - CIFSSMBClose(xid, tcon, netfid); - } - } - kfree(pinfo_buf); + attrs = kzalloc(sizeof(*attrs), GFP_KERNEL); + if (attrs == NULL) { + rc = -ENOMEM; + goto out_reval; } + + /* try to reset dos attributes */ + cifsInode = CIFS_I(inode); + dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY; + if (dosattr == 0) + dosattr |= ATTR_NORMAL; + dosattr |= ATTR_HIDDEN; + + rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr); + kfree(attrs); + if (rc != 0) + goto out_reval; + rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); if (rc == 0) { - rc = CIFSSMBDelFile(xid, tcon, full_path, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (!rc) { + if (inode) + drop_nlink(inode); + } else if (rc == -ETXTBSY) { + int oplock = 0; + __u16 netfid; + + rc = CIFSSMBOpen(xid, tcon, full_path, + FILE_OPEN, DELETE, + CREATE_NOT_DIR | + CREATE_DELETE_ON_CLOSE, + &netfid, &oplock, NULL, + cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + if (rc == 0) { + CIFSSMBRenameOpenFile(xid, tcon, + netfid, NULL, + cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + CIFSSMBClose(xid, tcon, netfid); if (inode) drop_nlink(inode); - } else if (rc == -ETXTBSY) { - int oplock = 0; - __u16 netfid; - - rc = CIFSSMBOpen(xid, tcon, full_path, - FILE_OPEN, DELETE, - CREATE_NOT_DIR | - CREATE_DELETE_ON_CLOSE, - &netfid, &oplock, NULL, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == 0) { - CIFSSMBRenameOpenFile(xid, tcon, - netfid, NULL, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - CIFSSMBClose(xid, tcon, netfid); - if (inode) - drop_nlink(inode); - } - /* BB if rc = -ETXTBUSY goto the rename logic BB */ } + /* BB if rc = -ETXTBUSY goto the rename logic BB */ } } +out_reval: if (inode) { cifsInode = CIFS_I(inode); cifsInode->time = 0; /* will force revalidate to get info @@ -1498,101 +1565,6 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs, return rc; } -static int -cifs_set_file_info(struct inode *inode, struct iattr *attrs, int xid, - char *full_path, __u32 dosattr) -{ - int rc; - int oplock = 0; - __u16 netfid; - __u32 netpid; - bool set_time = false; - struct cifsFileInfo *open_file; - struct cifsInodeInfo *cifsInode = CIFS_I(inode); - struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); - struct cifsTconInfo *pTcon = cifs_sb->tcon; - FILE_BASIC_INFO info_buf; - - if (attrs->ia_valid & ATTR_ATIME) { - set_time = true; - info_buf.LastAccessTime = - cpu_to_le64(cifs_UnixTimeToNT(attrs->ia_atime)); - } else - info_buf.LastAccessTime = 0; - - if (attrs->ia_valid & ATTR_MTIME) { - set_time = true; - info_buf.LastWriteTime = - cpu_to_le64(cifs_UnixTimeToNT(attrs->ia_mtime)); - } else - info_buf.LastWriteTime = 0; - - /* - * Samba throws this field away, but windows may actually use it. - * Do not set ctime unless other time stamps are changed explicitly - * (i.e. by utimes()) since we would then have a mix of client and - * server times. - */ - if (set_time && (attrs->ia_valid & ATTR_CTIME)) { - cFYI(1, ("CIFS - CTIME changed")); - info_buf.ChangeTime = - cpu_to_le64(cifs_UnixTimeToNT(attrs->ia_ctime)); - } else - info_buf.ChangeTime = 0; - - info_buf.CreationTime = 0; /* don't change */ - info_buf.Attributes = cpu_to_le32(dosattr); - - /* - * If the file is already open for write, just use that fileid - */ - open_file = find_writable_file(cifsInode); - if (open_file) { - netfid = open_file->netfid; - netpid = open_file->pid; - goto set_via_filehandle; - } - - /* - * NT4 apparently returns success on this call, but it doesn't - * really work. - */ - if (!(pTcon->ses->flags & CIFS_SES_NT4)) { - rc = CIFSSMBSetPathInfo(xid, pTcon, full_path, - &info_buf, cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc != -EOPNOTSUPP && rc != -EINVAL) - goto out; - } - - cFYI(1, ("calling SetFileInfo since SetPathInfo for " - "times not supported by this server")); - rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_OPEN, - SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, - CREATE_NOT_DIR, &netfid, &oplock, - NULL, cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - - if (rc != 0) { - if (rc == -EIO) - rc = -EINVAL; - goto out; - } - - netpid = current->tgid; - -set_via_filehandle: - rc = CIFSSMBSetFileInfo(xid, pTcon, &info_buf, netfid, netpid); - if (open_file == NULL) - CIFSSMBClose(xid, pTcon, netfid); - else - atomic_dec(&open_file->wrtPending); -out: - return rc; -} - static int cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs) { From 232087cb734c7035c0a8947fb05d3e8092ff6c4d Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Mon, 15 Sep 2008 13:22:54 +0300 Subject: [PATCH 03/19] cifs: don't use GFP_KERNEL with GFP_NOFS GFP_KERNEL and GFP_NOFS are mutually exclusive. If you combine them, you end up with plain GFP_KERNEL which can deadlock in cases where you really want GFP_NOFS. Cc: Steve French Signed-off-by: Pekka Enberg Signed-off-by: Steve French --- fs/cifs/misc.c | 6 ++---- fs/cifs/transport.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 4b17f8fe3157..654d972a88f4 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -150,8 +150,7 @@ cifs_buf_get(void) but it may be more efficient to always alloc same size albeit slightly larger than necessary and maxbuffersize defaults to this and can not be bigger */ - ret_buf = (struct smb_hdr *) mempool_alloc(cifs_req_poolp, - GFP_KERNEL | GFP_NOFS); + ret_buf = mempool_alloc(cifs_req_poolp, GFP_NOFS); /* clear the first few header bytes */ /* for most paths, more is cleared in header_assemble */ @@ -188,8 +187,7 @@ cifs_small_buf_get(void) but it may be more efficient to always alloc same size albeit slightly larger than necessary and maxbuffersize defaults to this and can not be bigger */ - ret_buf = (struct smb_hdr *) mempool_alloc(cifs_sm_req_poolp, - GFP_KERNEL | GFP_NOFS); + ret_buf = mempool_alloc(cifs_sm_req_poolp, GFP_NOFS); if (ret_buf) { /* No need to clear memory here, cleared in header assemble */ /* memset(ret_buf, 0, sizeof(struct smb_hdr) + 27);*/ diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index e286db9f5ee2..bf0e6d8e382a 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -50,8 +50,7 @@ AllocMidQEntry(const struct smb_hdr *smb_buffer, struct cifsSesInfo *ses) return NULL; } - temp = (struct mid_q_entry *) mempool_alloc(cifs_mid_poolp, - GFP_KERNEL | GFP_NOFS); + temp = mempool_alloc(cifs_mid_poolp, GFP_NOFS); if (temp == NULL) return temp; else { From 2846d3864738dd6e290755d0692cf377e09ba79f Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 22 Sep 2008 21:33:33 -0400 Subject: [PATCH 04/19] cifs: have find_writeable_file prefer filehandles opened by same task When the CIFS client goes to write out pages, it needs to pick a filehandle to write to. find_writeable_file however just picks the first filehandle that it finds. This can cause problems when a lock is issued against a particular filehandle and we pick a different filehandle to write to. This patch tries to avert this situation by having find_writable_file prefer filehandles that have a pid that matches the current task. This seems to fix lock test 11 from the connectathon test suite when run against a windows server. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/file.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index cbefe1f1f9fe..d39e852a28a9 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1065,6 +1065,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode) struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode) { struct cifsFileInfo *open_file; + bool any_available = false; int rc; /* Having a null inode here (because mapping->host was set to zero by @@ -1080,8 +1081,10 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode) read_lock(&GlobalSMBSeslock); refind_writable: list_for_each_entry(open_file, &cifs_inode->openFileList, flist) { - if (open_file->closePend) + if (open_file->closePend || + (!any_available && open_file->pid != current->tgid)) continue; + if (open_file->pfile && ((open_file->pfile->f_flags & O_RDWR) || (open_file->pfile->f_flags & O_WRONLY))) { @@ -1131,6 +1134,11 @@ refind_writable: of the loop here. */ } } + /* couldn't find useable FH with same pid, try any available */ + if (!any_available) { + any_available = true; + goto refind_writable; + } read_unlock(&GlobalSMBSeslock); return NULL; } From a12a1ac7a474b3680b9cce9f64a4f78123aecf37 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 23 Sep 2008 11:48:35 -0400 Subject: [PATCH 05/19] cifs: move rename and delete-on-close logic into helper function cifs: move rename and delete-on-close logic into helper function When a file is still open on the server, we attempt to set the DELETE_ON_CLOSE bit and rename it to a new filename. When the last opener closes the file, the server should delete it. This patch moves this mechanism into a helper function and has the two places in cifs_unlink that do this procedure call it. It also fixes the open flags to be correct. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 98 +++++++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 39 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 8dbc7c90309c..660aac81160a 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -760,6 +760,59 @@ out: return rc; } +/* + * open the given file (if it isn't already), set the DELETE_ON_CLOSE bit + * and rename it to a random name that hopefully won't conflict with + * anything else. + */ +static int +cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) +{ + int oplock = 0; + int rc; + __u16 netfid; + struct cifsInodeInfo *cifsInode = CIFS_I(inode); + struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); + struct cifsTconInfo *tcon = cifs_sb->tcon; + __u32 dosattr; + FILE_BASIC_INFO *info_buf; + + rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, + DELETE|FILE_WRITE_ATTRIBUTES, + CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE, + &netfid, &oplock, NULL, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); + if (rc != 0) + goto out; + + /* set ATTR_HIDDEN and clear ATTR_READONLY */ + cifsInode = CIFS_I(inode); + dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY; + if (dosattr == 0) + dosattr |= ATTR_NORMAL; + dosattr |= ATTR_HIDDEN; + + info_buf = kzalloc(sizeof(*info_buf), GFP_KERNEL); + if (info_buf == NULL) { + rc = -ENOMEM; + goto out_close; + } + info_buf->Attributes = cpu_to_le32(dosattr); + rc = CIFSSMBSetFileInfo(xid, tcon, info_buf, netfid, current->tgid); + kfree(info_buf); + if (rc != 0) + goto out_close; + + /* silly-rename the file */ + rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); +out_close: + CIFSSMBClose(xid, tcon, netfid); +out: + return rc; +} + int cifs_unlink(struct inode *dir, struct dentry *dentry) { int rc = 0; @@ -805,23 +858,9 @@ psx_del_no_retry: } else if (rc == -ENOENT) { d_drop(dentry); } else if (rc == -ETXTBSY) { - int oplock = 0; - __u16 netfid; - - rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, DELETE, - CREATE_NOT_DIR | CREATE_DELETE_ON_CLOSE, - &netfid, &oplock, NULL, cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == 0) { - CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - CIFSSMBClose(xid, tcon, netfid); - if (inode) - drop_nlink(inode); - } + rc = cifs_rename_pending_delete(full_path, inode, xid); + if (rc == 0) + drop_nlink(inode); } else if (rc == -EACCES) { /* try only if r/o attribute set in local lookup data? */ attrs = kzalloc(sizeof(*attrs), GFP_KERNEL); @@ -848,28 +887,9 @@ psx_del_no_retry: if (inode) drop_nlink(inode); } else if (rc == -ETXTBSY) { - int oplock = 0; - __u16 netfid; - - rc = CIFSSMBOpen(xid, tcon, full_path, - FILE_OPEN, DELETE, - CREATE_NOT_DIR | - CREATE_DELETE_ON_CLOSE, - &netfid, &oplock, NULL, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == 0) { - CIFSSMBRenameOpenFile(xid, tcon, - netfid, NULL, - cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - CIFSSMBClose(xid, tcon, netfid); - if (inode) - drop_nlink(inode); - } - /* BB if rc = -ETXTBUSY goto the rename logic BB */ + rc = cifs_rename_pending_delete(full_path, inode, xid); + if (rc == 0) + drop_nlink(inode); } } out_reval: From 7c9c3760b3a5ae87ee4d661703b6d5de3999fe46 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 23 Sep 2008 17:23:09 +0000 Subject: [PATCH 06/19] [CIFS] add constants for string lengths of keynames in SPNEGO upcall string Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifs_spnego.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c index 117ef4bba68e..fcee9298b620 100644 --- a/fs/cifs/cifs_spnego.c +++ b/fs/cifs/cifs_spnego.c @@ -66,11 +66,28 @@ struct key_type cifs_spnego_key_type = { .describe = user_describe, }; -#define MAX_VER_STR_LEN 8 /* length of longest version string e.g. - strlen("ver=0xFF") */ -#define MAX_MECH_STR_LEN 13 /* length of longest security mechanism name, eg - in future could have strlen(";sec=ntlmsspi") */ -#define MAX_IPV6_ADDR_LEN 42 /* eg FEDC:BA98:7654:3210:FEDC:BA98:7654:3210/60 */ +/* length of longest version string e.g. strlen("ver=0xFF") */ +#define MAX_VER_STR_LEN 8 + +/* length of longest security mechanism name, eg in future could have + * strlen(";sec=ntlmsspi") */ +#define MAX_MECH_STR_LEN 13 + +/* max possible addr len eg FEDC:BA98:7654:3210:FEDC:BA98:7654:3210/60 */ +#define MAX_IPV6_ADDR_LEN 42 + +/* strlen of "host=" */ +#define HOST_KEY_LEN 5 + +/* strlen of ";ip4=" or ";ip6=" */ +#define IP_KEY_LEN 5 + +/* strlen of ";uid=0x" */ +#define UID_KEY_LEN 7 + +/* strlen of ";user=" */ +#define USER_KEY_LEN 6 + /* get a key struct with a SPNEGO security blob, suitable for session setup */ struct key * cifs_get_spnego_key(struct cifsSesInfo *sesInfo) @@ -84,11 +101,11 @@ cifs_get_spnego_key(struct cifsSesInfo *sesInfo) /* length of fields (with semicolons): ver=0xyz ip4=ipaddress host=hostname sec=mechanism uid=0xFF user=username */ desc_len = MAX_VER_STR_LEN + - 6 /* len of "host=" */ + strlen(hostname) + - 5 /* len of ";ipv4=" */ + MAX_IPV6_ADDR_LEN + + HOST_KEY_LEN + strlen(hostname) + + IP_KEY_LEN + MAX_IPV6_ADDR_LEN + MAX_MECH_STR_LEN + - 7 /* len of ";uid=0x" */ + (sizeof(uid_t) * 2) + - 6 /* len of ";user=" */ + strlen(sesInfo->userName) + 1; + UID_KEY_LEN + (sizeof(uid_t) * 2) + + USER_KEY_LEN + strlen(sesInfo->userName) + 1; spnego_key = ERR_PTR(-ENOMEM); description = kzalloc(desc_len, GFP_KERNEL); From 6d22f09896c0d62c003ffa25fff25323e3ed608b Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 23 Sep 2008 11:48:35 -0400 Subject: [PATCH 07/19] cifs: add function to set file disposition cifs: add function to set file disposition The proper way to set the delete on close bit on an already existing file is to use SET_FILE_INFO with an infolevel of SMB_FILE_DISPOSITION_INFO. Add a function to do that and have the silly-rename code use it. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 2 ++ fs/cifs/cifssmb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++ fs/cifs/inode.c | 9 ++++++-- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index a729d083e6f4..014f26c7864f 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -179,6 +179,8 @@ extern int CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon, extern int CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon, const FILE_BASIC_INFO *data, __u16 fid, __u32 pid_of_opener); +extern int CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon, + bool delete_file, __u16 fid, __u32 pid_of_opener); #if 0 extern int CIFSSMBSetAttrLegacy(int xid, struct cifsTconInfo *tcon, char *fileName, __u16 dos_attributes, diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 994de7c90474..7b365842bfa1 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -4876,6 +4876,61 @@ CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon, return rc; } +int +CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon, + bool delete_file, __u16 fid, __u32 pid_of_opener) +{ + struct smb_com_transaction2_sfi_req *pSMB = NULL; + char *data_offset; + int rc = 0; + __u16 params, param_offset, offset, byte_count, count; + + cFYI(1, ("Set File Disposition (via SetFileInfo)")); + rc = small_smb_init(SMB_COM_TRANSACTION2, 15, tcon, (void **) &pSMB); + + if (rc) + return rc; + + pSMB->hdr.Pid = cpu_to_le16((__u16)pid_of_opener); + pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid_of_opener >> 16)); + + params = 6; + pSMB->MaxSetupCount = 0; + pSMB->Reserved = 0; + pSMB->Flags = 0; + pSMB->Timeout = 0; + pSMB->Reserved2 = 0; + param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4; + offset = param_offset + params; + + data_offset = (char *) (&pSMB->hdr.Protocol) + offset; + + count = 1; + pSMB->MaxParameterCount = cpu_to_le16(2); + /* BB find max SMB PDU from sess */ + pSMB->MaxDataCount = cpu_to_le16(1000); + pSMB->SetupCount = 1; + pSMB->Reserved3 = 0; + pSMB->SubCommand = cpu_to_le16(TRANS2_SET_FILE_INFORMATION); + byte_count = 3 /* pad */ + params + count; + pSMB->DataCount = cpu_to_le16(count); + pSMB->ParameterCount = cpu_to_le16(params); + pSMB->TotalDataCount = pSMB->DataCount; + pSMB->TotalParameterCount = pSMB->ParameterCount; + pSMB->ParameterOffset = cpu_to_le16(param_offset); + pSMB->DataOffset = cpu_to_le16(offset); + pSMB->Fid = fid; + pSMB->InformationLevel = cpu_to_le16(SMB_SET_FILE_DISPOSITION_INFO); + pSMB->Reserved4 = 0; + pSMB->hdr.smb_buf_length += byte_count; + pSMB->ByteCount = cpu_to_le16(byte_count); + *data_offset = delete_file ? 1 : 0; + rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0); + if (rc) + cFYI(1, ("Send error in SetFileDisposition = %d", rc)); + + return rc; +} int CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon, diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 660aac81160a..954b670f1687 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -778,8 +778,7 @@ cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) FILE_BASIC_INFO *info_buf; rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, - DELETE|FILE_WRITE_ATTRIBUTES, - CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE, + DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR, &netfid, &oplock, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); if (rc != 0) @@ -807,6 +806,12 @@ cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); + if (rc != 0) + goto out_close; + + /* set DELETE_ON_CLOSE */ + rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid); + out_close: CIFSSMBClose(xid, tcon, netfid); out: From ee2fd967fb23c5eecabc8a660ec66fcd79acbd47 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 23 Sep 2008 18:23:33 +0000 Subject: [PATCH 08/19] [CIFS] fix busy-file renames and refactor cifs_rename logic Break out the code that does the actual renaming into a separate function and have cifs_rename call that. That function will attempt a path based rename first and then do a filehandle based one if it looks like the source is busy. The existing logic tried a path based rename first, but if we needed to remove the destination then it only attempted a filehandle based rename afterward. Not all servers support renaming by filehandle, so we need to always attempt path rename first and fall back to filehandle rename if it doesn't work. This also fixes renames of open files on windows servers (at least when the source and destination directories are the same). CC: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 188 +++++++++++++++++++++++++++--------------------- 1 file changed, 105 insertions(+), 83 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 954b670f1687..82612be9477b 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -806,8 +806,6 @@ cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc != 0) - goto out_close; /* set DELETE_ON_CLOSE */ rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid); @@ -1180,117 +1178,141 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry) return rc; } +static int +cifs_do_rename(int xid, struct dentry *from_dentry, const char *fromPath, + struct dentry *to_dentry, const char *toPath) +{ + struct cifs_sb_info *cifs_sb = CIFS_SB(from_dentry->d_sb); + struct cifsTconInfo *pTcon = cifs_sb->tcon; + __u16 srcfid; + int oplock, rc; + + /* try path-based rename first */ + rc = CIFSSMBRename(xid, pTcon, fromPath, toPath, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + + /* + * don't bother with rename by filehandle unless file is busy and + * source Note that cross directory moves do not work with + * rename by filehandle to various Windows servers. + */ + if (rc == 0 || rc != -ETXTBSY) + return rc; + + /* open the file to be renamed -- we need DELETE perms */ + rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE, + CREATE_NOT_DIR, &srcfid, &oplock, NULL, + cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + + if (rc == 0) { + rc = CIFSSMBRenameOpenFile(xid, pTcon, srcfid, + (const char *) to_dentry->d_name.name, + cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & + CIFS_MOUNT_MAP_SPECIAL_CHR); + + CIFSSMBClose(xid, pTcon, srcfid); + } + + return rc; +} + int cifs_rename(struct inode *source_inode, struct dentry *source_direntry, struct inode *target_inode, struct dentry *target_direntry) { - char *fromName; - char *toName; + char *fromName = NULL; + char *toName = NULL; struct cifs_sb_info *cifs_sb_source; struct cifs_sb_info *cifs_sb_target; struct cifsTconInfo *pTcon; + FILE_UNIX_BASIC_INFO *info_buf_source = NULL; + FILE_UNIX_BASIC_INFO *info_buf_target; int xid; - int rc = 0; - - xid = GetXid(); + int rc; cifs_sb_target = CIFS_SB(target_inode->i_sb); cifs_sb_source = CIFS_SB(source_inode->i_sb); pTcon = cifs_sb_source->tcon; + xid = GetXid(); + + /* + * BB: this might be allowed if same server, but different share. + * Consider adding support for this + */ if (pTcon != cifs_sb_target->tcon) { - FreeXid(xid); - return -EXDEV; /* BB actually could be allowed if same server, - but different share. - Might eventually add support for this */ + rc = -EXDEV; + goto cifs_rename_exit; } - /* we already have the rename sem so we do not need to grab it again - here to protect the path integrity */ + /* + * we already have the rename sem so we do not need to + * grab it again here to protect the path integrity + */ fromName = build_path_from_dentry(source_direntry); - toName = build_path_from_dentry(target_direntry); - if ((fromName == NULL) || (toName == NULL)) { + if (fromName == NULL) { rc = -ENOMEM; goto cifs_rename_exit; } - rc = CIFSSMBRename(xid, pTcon, fromName, toName, - cifs_sb_source->local_nls, - cifs_sb_source->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == -EEXIST) { - /* check if they are the same file because rename of hardlinked - files is a noop */ - FILE_UNIX_BASIC_INFO *info_buf_source; - FILE_UNIX_BASIC_INFO *info_buf_target; + toName = build_path_from_dentry(target_direntry); + if (toName == NULL) { + rc = -ENOMEM; + goto cifs_rename_exit; + } + + rc = cifs_do_rename(xid, source_direntry, fromName, + target_direntry, toName); + + if (rc == -EEXIST) { + if (pTcon->unix_ext) { + /* + * Are src and dst hardlinks of same inode? We can + * only tell with unix extensions enabled + */ + info_buf_source = + kmalloc(2 * sizeof(FILE_UNIX_BASIC_INFO), + GFP_KERNEL); + if (info_buf_source != NULL) + goto unlink_target; - info_buf_source = - kmalloc(2 * sizeof(FILE_UNIX_BASIC_INFO), GFP_KERNEL); - if (info_buf_source != NULL) { info_buf_target = info_buf_source + 1; - if (pTcon->unix_ext) - rc = CIFSSMBUnixQPathInfo(xid, pTcon, fromName, - info_buf_source, - cifs_sb_source->local_nls, - cifs_sb_source->mnt_cifs_flags & + rc = CIFSSMBUnixQPathInfo(xid, pTcon, fromName, + info_buf_source, + cifs_sb_source->local_nls, + cifs_sb_source->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - /* else rc is still EEXIST so will fall through to - unlink the target and retry rename */ - if (rc == 0) { - rc = CIFSSMBUnixQPathInfo(xid, pTcon, toName, - info_buf_target, + if (rc != 0) + goto unlink_target; + + rc = CIFSSMBUnixQPathInfo(xid, pTcon, + toName, info_buf_target, cifs_sb_target->local_nls, /* remap based on source sb */ cifs_sb_source->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - } - if ((rc == 0) && - (info_buf_source->UniqueId == - info_buf_target->UniqueId)) { - /* do not rename since the files are hardlinked which - is a noop */ - } else { - /* we either can not tell the files are hardlinked - (as with Windows servers) or files are not - hardlinked so delete the target manually before - renaming to follow POSIX rather than Windows - semantics */ - cifs_unlink(target_inode, target_direntry); - rc = CIFSSMBRename(xid, pTcon, fromName, - toName, - cifs_sb_source->local_nls, - cifs_sb_source->mnt_cifs_flags - & CIFS_MOUNT_MAP_SPECIAL_CHR); - } - kfree(info_buf_source); - } /* if we can not get memory just leave rc as EEXIST */ - } - - if (rc) - cFYI(1, ("rename rc %d", rc)); - - if ((rc == -EIO) || (rc == -EEXIST)) { - int oplock = 0; - __u16 netfid; - - /* BB FIXME Is Generic Read correct for rename? */ - /* if renaming directory - we should not say CREATE_NOT_DIR, - need to test renaming open directory, also GENERIC_READ - might not right be right access to request */ - rc = CIFSSMBOpen(xid, pTcon, fromName, FILE_OPEN, GENERIC_READ, - CREATE_NOT_DIR, &netfid, &oplock, NULL, - cifs_sb_source->local_nls, - cifs_sb_source->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == 0) { - rc = CIFSSMBRenameOpenFile(xid, pTcon, netfid, toName, - cifs_sb_source->local_nls, - cifs_sb_source->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - CIFSSMBClose(xid, pTcon, netfid); - } + + if (rc == 0 && (info_buf_source->UniqueId == + info_buf_target->UniqueId)) + /* same file, POSIX says that this is a noop */ + goto cifs_rename_exit; + } /* else ... BB we could add the same check for Windows by + checking the UniqueId via FILE_INTERNAL_INFO */ +unlink_target: + /* + * we either can not tell the files are hardlinked (as with + * Windows servers) or files are not hardlinked. Delete the + * target manually before renaming to follow POSIX rather than + * Windows semantics + */ + cifs_unlink(target_inode, target_direntry); + rc = cifs_do_rename(xid, source_direntry, fromName, + target_direntry, toName); } cifs_rename_exit: + kfree(info_buf_source); kfree(fromName); kfree(toName); FreeXid(xid); From 9d81523480c8c5b07a4899a084b3f4264a575184 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 23 Sep 2008 18:46:07 +0000 Subject: [PATCH 09/19] [CIFS] clean up upcall handling for dns_resolver keys We're given the datalen in the downcall, so there's no need to do any calls to strlen(). Just keep track of the datalen in the key. Finally, add a sanity check of the data in the downcall to make sure that it looks like a real IP address. Signed-off-by: Jeff Layton Acked-by: David Howells Signed-off-by: Steve French --- fs/cifs/dns_resolve.c | 84 +++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/fs/cifs/dns_resolve.c b/fs/cifs/dns_resolve.c index a2e0673e1b08..1e0c1bd8f2e4 100644 --- a/fs/cifs/dns_resolve.c +++ b/fs/cifs/dns_resolve.c @@ -29,45 +29,13 @@ #include "cifsproto.h" #include "cifs_debug.h" -static int dns_resolver_instantiate(struct key *key, const void *data, - size_t datalen) -{ - int rc = 0; - char *ip; - - ip = kmalloc(datalen+1, GFP_KERNEL); - if (!ip) - return -ENOMEM; - - memcpy(ip, data, datalen); - ip[datalen] = '\0'; - - rcu_assign_pointer(key->payload.data, ip); - - return rc; -} - -static void -dns_resolver_destroy(struct key *key) -{ - kfree(key->payload.data); -} - -struct key_type key_type_dns_resolver = { - .name = "dns_resolver", - .def_datalen = sizeof(struct in_addr), - .describe = user_describe, - .instantiate = dns_resolver_instantiate, - .destroy = dns_resolver_destroy, - .match = user_match, -}; - /* Checks if supplied name is IP address * returns: * 1 - name is IP * 0 - name is not IP */ -static int is_ip(const char *name) +static int +is_ip(const char *name) { int rc; struct sockaddr_in sin_server; @@ -89,6 +57,47 @@ static int is_ip(const char *name) return 0; } +static int +dns_resolver_instantiate(struct key *key, const void *data, + size_t datalen) +{ + int rc = 0; + char *ip; + + ip = kmalloc(datalen + 1, GFP_KERNEL); + if (!ip) + return -ENOMEM; + + memcpy(ip, data, datalen); + ip[datalen] = '\0'; + + /* make sure this looks like an address */ + if (!is_ip((const char *) ip)) { + kfree(ip); + return -EINVAL; + } + + key->type_data.x[0] = datalen; + rcu_assign_pointer(key->payload.data, ip); + + return rc; +} + +static void +dns_resolver_destroy(struct key *key) +{ + kfree(key->payload.data); +} + +struct key_type key_type_dns_resolver = { + .name = "dns_resolver", + .def_datalen = sizeof(struct in_addr), + .describe = user_describe, + .instantiate = dns_resolver_instantiate, + .destroy = dns_resolver_destroy, + .match = user_match, +}; + /* Resolves server name to ip address. * input: * unc - server UNC @@ -140,6 +149,7 @@ dns_resolve_server_name_to_ip(const char *unc, char **ip_addr) rkey = request_key(&key_type_dns_resolver, name, ""); if (!IS_ERR(rkey)) { + len = rkey->type_data.x[0]; data = rkey->payload.data; } else { cERROR(1, ("%s: unable to resolve: %s", __func__, name)); @@ -148,11 +158,9 @@ dns_resolve_server_name_to_ip(const char *unc, char **ip_addr) skip_upcall: if (data) { - len = strlen(data); - *ip_addr = kmalloc(len+1, GFP_KERNEL); + *ip_addr = kmalloc(len + 1, GFP_KERNEL); if (*ip_addr) { - memcpy(*ip_addr, data, len); - (*ip_addr)[len] = '\0'; + memcpy(*ip_addr, data, len + 1); if (!IS_ERR(rkey)) cFYI(1, ("%s: resolved: %s to %s", __func__, name, From 74553b1b6a8556e08757b4bce537fd8332b93898 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 24 Sep 2008 14:55:51 -0400 Subject: [PATCH 10/19] cifs: fix inverted NULL check after kmalloc cifs: fix inverted NULL check after kmalloc Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 82612be9477b..079f39a8dd3b 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -1274,7 +1274,7 @@ int cifs_rename(struct inode *source_inode, struct dentry *source_direntry, info_buf_source = kmalloc(2 * sizeof(FILE_UNIX_BASIC_INFO), GFP_KERNEL); - if (info_buf_source != NULL) + if (info_buf_source == NULL) goto unlink_target; info_buf_target = info_buf_source + 1; From 7ce86d5a93ffe2542e6558a97ab055377df8cde3 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 24 Sep 2008 11:32:59 -0400 Subject: [PATCH 11/19] cifs: work around samba returning -ENOENT on SetFileDisposition call cifs: work around samba returning -ENOENT on SetFileDisposition call Samba seems to return STATUS_OBJECT_NAME_NOT_FOUND when we try to set the delete on close bit after doing a rename by filehandle. This looks like a samba bug to me, but a lot of servers will do this. For now, pretend an -ENOENT return is a success. Samba does however seem to respect the CREATE_DELETE_ON_CLOSE bit when opening files that already exist. Windows will ignore it, but so adding it to the open flags should be harmless. We're also currently ignoring the return code on the rename by filehandle, so no need to set rc based on it. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 079f39a8dd3b..27e97d43c759 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -778,7 +778,8 @@ cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) FILE_BASIC_INFO *info_buf; rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, - DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR, + DELETE|FILE_WRITE_ATTRIBUTES, + CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE, &netfid, &oplock, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); if (rc != 0) @@ -803,13 +804,20 @@ cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) goto out_close; /* silly-rename the file */ - rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, + CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); /* set DELETE_ON_CLOSE */ rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid); + /* + * some samba versions return -ENOENT when we try to set the file + * disposition here. Likely a samba bug, but work around it for now + */ + if (rc == -ENOENT) + rc = 0; + out_close: CIFSSMBClose(xid, tcon, netfid); out: From 391e575556109744ae0aa198c1e245588a3ea76a Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 24 Sep 2008 11:32:59 -0400 Subject: [PATCH 12/19] cifs: remove NULL termination from rename target in CIFSSMBRenameOpenFIle cifs: remove NULL termination from rename target in CIFSSMBRenameOpenFIle The rename destination isn't supposed to be null terminated. Also, change the name string arg to be const. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 2 +- fs/cifs/cifssmb.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 014f26c7864f..0cff7fe986e8 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -231,7 +231,7 @@ extern int CIFSSMBRename(const int xid, struct cifsTconInfo *tcon, const struct nls_table *nls_codepage, int remap_special_chars); extern int CIFSSMBRenameOpenFile(const int xid, struct cifsTconInfo *pTcon, - int netfid, char *target_name, + int netfid, const char *target_name, const struct nls_table *nls_codepage, int remap_special_chars); extern int CIFSCreateHardLink(const int xid, diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 7b365842bfa1..7504d1514c77 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -2017,7 +2017,7 @@ renameRetry: } int CIFSSMBRenameOpenFile(const int xid, struct cifsTconInfo *pTcon, - int netfid, char *target_name, + int netfid, const char *target_name, const struct nls_table *nls_codepage, int remap) { struct smb_com_transaction2_sfi_req *pSMB = NULL; @@ -2071,7 +2071,7 @@ int CIFSSMBRenameOpenFile(const int xid, struct cifsTconInfo *pTcon, remap); } rename_info->target_name_len = cpu_to_le32(2 * len_of_str); - count = 12 /* sizeof(struct set_file_rename) */ + (2 * len_of_str) + 2; + count = 12 /* sizeof(struct set_file_rename) */ + (2 * len_of_str); byte_count += count; pSMB->DataCount = cpu_to_le16(count); pSMB->TotalDataCount = pSMB->DataCount; From d388908ec40ada0001dfe05134de31d0cc62907c Mon Sep 17 00:00:00 2001 From: Steve French Date: Wed, 24 Sep 2008 19:22:52 +0000 Subject: [PATCH 13/19] [CIFS] update DOS attributes in cifsInode if we successfully changed them Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 27e97d43c759..db091c516c2a 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -752,6 +752,9 @@ cifs_set_file_info(struct inode *inode, struct iattr *attrs, int xid, set_via_filehandle: rc = CIFSSMBSetFileInfo(xid, pTcon, &info_buf, netfid, netpid); + if (!rc) + cifsInode->cifsAttrs = dosattr; + if (open_file == NULL) CIFSSMBClose(xid, pTcon, netfid); else @@ -902,6 +905,7 @@ psx_del_no_retry: if (rc == 0) drop_nlink(inode); } + cifsInode->cifsAttrs = dosattr; } out_reval: if (inode) { From d9414774dc0c7b395036deeca000af42e2d13612 Mon Sep 17 00:00:00 2001 From: Nick Piggin Date: Wed, 24 Sep 2008 11:32:59 -0400 Subject: [PATCH 14/19] cifs: Convert cifs to new aops. cifs: Convert cifs to new aops. This patch is based on the one originally posted by Nick Piggin. His patch was very close, but had a couple of small bugs. Nick's original comments follow: This is another relatively naive conversion. Always do the read upfront when the page is not uptodate (unless we're in the writethrough path). Fix an uninitialized data exposure where SetPageUptodate was called before the page was uptodate. SetPageUptodate and switch to writeback mode in the case that the full page was dirtied. Acked-by: Shaggy Acked-by: Badari Pulavarty Signed-off-by: Nick Piggin Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/file.c | 120 ++++++++++++++++++++++++------------------------- 1 file changed, 59 insertions(+), 61 deletions(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index d39e852a28a9..c4a8a0605125 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -107,7 +107,7 @@ static inline int cifs_open_inode_helper(struct inode *inode, struct file *file, /* want handles we can use to read with first in the list so we do not have to walk the - list to search for one in prepare_write */ + list to search for one in write_begin */ if ((file->f_flags & O_ACCMODE) == O_WRONLY) { list_add_tail(&pCifsFile->flist, &pCifsInode->openFileList); @@ -915,7 +915,7 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data, } static ssize_t cifs_write(struct file *file, const char *write_data, - size_t write_size, loff_t *poffset) + size_t write_size, loff_t *poffset) { int rc = 0; unsigned int bytes_written = 0; @@ -1455,49 +1455,52 @@ static int cifs_writepage(struct page *page, struct writeback_control *wbc) return rc; } -static int cifs_commit_write(struct file *file, struct page *page, - unsigned offset, unsigned to) +static int cifs_write_end(struct file *file, struct address_space *mapping, + loff_t pos, unsigned len, unsigned copied, + struct page *page, void *fsdata) { - int xid; - int rc = 0; - struct inode *inode = page->mapping->host; - loff_t position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to; - char *page_data; + int rc; + struct inode *inode = mapping->host; - xid = GetXid(); - cFYI(1, ("commit write for page %p up to position %lld for %d", - page, position, to)); - spin_lock(&inode->i_lock); - if (position > inode->i_size) - i_size_write(inode, position); + cFYI(1, ("write_end for page %p from pos %lld with %d bytes", + page, pos, copied)); + + if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE) + SetPageUptodate(page); - spin_unlock(&inode->i_lock); if (!PageUptodate(page)) { - position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset; - /* can not rely on (or let) writepage write this data */ - if (to < offset) { - cFYI(1, ("Illegal offsets, can not copy from %d to %d", - offset, to)); - FreeXid(xid); - return rc; - } + char *page_data; + unsigned offset = pos & (PAGE_CACHE_SIZE - 1); + int xid; + + xid = GetXid(); /* this is probably better than directly calling partialpage_write since in this function the file handle is known which we might as well leverage */ /* BB check if anything else missing out of ppw such as updating last write time */ page_data = kmap(page); - rc = cifs_write(file, page_data + offset, to-offset, - &position); - if (rc > 0) - rc = 0; - /* else if (rc < 0) should we set writebehind rc? */ + rc = cifs_write(file, page_data + offset, copied, &pos); + /* if (rc < 0) should we set writebehind rc? */ kunmap(page); + + FreeXid(xid); } else { + rc = copied; + pos += copied; set_page_dirty(page); } - FreeXid(xid); + if (rc > 0) { + spin_lock(&inode->i_lock); + if (pos > inode->i_size) + i_size_write(inode, pos); + spin_unlock(&inode->i_lock); + } + + unlock_page(page); + page_cache_release(page); + return rc; } @@ -2043,49 +2046,44 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file) return true; } -static int cifs_prepare_write(struct file *file, struct page *page, - unsigned from, unsigned to) +static int cifs_write_begin(struct file *file, struct address_space *mapping, + loff_t pos, unsigned len, unsigned flags, + struct page **pagep, void **fsdata) { - int rc = 0; - loff_t i_size; - loff_t offset; + pgoff_t index = pos >> PAGE_CACHE_SHIFT; + loff_t offset = pos & (PAGE_CACHE_SIZE - 1); - cFYI(1, ("prepare write for page %p from %d to %d", page, from, to)); - if (PageUptodate(page)) + cFYI(1, ("write_begin from %lld len %d", (long long)pos, len)); + + *pagep = __grab_cache_page(mapping, index); + if (!*pagep) + return -ENOMEM; + + if (PageUptodate(*pagep)) return 0; /* If we are writing a full page it will be up to date, no need to read from the server */ - if ((to == PAGE_CACHE_SIZE) && (from == 0)) { - SetPageUptodate(page); + if (len == PAGE_CACHE_SIZE && flags & AOP_FLAG_UNINTERRUPTIBLE) return 0; - } - offset = (loff_t)page->index << PAGE_CACHE_SHIFT; - i_size = i_size_read(page->mapping->host); + if ((file->f_flags & O_ACCMODE) != O_WRONLY) { + int rc; - if ((offset >= i_size) || - ((from == 0) && (offset + to) >= i_size)) { - /* - * We don't need to read data beyond the end of the file. - * zero it, and set the page uptodate - */ - simple_prepare_write(file, page, from, to); - SetPageUptodate(page); - } else if ((file->f_flags & O_ACCMODE) != O_WRONLY) { /* might as well read a page, it is fast enough */ - rc = cifs_readpage_worker(file, page, &offset); + rc = cifs_readpage_worker(file, *pagep, &offset); + + /* we do not need to pass errors back + e.g. if we do not have read access to the file + because cifs_write_end will attempt synchronous writes + -- shaggy */ } else { /* we could try using another file handle if there is one - but how would we lock it to prevent close of that handle racing with this read? In any case - this will be written out by commit_write so is fine */ + this will be written out by write_end so is fine */ } - /* we do not need to pass errors back - e.g. if we do not have read access to the file - because cifs_commit_write will do the right thing. -- shaggy */ - return 0; } @@ -2094,8 +2092,8 @@ const struct address_space_operations cifs_addr_ops = { .readpages = cifs_readpages, .writepage = cifs_writepage, .writepages = cifs_writepages, - .prepare_write = cifs_prepare_write, - .commit_write = cifs_commit_write, + .write_begin = cifs_write_begin, + .write_end = cifs_write_end, .set_page_dirty = __set_page_dirty_nobuffers, /* .sync_page = cifs_sync_page, */ /* .direct_IO = */ @@ -2110,8 +2108,8 @@ const struct address_space_operations cifs_addr_ops_smallbuf = { .readpage = cifs_readpage, .writepage = cifs_writepage, .writepages = cifs_writepages, - .prepare_write = cifs_prepare_write, - .commit_write = cifs_commit_write, + .write_begin = cifs_write_begin, + .write_end = cifs_write_end, .set_page_dirty = __set_page_dirty_nobuffers, /* .sync_page = cifs_sync_page, */ /* .direct_IO = */ From dfd15c46a6c2cafb006183c0c14f07e59eee4ac0 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 24 Sep 2008 11:32:59 -0400 Subject: [PATCH 15/19] cifs: explicitly revoke SPNEGO key after session setup cifs: explicitly revoke SPNEGO key after session setup The SPNEGO blob returned by an upcall can only be used once. Explicitly revoke it to make sure that we never pick it up again after session setup exits. This doesn't seem to be that big an issue on more recent kernels, but older kernels seem to link keys into the session keyring by default. That said, explicitly revoking the key seems like a reasonable thing to do here. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/sess.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 252fdc0567f1..2851d5da0c8c 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -624,8 +624,10 @@ CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, int first_time, ses, nls_cp); ssetup_exit: - if (spnego_key) + if (spnego_key) { + key_revoke(spnego_key); key_put(spnego_key); + } kfree(str_area); if (resp_buf_type == CIFS_SMALL_BUFFER) { cFYI(1, ("ssetup freeing small buf %p", iov[0].iov_base)); From 6b37faa175311128dc920aaa57a5f7fab85537d7 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 6 Oct 2008 21:54:41 +0000 Subject: [PATCH 16/19] [CIFS] fix some settings of cifsAttrs after calling SetFileInfo and SetPathInfo We only need to set them when we call SetFileInfo or SetPathInfo directly, and as soon as possible after then. We had one place setting it where it didn't need to be, and another place where it was missing. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index db091c516c2a..e387ed3f9446 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -729,7 +729,10 @@ cifs_set_file_info(struct inode *inode, struct iattr *attrs, int xid, &info_buf, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc != -EOPNOTSUPP && rc != -EINVAL) + if (rc == 0) { + cifsInode->cifsAttrs = dosattr; + goto out; + } else if (rc != -EOPNOTSUPP && rc != -EINVAL) goto out; } @@ -805,6 +808,7 @@ cifs_rename_pending_delete(char *full_path, struct inode *inode, int xid) kfree(info_buf); if (rc != 0) goto out_close; + cifsInode->cifsAttrs = dosattr; /* silly-rename the file */ CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, @@ -905,7 +909,6 @@ psx_del_no_retry: if (rc == 0) drop_nlink(inode); } - cifsInode->cifsAttrs = dosattr; } out_reval: if (inode) { @@ -963,7 +966,7 @@ static void posix_fill_in_inode(struct inode *tmp_inode, int cifs_mkdir(struct inode *inode, struct dentry *direntry, int mode) { - int rc = 0; + int rc = 0, tmprc; int xid; struct cifs_sb_info *cifs_sb; struct cifsTconInfo *pTcon; @@ -1025,6 +1028,7 @@ int cifs_mkdir(struct inode *inode, struct dentry *direntry, int mode) kfree(pInfo); goto mkdir_get_info; } + /* Is an i_ino of zero legal? */ /* Are there sanity checks we can use to ensure that the server is really filling in that field? */ @@ -1113,12 +1117,20 @@ mkdir_get_info: if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) && (mode & S_IWUGO) == 0) { FILE_BASIC_INFO pInfo; + struct cifsInodeInfo *cifsInode; + u32 dosattrs; + memset(&pInfo, 0, sizeof(pInfo)); - pInfo.Attributes = cpu_to_le32(ATTR_READONLY); - CIFSSMBSetPathInfo(xid, pTcon, full_path, - &pInfo, cifs_sb->local_nls, + cifsInode = CIFS_I(newinode); + dosattrs = cifsInode->cifsAttrs|ATTR_READONLY; + pInfo.Attributes = cpu_to_le32(dosattrs); + tmprc = CIFSSMBSetPathInfo(xid, pTcon, + full_path, &pInfo, + cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); + if (tmprc == 0) + cifsInode->cifsAttrs = dosattrs; } if (direntry->d_inode) { if (cifs_sb->mnt_cifs_flags & From 6050247d8089037d6d8ea0f3c62fe4a931c1ab14 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 7 Oct 2008 18:42:52 +0000 Subject: [PATCH 17/19] [CIFS] clean up error handling in cifs_unlink Currently, if a standard delete fails and we end up getting -EACCES we try to clear ATTR_READONLY and try the delete again. If that then fails with -ETXTBSY then we try a rename_pending_delete. We aren't handling other errors appropriately though. Another client could have deleted the file in the meantime and we get back -ENOENT, for instance. In that case we wouldn't do a d_drop. Instead of retrying in a separate call, simply goto the original call and use the error handling from that. Also, we weren't properly undoing any attribute changes that were done before returning an error back to the caller. CC: Jeff Layton Signed-off-by: Steve French --- fs/cifs/inode.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index e387ed3f9446..a8c833345fc9 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -837,12 +837,12 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) int xid; char *full_path = NULL; struct inode *inode = dentry->d_inode; - struct cifsInodeInfo *cifsInode; + struct cifsInodeInfo *cifsInode = CIFS_I(inode); struct super_block *sb = dir->i_sb; struct cifs_sb_info *cifs_sb = CIFS_SB(sb); struct cifsTconInfo *tcon = cifs_sb->tcon; - struct iattr *attrs; - __u32 dosattr; + struct iattr *attrs = NULL; + __u32 dosattr = 0, origattr = 0; cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry)); @@ -867,8 +867,10 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) goto psx_del_no_retry; } +retry_std_delete: rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); + psx_del_no_retry: if (!rc) { if (inode) @@ -879,8 +881,7 @@ psx_del_no_retry: rc = cifs_rename_pending_delete(full_path, inode, xid); if (rc == 0) drop_nlink(inode); - } else if (rc == -EACCES) { - /* try only if r/o attribute set in local lookup data? */ + } else if (rc == -EACCES && dosattr == 0) { attrs = kzalloc(sizeof(*attrs), GFP_KERNEL); if (attrs == NULL) { rc = -ENOMEM; @@ -888,28 +889,25 @@ psx_del_no_retry: } /* try to reset dos attributes */ - cifsInode = CIFS_I(inode); - dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY; + origattr = cifsInode->cifsAttrs; + if (origattr == 0) + origattr |= ATTR_NORMAL; + dosattr = origattr & ~ATTR_READONLY; if (dosattr == 0) dosattr |= ATTR_NORMAL; dosattr |= ATTR_HIDDEN; rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr); - kfree(attrs); if (rc != 0) goto out_reval; - rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls, - cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc == 0) { - if (inode) - drop_nlink(inode); - } else if (rc == -ETXTBSY) { - rc = cifs_rename_pending_delete(full_path, inode, xid); - if (rc == 0) - drop_nlink(inode); - } + + goto retry_std_delete; } + + /* undo the setattr if we errored out and it's needed */ + if (rc != 0 && dosattr != 0) + cifs_set_file_info(inode, attrs, xid, full_path, origattr); + out_reval: if (inode) { cifsInode = CIFS_I(inode); @@ -919,9 +917,10 @@ out_reval: } dir->i_ctime = dir->i_mtime = current_fs_time(sb); cifsInode = CIFS_I(dir); - cifsInode->time = 0; /* force revalidate of dir as well */ + CIFS_I(dir)->time = 0; /* force revalidate of dir as well */ kfree(full_path); + kfree(attrs); FreeXid(xid); return rc; } From 0752f1522a9120f731232919f7ad904e9e22b8ce Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 7 Oct 2008 20:03:33 +0000 Subject: [PATCH 18/19] [CIFS] make sure we have the right resume info before calling CIFSFindNext When we do a seekdir() or equivalent, we usually end up doing a FindFirst call and then call FindNext until we get to the offset that we want. The problem is that when we call FindNext, the code usually doesn't have the proper info (mostly, the filename of the entry from the last search) to resume the search. Add a "last_entry" field to the cifs_search_info that points to the last entry in the search. We calculate this pointer by using the LastNameOffset field from the search parms that are returned. We then use that info to do a cifs_save_resume_key before we call CIFSFindNext. This patch allows CIFS to reliably pass the "telldir" connectathon test. Signed-off-by: Jeff Layton CC: Stable Signed-off-by: Steve French --- fs/cifs/cifsglob.h | 1 + fs/cifs/cifssmb.c | 4 ++ fs/cifs/readdir.c | 128 +++++++++++++++++++++++---------------------- 3 files changed, 70 insertions(+), 63 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 8dfd6f24d488..0d22479d99b7 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -309,6 +309,7 @@ struct cifs_search_info { __u32 resume_key; char *ntwrk_buf_start; char *srch_entries_start; + char *last_entry; char *presume_name; unsigned int resume_name_len; bool endOfSearch:1; diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 7504d1514c77..7b00a16e1352 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -3636,6 +3636,8 @@ findFirstRetry: le16_to_cpu(parms->SearchCount); psrch_inf->index_of_last_entry = 2 /* skip . and .. */ + psrch_inf->entries_in_buffer; + psrch_inf->last_entry = psrch_inf->srch_entries_start + + le16_to_cpu(parms->LastNameOffset); *pnetfid = parms->SearchHandle; } else { cifs_buf_release(pSMB); @@ -3751,6 +3753,8 @@ int CIFSFindNext(const int xid, struct cifsTconInfo *tcon, le16_to_cpu(parms->SearchCount); psrch_inf->index_of_last_entry += psrch_inf->entries_in_buffer; + psrch_inf->last_entry = psrch_inf->srch_entries_start + + le16_to_cpu(parms->LastNameOffset); /* cFYI(1,("fnxt2 entries in buf %d index_of_last %d", psrch_inf->entries_in_buffer, psrch_inf->index_of_last_entry)); */ diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 5f40ed3473f5..765adf12d54f 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -640,6 +640,70 @@ static int is_dir_changed(struct file *file) } +static int cifs_save_resume_key(const char *current_entry, + struct cifsFileInfo *cifsFile) +{ + int rc = 0; + unsigned int len = 0; + __u16 level; + char *filename; + + if ((cifsFile == NULL) || (current_entry == NULL)) + return -EINVAL; + + level = cifsFile->srch_inf.info_level; + + if (level == SMB_FIND_FILE_UNIX) { + FILE_UNIX_INFO *pFindData = (FILE_UNIX_INFO *)current_entry; + + filename = &pFindData->FileName[0]; + if (cifsFile->srch_inf.unicode) { + len = cifs_unicode_bytelen(filename); + } else { + /* BB should we make this strnlen of PATH_MAX? */ + len = strnlen(filename, PATH_MAX); + } + cifsFile->srch_inf.resume_key = pFindData->ResumeKey; + } else if (level == SMB_FIND_FILE_DIRECTORY_INFO) { + FILE_DIRECTORY_INFO *pFindData = + (FILE_DIRECTORY_INFO *)current_entry; + filename = &pFindData->FileName[0]; + len = le32_to_cpu(pFindData->FileNameLength); + cifsFile->srch_inf.resume_key = pFindData->FileIndex; + } else if (level == SMB_FIND_FILE_FULL_DIRECTORY_INFO) { + FILE_FULL_DIRECTORY_INFO *pFindData = + (FILE_FULL_DIRECTORY_INFO *)current_entry; + filename = &pFindData->FileName[0]; + len = le32_to_cpu(pFindData->FileNameLength); + cifsFile->srch_inf.resume_key = pFindData->FileIndex; + } else if (level == SMB_FIND_FILE_ID_FULL_DIR_INFO) { + SEARCH_ID_FULL_DIR_INFO *pFindData = + (SEARCH_ID_FULL_DIR_INFO *)current_entry; + filename = &pFindData->FileName[0]; + len = le32_to_cpu(pFindData->FileNameLength); + cifsFile->srch_inf.resume_key = pFindData->FileIndex; + } else if (level == SMB_FIND_FILE_BOTH_DIRECTORY_INFO) { + FILE_BOTH_DIRECTORY_INFO *pFindData = + (FILE_BOTH_DIRECTORY_INFO *)current_entry; + filename = &pFindData->FileName[0]; + len = le32_to_cpu(pFindData->FileNameLength); + cifsFile->srch_inf.resume_key = pFindData->FileIndex; + } else if (level == SMB_FIND_FILE_INFO_STANDARD) { + FIND_FILE_STANDARD_INFO *pFindData = + (FIND_FILE_STANDARD_INFO *)current_entry; + filename = &pFindData->FileName[0]; + /* one byte length, no name conversion */ + len = (unsigned int)pFindData->FileNameLength; + cifsFile->srch_inf.resume_key = pFindData->ResumeKey; + } else { + cFYI(1, ("Unknown findfirst level %d", level)); + return -EINVAL; + } + cifsFile->srch_inf.resume_name_len = len; + cifsFile->srch_inf.presume_name = filename; + return rc; +} + /* find the corresponding entry in the search */ /* Note that the SMB server returns search entries for . and .. which complicates logic here if we choose to parse for them and we do not @@ -703,6 +767,7 @@ static int find_cifs_entry(const int xid, struct cifsTconInfo *pTcon, while ((index_to_find >= cifsFile->srch_inf.index_of_last_entry) && (rc == 0) && !cifsFile->srch_inf.endOfSearch) { cFYI(1, ("calling findnext2")); + cifs_save_resume_key(cifsFile->srch_inf.last_entry, cifsFile); rc = CIFSFindNext(xid, pTcon, cifsFile->netfid, &cifsFile->srch_inf); if (rc) @@ -919,69 +984,6 @@ static int cifs_filldir(char *pfindEntry, struct file *file, return rc; } -static int cifs_save_resume_key(const char *current_entry, - struct cifsFileInfo *cifsFile) -{ - int rc = 0; - unsigned int len = 0; - __u16 level; - char *filename; - - if ((cifsFile == NULL) || (current_entry == NULL)) - return -EINVAL; - - level = cifsFile->srch_inf.info_level; - - if (level == SMB_FIND_FILE_UNIX) { - FILE_UNIX_INFO *pFindData = (FILE_UNIX_INFO *)current_entry; - - filename = &pFindData->FileName[0]; - if (cifsFile->srch_inf.unicode) { - len = cifs_unicode_bytelen(filename); - } else { - /* BB should we make this strnlen of PATH_MAX? */ - len = strnlen(filename, PATH_MAX); - } - cifsFile->srch_inf.resume_key = pFindData->ResumeKey; - } else if (level == SMB_FIND_FILE_DIRECTORY_INFO) { - FILE_DIRECTORY_INFO *pFindData = - (FILE_DIRECTORY_INFO *)current_entry; - filename = &pFindData->FileName[0]; - len = le32_to_cpu(pFindData->FileNameLength); - cifsFile->srch_inf.resume_key = pFindData->FileIndex; - } else if (level == SMB_FIND_FILE_FULL_DIRECTORY_INFO) { - FILE_FULL_DIRECTORY_INFO *pFindData = - (FILE_FULL_DIRECTORY_INFO *)current_entry; - filename = &pFindData->FileName[0]; - len = le32_to_cpu(pFindData->FileNameLength); - cifsFile->srch_inf.resume_key = pFindData->FileIndex; - } else if (level == SMB_FIND_FILE_ID_FULL_DIR_INFO) { - SEARCH_ID_FULL_DIR_INFO *pFindData = - (SEARCH_ID_FULL_DIR_INFO *)current_entry; - filename = &pFindData->FileName[0]; - len = le32_to_cpu(pFindData->FileNameLength); - cifsFile->srch_inf.resume_key = pFindData->FileIndex; - } else if (level == SMB_FIND_FILE_BOTH_DIRECTORY_INFO) { - FILE_BOTH_DIRECTORY_INFO *pFindData = - (FILE_BOTH_DIRECTORY_INFO *)current_entry; - filename = &pFindData->FileName[0]; - len = le32_to_cpu(pFindData->FileNameLength); - cifsFile->srch_inf.resume_key = pFindData->FileIndex; - } else if (level == SMB_FIND_FILE_INFO_STANDARD) { - FIND_FILE_STANDARD_INFO *pFindData = - (FIND_FILE_STANDARD_INFO *)current_entry; - filename = &pFindData->FileName[0]; - /* one byte length, no name conversion */ - len = (unsigned int)pFindData->FileNameLength; - cifsFile->srch_inf.resume_key = pFindData->ResumeKey; - } else { - cFYI(1, ("Unknown findfirst level %d", level)); - return -EINVAL; - } - cifsFile->srch_inf.resume_name_len = len; - cifsFile->srch_inf.presume_name = filename; - return rc; -} int cifs_readdir(struct file *file, void *direntry, filldir_t filldir) { From b77d753c413e02559669df66e543869dad40c847 Mon Sep 17 00:00:00 2001 From: Steve French Date: Wed, 8 Oct 2008 19:13:46 +0000 Subject: [PATCH 19/19] [CIFS] Check that last search entry resume key is valid Jeff's recent patch to add a last_entry field in the search structure to better construct resume keys did not validate that the server sent us a plausible pointer to the last entry. This adds that. Signed-off-by: Steve French --- fs/cifs/cifssmb.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 7b00a16e1352..6f4ffe15d68d 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -3614,6 +3614,8 @@ findFirstRetry: /* BB remember to free buffer if error BB */ rc = validate_t2((struct smb_t2_rsp *)pSMBr); if (rc == 0) { + unsigned int lnoff; + if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) psrch_inf->unicode = true; else @@ -3636,8 +3638,17 @@ findFirstRetry: le16_to_cpu(parms->SearchCount); psrch_inf->index_of_last_entry = 2 /* skip . and .. */ + psrch_inf->entries_in_buffer; + lnoff = le16_to_cpu(parms->LastNameOffset); + if (tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE < + lnoff) { + cERROR(1, ("ignoring corrupt resume name")); + psrch_inf->last_entry = NULL; + return rc; + } + psrch_inf->last_entry = psrch_inf->srch_entries_start + - le16_to_cpu(parms->LastNameOffset); + lnoff; + *pnetfid = parms->SearchHandle; } else { cifs_buf_release(pSMB); @@ -3727,6 +3738,8 @@ int CIFSFindNext(const int xid, struct cifsTconInfo *tcon, rc = validate_t2((struct smb_t2_rsp *)pSMBr); if (rc == 0) { + unsigned int lnoff; + /* BB fixme add lock for file (srch_info) struct here */ if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) psrch_inf->unicode = true; @@ -3753,8 +3766,16 @@ int CIFSFindNext(const int xid, struct cifsTconInfo *tcon, le16_to_cpu(parms->SearchCount); psrch_inf->index_of_last_entry += psrch_inf->entries_in_buffer; - psrch_inf->last_entry = psrch_inf->srch_entries_start + - le16_to_cpu(parms->LastNameOffset); + lnoff = le16_to_cpu(parms->LastNameOffset); + if (tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE < + lnoff) { + cERROR(1, ("ignoring corrupt resume name")); + psrch_inf->last_entry = NULL; + return rc; + } else + psrch_inf->last_entry = + psrch_inf->srch_entries_start + lnoff; + /* cFYI(1,("fnxt2 entries in buf %d index_of_last %d", psrch_inf->entries_in_buffer, psrch_inf->index_of_last_entry)); */