From: Jeff Layton <jlayton@redhat.com>
Date: Tue, 19 May 2009 12:38:09 -0400
Subject: [fs] cifs: fix pointer and checks in cifs_follow_symlink
Message-id: 1242751089-14570-1-git-send-email-jlayton@redhat.com
O-Subject: [RHEL5.4 PATCH] BZ#496577: cifs: fix pointer initialization and checks in cifs_follow_symlink
Bugzilla: 496577
RH-Acked-by: Jeff Moyer <jmoyer@redhat.com>
RH-Nacked-by: Jeff Layton <jlayton@redhat.com>
CVE: CVE-2009-1633
RH-Acked-by: Josef Bacik <josef@redhat.com>
RH-Acked-by: Peter Staubach <staubach@redhat.com>

Upstream commit: 8b6427a2a8f7dd43e9208fb33a3b116d66db4979

This is a respin of the patch that I posted yesterday. It includes the
fix for a bogus NULL pointer check in CIFSSMBQueryUnixSymLink that Jeff
Moyer spotted. I also took the liberty of cleaning up cifs_follow_link
for better readability and efficiency.

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 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).

The upstream version of this was tested by running connectathon and
fsstress on it overnight, and the RHEL5 version was (more lightly)
tested this morning using the same tests.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 3a08aa2..650ceeb 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -2479,7 +2479,7 @@ querySymLinkRetry:
 						    pSMBr->hdr.Flags2 &
 							SMBFLG2_UNICODE,
 						    nls_codepage);
-			if (!symlinkinfo)
+			if (!*symlinkinfo)
 				rc = -ENOMEM;
 		}
 	}
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 75027f8..146daf4 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -110,48 +110,48 @@ void *
 cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
 {
 	struct inode *inode = direntry->d_inode;
-	int rc = -EACCES;
+	int rc = -ENOMEM;
 	int xid;
 	char *full_path = NULL;
-	char *target_path = ERR_PTR(-ENOMEM);
-	struct cifs_sb_info *cifs_sb;
-	struct cifsTconInfo *pTcon;
+	char *target_path = NULL;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	struct cifsTconInfo *tcon = cifs_sb->tcon;
 
 	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)
 		goto out;
 
 	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) {
 		kfree(target_path);
 		target_path = ERR_PTR(rc);
 	}
 
-	kfree(full_path);
-out:
 	FreeXid(xid);
 	nd_set_link(nd, target_path);
 	return NULL;