Commit 8b6427a2 authored by Jeff Layton's avatar Jeff Layton Committed by Steve French

cifs: fix pointer initialization and checks in cifs_follow_symlink (try #4)

This is the third respin of the patch posted yesterday to fix the error
handling in cifs_follow_symlink. It also includes a fix for a bogus NULL
pointer check in CIFSSMBQueryUnixSymLink that Jeff Moyer spotted.

It's possible for CIFSSMBQueryUnixSymLink to return without setting
target_path to a valid pointer. If that happens then the current value
to which we're initializing this pointer could cause an oops when it's
kfree'd.

This patch is a little more comprehensive than the last patches. It
reorganizes cifs_follow_link a bit for (hopefully) better readability.
It should also eliminate the uneeded allocation of full_path on servers
without unix extensions (assuming they can get to this point anyway, of
which I'm not convinced).

On a side note, I'm not sure I agree with the logic of enabling this
query even when unix extensions are disabled on the client. It seems
like that should disable this as well. But, changing that is outside the
scope of this fix, so I've left it alone for now.
Reported-by: default avatarJeff Moyer <jmoyer@redhat.com>
Signed-off-by: default avatarJeff Layton <jlayton@redhat.com>
Reviewed-by: default avatarJeff Moyer <jmoyer@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@inraded.com>
Signed-off-by: default avatarSteve French <sfrench@us.ibm.com>
parent b41a080f
...@@ -2475,7 +2475,7 @@ CIFSSMBUnixQuerySymLink(const int xid, struct cifsTconInfo *tcon, ...@@ -2475,7 +2475,7 @@ CIFSSMBUnixQuerySymLink(const int xid, struct cifsTconInfo *tcon,
/* BB FIXME investigate remapping reserved chars here */ /* BB FIXME investigate remapping reserved chars here */
*symlinkinfo = cifs_strndup_from_ucs(data_start, count, *symlinkinfo = cifs_strndup_from_ucs(data_start, count,
is_unicode, nls_codepage); is_unicode, nls_codepage);
if (!symlinkinfo) if (!*symlinkinfo)
rc = -ENOMEM; rc = -ENOMEM;
} }
} }
......
...@@ -107,48 +107,48 @@ void * ...@@ -107,48 +107,48 @@ void *
cifs_follow_link(struct dentry *direntry, struct nameidata *nd) cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
{ {
struct inode *inode = direntry->d_inode; struct inode *inode = direntry->d_inode;
int rc = -EACCES; int rc = -ENOMEM;
int xid; int xid;
char *full_path = NULL; char *full_path = NULL;
char *target_path = ERR_PTR(-ENOMEM); char *target_path = NULL;
struct cifs_sb_info *cifs_sb; struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
struct cifsTconInfo *pTcon; struct cifsTconInfo *tcon = cifs_sb->tcon;
xid = GetXid(); xid = GetXid();
full_path = build_path_from_dentry(direntry); /*
* For now, we just handle symlinks with unix extensions enabled.
* Eventually we should handle NTFS reparse points, and MacOS
* symlink support. For instance...
*
* rc = CIFSSMBQueryReparseLinkInfo(...)
*
* For now, just return -EACCES when the server doesn't support posix
* extensions. Note that we still allow querying symlinks when posix
* extensions are manually disabled. We could disable these as well
* but there doesn't seem to be any harm in allowing the client to
* read them.
*/
if (!(tcon->ses->capabilities & CAP_UNIX)) {
rc = -EACCES;
goto out;
}
full_path = build_path_from_dentry(direntry);
if (!full_path) if (!full_path)
goto out; goto out;
cFYI(1, ("Full path: %s inode = 0x%p", full_path, inode)); cFYI(1, ("Full path: %s inode = 0x%p", full_path, inode));
cifs_sb = CIFS_SB(inode->i_sb);
pTcon = cifs_sb->tcon;
/* We could change this to:
if (pTcon->unix_ext)
but there does not seem any point in refusing to
get symlink info if we can, even if unix extensions
turned off for this mount */
if (pTcon->ses->capabilities & CAP_UNIX)
rc = CIFSSMBUnixQuerySymLink(xid, pTcon, full_path,
&target_path,
cifs_sb->local_nls);
else {
/* BB add read reparse point symlink code here */
/* rc = CIFSSMBQueryReparseLinkInfo */
/* BB Add code to Query ReparsePoint info */
/* BB Add MAC style xsymlink check here if enabled */
}
rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
cifs_sb->local_nls);
kfree(full_path);
out:
if (rc != 0) { if (rc != 0) {
kfree(target_path); kfree(target_path);
target_path = ERR_PTR(rc); target_path = ERR_PTR(rc);
} }
kfree(full_path);
out:
FreeXid(xid); FreeXid(xid);
nd_set_link(nd, target_path); nd_set_link(nd, target_path);
return NULL; return NULL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment