[PATCH v1] cifs: make shares unaccessible at root level mountable

["Aurélien Aptel" <aaptel-IBi9RG/b67k@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1656 bytes --]

Hi,

I've been working on solving this bug [1] for some time now. You can
find some of my research notes here [2]. I was recently able to
finally make it work.

The patch is based on Shirish's patch (available at [1]) to which I've
made the following modification:

- create disconnected root when any intermediary path are "not
  found" (ENOENT). Previously it was only done for EACCES errors.

- fix build_path_from_dentry() to correctly prefix dentry that are
  children of a disconnected root with the disconnected root actual
  path. This path is only avaible when using the struct rdelem linked
  list stored in the cifs superblock i.e. when we mount with
  noserverino.

So basically if you have the bug described in [1] you can solve it by
applying this patch and mounting with -o noserverino. I would like to
get rid this restriction (noserverino) and unconditionnaly use the
rdelem linked list but I don't know how feasible/safe that is (I'm still
new to all this stuff, this is my first kernel patch).

Shirish, I'd like to hear your comments if you have any. I've noticed
your find_rdelem_by_dentry() removes the dentry from the rdelem list
and I'm not sure I understand why. I've added and used
find_rdelem_by_dentry_no_del() to do the same thing without removing it.

1: https://bugzilla.samba.org/show_bug.cgi?id=8950
2: http://diobla.info/doc/suse-todo#bnc799133

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
Nürnberg)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-cifs-make-shares-unaccessible-at-root-level-mountabl.patch --]
[-- Type: text/x-patch, Size: 16403 bytes --]

From 92520bbeef256957c150df01f946ad8890140a78 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
Date: Thu, 5 Nov 2015 15:24:20 +0100
Subject: [PATCH] cifs: make shares unaccessible at root level mountable

Based on Shirish Pargaonkar's disconnected root patch [1].

The initial patch was modified to create a disconnected root when any
intermediary path is unaccessible (EACCESS) but *also* when "not
found" (ENOENT).

The other change was to modify build_path_from_dentry() in dir.c so that
if it has to build a path for a dentry that uses a disconnected root, it
prefixes that path with the disconnected root actual path.

Previously, when called with a dentry that had a disconnected root,
build_path_path() would stop when it reached that disconnected root,
which is not the same as the root of the share, resulting in a wrong
output path.

1: https://bugzilla.samba.org/show_bug.cgi?id=8950

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifs_fs_sb.h |  10 +++
 fs/cifs/cifsfs.c     |  84 +++++++++++++++++++-
 fs/cifs/cifsproto.h  |   1 +
 fs/cifs/connect.c    |   3 +
 fs/cifs/dir.c        | 217 ++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/cifs/inode.c      |  36 ++++++++-
 6 files changed, 325 insertions(+), 26 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index 3182273..b2b65f6 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -67,5 +67,15 @@ struct cifs_sb_info {
 	struct backing_dev_info bdi;
 	struct delayed_work prune_tlinks;
 	struct rcu_head rcu;
+	struct list_head rtdislist; /* list of disconnected root dentries */
+	spinlock_t rtdislock; /* lock for disconnected root dentry list */
+};
+
+struct cifs_rdelem {
+	int                     rdcount;
+	struct list_head        rdlist;
+	char *                  rdname;
+	struct dentry *		rdentry;
+	struct inode *          rdinode;
 };
 #endif				/* _CIFS_FS_SB_H */
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 6a1119e..55160df 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -584,6 +584,80 @@ static const struct super_operations cifs_super_ops = {
 #endif
 };
 
+void
+cifs_free_rdelem(struct cifs_rdelem *rdelem)
+{
+	kfree(rdelem->rdname);
+	kfree(rdelem);
+}
+
+struct cifs_rdelem *
+cifs_alloc_rdelem(char *full_path, struct dentry *rdentry,
+				struct inode *rdinode)
+{
+	struct cifs_rdelem *rdelem;
+	rdelem = kmalloc(sizeof(struct cifs_rdelem), GFP_KERNEL);
+	if (!rdelem) {
+		cifs_dbg(FYI, "%s Can't allocate root dentry\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	rdelem->rdname = kstrdup(full_path, GFP_KERNEL);
+
+	if (!rdelem->rdname) {
+		cifs_dbg(FYI, "%s Can't allocate root dentry name\n", __func__);
+		kfree(rdelem);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	rdelem->rdinode = rdinode;
+	rdelem->rdentry = rdentry;
+
+	return rdelem;
+}
+
+static struct dentry *
+create_root_dis_dentry(struct super_block *sb, struct inode *rinode,
+			char *fpath)
+{
+	int rc;
+	unsigned int xid;
+	struct dentry *dentry = NULL;
+	struct cifs_rdelem *rdelem = NULL;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
+
+	xid = get_xid();
+	if (tcon->unix_ext)
+		rc = cifs_get_inode_info_unix(&rinode, fpath, sb, xid);
+	else
+		rc = cifs_get_inode_info(&rinode, fpath, NULL, sb, xid, NULL);
+	free_xid(xid);
+
+	if ((rc == 0) && (rinode != NULL)) {
+		dentry = d_obtain_alias(rinode);
+		if (IS_ERR(dentry)) {
+			iput(rinode);
+			goto rdelem_ret;
+		}
+
+		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
+
+			rdelem =
+				cifs_alloc_rdelem(fpath, dentry, rinode);
+			if (IS_ERR(rdelem))
+				goto rdelem_ret;
+			spin_lock(&cifs_sb->rtdislock);
+			list_add(&rdelem->rdlist, &cifs_sb->rtdislist);
+			spin_unlock(&cifs_sb->rtdislock);
+		}
+		return dentry;
+	}
+
+rdelem_ret:
+	return ERR_PTR(-EACCES);
+}
+
 /*
  * Get root dentry from superblock according to prefix path mount option.
  * Return dentry with refcount + 1 on success and NULL otherwise.
@@ -596,9 +670,10 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
 	char *full_path = NULL;
 	char *s, *p;
 	char sep;
+	struct inode *rinode = NULL;
+	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 
-	full_path = cifs_build_path_to_root(vol, cifs_sb,
-					    cifs_sb_master_tcon(cifs_sb));
+	full_path = cifs_build_path_to_root(vol, cifs_sb, tcon);
 	if (full_path == NULL)
 		return ERR_PTR(-ENOMEM);
 
@@ -639,6 +714,11 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
 		dput(dentry);
 		dentry = child;
 	} while (!IS_ERR(dentry));
+
+	if (IS_ERR(dentry) && (PTR_ERR(dentry) == -EACCES || PTR_ERR(dentry) == -ENOENT) && *s) {
+		dentry = create_root_dis_dentry(sb, rinode, full_path);
+	}
+
 	kfree(full_path);
 	return dentry;
 }
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index c63fd1d..b0a813f 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -205,6 +205,7 @@ extern void cifs_add_pending_open_locked(struct cifs_fid *fid,
 					 struct tcon_link *tlink,
 					 struct cifs_pending_open *open);
 extern void cifs_del_pending_open(struct cifs_pending_open *open);
+extern void cifs_free_rdelem(struct cifs_rdelem *);
 
 #if IS_ENABLED(CONFIG_CIFS_DFS_UPCALL)
 extern void cifs_dfs_release_automount_timer(void);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 773f4dc..018c156 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3190,6 +3190,9 @@ void cifs_setup_cifs_sb(struct smb_vol *pvolume_info,
 	spin_lock_init(&cifs_sb->tlink_tree_lock);
 	cifs_sb->tlink_tree = RB_ROOT;
 
+	spin_lock_init(&cifs_sb->rtdislock);
+	INIT_LIST_HEAD(&cifs_sb->rtdislist);
+
 	/*
 	 * Temporarily set r/wsize for matching superblock. If we end up using
 	 * new sb then client will later negotiate it downward if needed.
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index c3eb998..bc02265 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -77,6 +77,9 @@ cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb,
 	return full_path;
 }
 
+static struct cifs_rdelem * find_rdelem_by_dentry_no_del(const struct dentry *rdentry,
+							 struct cifs_sb_info * cifs_sb);
+
 /* Note: caller must free return buffer */
 char *
 build_path_from_dentry(struct dentry *direntry)
@@ -89,6 +92,25 @@ build_path_from_dentry(struct dentry *direntry)
 	struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb);
 	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 	unsigned seq;
+	struct cifs_rdelem *dis_root = NULL;
+	size_t dis_root_len = 0;
+
+	/*
+	 * First look if the dentry has a disconnected root in its
+	 * parent hierarchy. If that's the case we must append that
+	 * root path as a prefix.
+	 */
+
+	rcu_read_lock();
+	for (temp = direntry; temp && !IS_ROOT(temp); temp = temp->d_parent);
+	if (temp && (dis_root = find_rdelem_by_dentry_no_del(temp, cifs_sb)))
+		dis_root_len = strlen(dis_root->rdname);
+	rcu_read_unlock();
+
+	/*
+	 * Now let's compute the length of the path so that we can
+	 * allocate the right size
+	 */
 
 	dirsep = CIFS_DIR_SEP(cifs_sb);
 	if (tcon->Flags & SMB_SHARE_IS_IN_DFS)
@@ -108,8 +130,19 @@ cifs_bp_rename_retry:
 			return NULL;
 		}
 	}
+	if (dis_root && temp == dis_root->rdentry) {
+		/*
+		 *  Account for the disconnected root prefix
+		 *  Note: leading dir separator already in the length
+		 */
+		namelen += dis_root_len;
+	}
 	rcu_read_unlock();
 
+	/*
+	 * Next step is to actually fill the full_path string
+	 */
+
 	full_path = kmalloc(namelen+1, GFP_KERNEL);
 	if (full_path == NULL)
 		return full_path;
@@ -136,7 +169,12 @@ cifs_bp_rename_retry:
 			return NULL;
 		}
 	}
+	if (dis_root && temp == dis_root->rdentry) {
+		namelen -= dis_root_len;
+		strncpy(full_path + namelen, dis_root->rdname, dis_root_len);
+	}
 	rcu_read_unlock();
+
 	if (namelen != dfsplen || read_seqretry(&rename_lock, seq)) {
 		cifs_dbg(FYI, "did not end path lookup where expected. namelen=%ddfsplen=%d\n",
 			 namelen, dfsplen);
@@ -704,6 +742,73 @@ mknod_out:
 	return rc;
 }
 
+static struct cifs_rdelem *
+find_rdelem_by_inode(struct inode *rdinode, struct cifs_sb_info * cifs_sb)
+{
+	struct cifs_rdelem *rdelem;
+	spin_lock(&cifs_sb->rtdislock);
+	list_for_each_entry(rdelem, &cifs_sb->rtdislist, rdlist) {
+		if (rdelem->rdinode == rdinode) {
+			list_del(&rdelem->rdlist);
+			spin_unlock(&cifs_sb->rtdislock);
+			return rdelem;
+		}
+	}
+	spin_unlock(&cifs_sb->rtdislock);
+	return NULL;
+}
+
+static struct cifs_rdelem *
+find_rdelem_by_dentry(const struct dentry *rdentry,
+			struct cifs_sb_info * cifs_sb)
+{
+	struct cifs_rdelem *rdelem;
+	spin_lock(&cifs_sb->rtdislock);
+	list_for_each_entry(rdelem, &cifs_sb->rtdislist, rdlist) {
+		if (rdelem->rdentry == rdentry) {
+			list_del(&rdelem->rdlist);
+			spin_unlock(&cifs_sb->rtdislock);
+			return rdelem;
+		}
+	}
+	spin_unlock(&cifs_sb->rtdislock);
+	return NULL;
+}
+
+static struct cifs_rdelem *
+find_rdelem_by_dentry_no_del(const struct dentry *rdentry,
+			struct cifs_sb_info * cifs_sb)
+{
+	struct cifs_rdelem *rdelem;
+	struct cifs_rdelem *found = NULL;
+	spin_lock(&cifs_sb->rtdislock);
+	list_for_each_entry(rdelem, &cifs_sb->rtdislist, rdlist) {
+		if (rdelem->rdentry == rdentry) {
+			found = rdelem;
+			break;
+		}
+	}
+	spin_unlock(&cifs_sb->rtdislock);
+	return found;
+}
+
+
+static void
+find_rdelem_by_path(char *full_path, struct inode **newInode,
+			struct cifs_sb_info * cifs_sb)
+{
+	struct cifs_rdelem *rdelem;
+	spin_lock(&cifs_sb->rtdislock);
+	list_for_each_entry(rdelem, &cifs_sb->rtdislist, rdlist) {
+		if (!strcmp(rdelem->rdname, full_path)) {
+			*newInode = ilookup(rdelem->rdinode->i_sb,
+					    rdelem->rdinode->i_ino);
+			break;
+		}
+	}
+	spin_unlock(&cifs_sb->rtdislock);
+}
+
 struct dentry *
 cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 	    unsigned int flags)
@@ -715,6 +820,9 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 	struct cifs_tcon *pTcon;
 	struct inode *newInode = NULL;
 	char *full_path = NULL;
+	struct dentry *ret = NULL;
+	struct cifs_rdelem *rdelem;
+	struct qstr dname;
 
 	xid = get_xid();
 
@@ -751,20 +859,66 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 	}
 	cifs_dbg(FYI, "Full path: %s inode = 0x%p\n",
 		 full_path, d_inode(direntry));
+	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
+		/*
+		 * Looking for an existing disconnected root dentry if any,
+		 * before sending out a lookup on the wire.
+		 */
+		find_rdelem_by_path(full_path, &newInode, cifs_sb);
+	}
 
-	if (pTcon->unix_ext) {
-		rc = cifs_get_inode_info_unix(&newInode, full_path,
-					      parent_dir_inode->i_sb, xid);
-	} else {
-		rc = cifs_get_inode_info(&newInode, full_path, NULL,
-				parent_dir_inode->i_sb, xid, NULL);
+	if (!newInode) {
+		if (pTcon->unix_ext) {
+			rc = cifs_get_inode_info_unix(&newInode, full_path,
+					parent_dir_inode->i_sb, xid);
+		} else
+			rc = cifs_get_inode_info(&newInode, full_path, NULL,
+					parent_dir_inode->i_sb, xid, NULL);
 	}
+	/* else, found an anonymous root dentry with an inode */
+
+
 
 	if ((rc == 0) && (newInode != NULL)) {
-		d_add(direntry, newInode);
+
 		/* since paths are not looked up by component - the parent
 		   directories are presumed to be good here */
-		renew_parental_timestamps(direntry);
+		if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
+			dname.name = direntry->d_name.name;
+			dname.len = strlen(direntry->d_name.name) + 1;
+			/*
+			 * Perhaps another lookup beat us to this.
+			 */
+			spin_lock(&cifs_sb->rtdislock);
+			ret = d_lookup(direntry->d_parent, &dname);
+			if (ret && !IS_ERR(ret)) {
+				dput(ret);
+				spin_unlock(&cifs_sb->rtdislock);
+				goto lookup_out;
+			} else
+				ret = d_splice_alias(newInode, direntry);
+			spin_unlock(&cifs_sb->rtdislock);
+		} else
+			ret = d_splice_alias(newInode, direntry);
+		if (!ret)
+			renew_parental_timestamps(direntry);
+
+		else {
+			if (!IS_ERR(ret)) {
+				if (!(cifs_sb->mnt_cifs_flags &
+						CIFS_MOUNT_SERVER_INUM)) {
+					rdelem =
+					find_rdelem_by_inode(newInode, cifs_sb);
+					if (rdelem)
+						cifs_free_rdelem(rdelem);
+				}
+				renew_parental_timestamps(ret);
+				dput(ret);
+				goto lookup_out;
+			} else
+				rc = PTR_ERR(ret);
+		}
+
 
 	} else if (rc == -ENOENT) {
 		rc = 0;
@@ -777,12 +931,41 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 		/* We special case check for Access Denied - since that
 		is a common return code */
 	}
-
+	ret = ERR_PTR(rc);
 lookup_out:
 	kfree(full_path);
 	cifs_put_tlink(tlink);
 	free_xid(xid);
-	return ERR_PTR(rc);
+	return ret;
+}
+
+static void
+cifs_d_common_releasedelete(const struct dentry *dentry)
+{
+	struct cifs_rdelem *rdelem;
+	struct cifs_sb_info *cifs_sb;
+
+	cifs_sb = CIFS_SB(dentry->d_sb);
+
+	/* disconnected root dentries that did not get spliced */
+	if (IS_ROOT(dentry) && dentry->d_flags & DCACHE_DISCONNECTED) {
+		rdelem = find_rdelem_by_dentry(dentry, cifs_sb);
+		if (rdelem)
+			cifs_free_rdelem(rdelem);
+	}
+}
+
+static int
+cifs_d_delete(const struct dentry *dentry)
+{
+	cifs_d_common_releasedelete(dentry);
+	return 0;
+}
+
+static void
+cifs_d_release(struct dentry *dentry)
+{
+	cifs_d_common_releasedelete(dentry);
 }
 
 static int
@@ -834,19 +1017,11 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
 	return 1;
 }
 
-/* static int cifs_d_delete(struct dentry *direntry)
-{
-	int rc = 0;
-
-	cifs_dbg(FYI, "In cifs d_delete, name = %pd\n", direntry);
-
-	return rc;
-}     */
-
 const struct dentry_operations cifs_dentry_ops = {
 	.d_revalidate = cifs_d_revalidate,
 	.d_automount = cifs_dfs_d_automount,
-/* d_delete:       cifs_d_delete,      */ /* not needed except for debugging */
+	.d_delete = cifs_d_delete,
+	.d_release = cifs_d_release,
 };
 
 static int cifs_ci_hash(const struct dentry *dentry, struct qstr *q)
@@ -921,4 +1096,6 @@ const struct dentry_operations cifs_ci_dentry_ops = {
 	.d_hash = cifs_ci_hash,
 	.d_compare = cifs_ci_compare,
 	.d_automount = cifs_dfs_d_automount,
+	.d_delete = cifs_d_delete,
+	.d_release = cifs_d_release,
 };
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index f621b44..7d17911 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -923,8 +923,10 @@ inode_has_hashed_dentries(struct inode *inode)
 	spin_lock(&inode->i_lock);
 	hlist_for_each_entry(dentry, &inode->i_dentry, d_u.d_alias) {
 		if (!d_unhashed(dentry) || IS_ROOT(dentry)) {
-			spin_unlock(&inode->i_lock);
-			return true;
+			if (!(dentry->d_flags & DCACHE_DISCONNECTED)) {
+				spin_unlock(&inode->i_lock);
+				return true;
+			}
 		}
 	}
 	spin_unlock(&inode->i_lock);
@@ -974,6 +976,23 @@ retry_iget5_locked:
 	return inode;
 }
 
+void
+fill_phfattr(struct cifs_fattr *cf, struct super_block *sb)
+{
+
+	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+
+	memset(cf, 0, sizeof(*cf));
+	cf->cf_uniqueid = ROOT_I;
+	cf->cf_nlink = 1;
+	cf->cf_atime = CURRENT_TIME;
+	cf->cf_ctime = CURRENT_TIME;
+	cf->cf_mtime = CURRENT_TIME;
+	cf->cf_mode = S_IFDIR | S_IXUGO | S_IRWXU;
+	cf->cf_uid = cifs_sb->mnt_uid;
+	cf->cf_gid = cifs_sb->mnt_gid;
+}
+
 /* gets root inode */
 struct inode *cifs_root_iget(struct super_block *sb)
 {
@@ -982,6 +1001,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
 	struct inode *inode = NULL;
 	long rc;
 	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
+	struct cifs_fattr phfattr;
 
 	xid = get_xid();
 	if (tcon->unix_ext) {
@@ -997,8 +1017,16 @@ struct inode *cifs_root_iget(struct super_block *sb)
 
 iget_no_retry:
 	if (!inode) {
-		inode = ERR_PTR(rc);
-		goto out;
+		if (rc == -EACCES) {
+			fill_phfattr(&phfattr, sb);
+			inode = cifs_iget(sb, &phfattr);
+			if (inode)
+				rc = 0;
+		}
+		if (rc) {
+			inode = ERR_PTR(rc);
+			goto out;
+		}
 	}
 
 #ifdef CONFIG_CIFS_FSCACHE
-- 
2.1.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]