From 8367224b2e90eb716dc54f3d83cd73b7efb2ea30 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Thu, 13 Dec 2012 06:02:51 -0500 Subject: cifs: fix double-free of "string" in cifs_parse_mount_options Dan reported the following regression in commit d387a5c5: + fs/cifs/connect.c:1903 cifs_parse_mount_options() error: double free of 'string' That patch has some of the new option parsing code free "string" without setting the variable to NULL afterward. Since "string" is automatically freed in an error condition, fix the code to just rely on that instead of freeing it explicitly. Reported-by: Dan Carpenter Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 7635b5db26a..17c3643e595 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1624,14 +1624,11 @@ cifs_parse_mount_options(const char *mountdata, const char *devname, case Opt_unc: string = vol->UNC; vol->UNC = match_strdup(args); - if (vol->UNC == NULL) { - kfree(string); + if (vol->UNC == NULL) goto out_nomem; - } convert_delimiter(vol->UNC, '\\'); if (vol->UNC[0] != '\\' || vol->UNC[1] != '\\') { - kfree(string); printk(KERN_ERR "CIFS: UNC Path does not " "begin with // or \\\\\n"); goto cifs_parse_mount_err; @@ -1687,10 +1684,8 @@ cifs_parse_mount_options(const char *mountdata, const char *devname, string = vol->prepath; vol->prepath = match_strdup(args); - if (vol->prepath == NULL) { - kfree(string); + if (vol->prepath == NULL) goto out_nomem; - } /* Compare old prefixpath= option to new one */ if (!string || strcmp(string, vol->prepath)) printk(KERN_WARNING "CIFS: the value of the " -- cgit v1.2.3 From 2f2591a34db6c9361faa316c91a6e320cb4e6aee Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 18 Dec 2012 06:35:10 -0500 Subject: cifs: don't compare uniqueids in cifs_prime_dcache unless server inode numbers are in use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oliver reported that commit cd60042c caused his cifs mounts to continually thrash through new inodes on readdir. His servers are not sending inode numbers (or he's not using them), and the new test in that function doesn't account for that sort of setup correctly. If we're not using server inode numbers, then assume that the inode attached to the dentry hasn't changed. Go ahead and update the attributes in place, but keep the same inode number. Cc: # v3.5+ Reported-and-Tested-by: Oliver Mössinger Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/readdir.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 6002fdc920a..cdd6ff48246 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -78,6 +78,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, struct dentry *dentry, *alias; struct inode *inode; struct super_block *sb = parent->d_inode->i_sb; + struct cifs_sb_info *cifs_sb = CIFS_SB(sb); cFYI(1, "%s: for %s", __func__, name->name); @@ -91,10 +92,20 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, int err; inode = dentry->d_inode; - /* update inode in place if i_ino didn't change */ - if (inode && CIFS_I(inode)->uniqueid == fattr->cf_uniqueid) { - cifs_fattr_to_inode(inode, fattr); - goto out; + if (inode) { + /* + * If we're generating inode numbers, then we don't + * want to clobber the existing one with the one that + * the readdir code created. + */ + if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) + fattr->cf_uniqueid = CIFS_I(inode)->uniqueid; + + /* update inode in place if i_ino didn't change */ + if (CIFS_I(inode)->uniqueid == fattr->cf_uniqueid) { + cifs_fattr_to_inode(inode, fattr); + goto out; + } } err = d_invalidate(dentry); dput(dentry); -- cgit v1.2.3 From 9acbd26b0a5ac4a3d52d31034feb3d935e39032a Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 18 Dec 2012 06:35:10 -0500 Subject: cifs: eliminate cifsERROR variable It's always set to "1" and there's no way to change it to anything else. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifs_debug.h | 6 +----- fs/cifs/cifsfs.c | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h index 86e92ef2abc..69ae3d3c3b3 100644 --- a/fs/cifs/cifs_debug.h +++ b/fs/cifs/cifs_debug.h @@ -37,7 +37,6 @@ void dump_smb(void *, int); #define CIFS_TIMER 0x04 extern int cifsFYI; -extern int cifsERROR; /* * debug ON @@ -64,10 +63,7 @@ do { \ /* error event message: e.g., i/o error */ #define cifserror(fmt, ...) \ -do { \ - if (cifsERROR) \ - printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__); \ -} while (0) + printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__); \ #define cERROR(set, fmt, ...) \ do { \ diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index ce9f3c5421b..f653835d067 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -54,7 +54,6 @@ #endif int cifsFYI = 0; -int cifsERROR = 1; int traceSMB = 0; bool enable_oplocks = true; unsigned int linuxExtEnabled = 1; -- cgit v1.2.3