cifs: make shares unaccessible at root level mountable

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)

From 92520bbeef256957c150df01f946ad8890140a78 Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel-IBi9RG/[email protected]>
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

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

index 3182273..b2b65f6 100644
@@ -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 */

index c63fd1d..b0a813f 100644
@@ -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);

index 773f4dc..018c156 100644
@@ -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.

index f621b44..7d17911 100644
@@ -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

I do not understand why you need to entertain error ENOENT on an intermediate path.
If a server is returning that error, there does not exist such an entry. In such a case, does Query Path Info on the full path return any error?

I still do not understand why noserverino is in picture. Only the disconnected root dentry
has an inode generated with an id by the cifs client. Once this disconnected root denty
gets spliced, the inode is filled with correct inode (fileid) available from the server
(since that intermediate path is now accessible hence splicable). You have fileid/inode available for all eligible/accessible file objects under the disconnected
root entry.

In cifs_get_root() @ fs/cifs/cifsfs.c when one of the intermediary path component access is restricted I get a NULL direntry->d_inode. Which is translated to ENOENT.

		struct inode *dir = d_inode(dentry);
		struct dentry *child;

		if (!dir) {
			dput(dentry);
			dentry = ERR_PTR(-ENOENT);
			break;
		}

Again, the only function that adds disconneted roots
to the linked list in the superblock is cifs_alloc_rdelem() which is *only called* with noserverino. This is from your patch.

% grep -R cifs_alloc_rdelem fs/cifs/*.c
fs/cifs/cifsfs.c-}
fs/cifs/cifsfs.c-
fs/cifs/cifsfs.c-struct cifs_rdelem * fs/cifs/cifsfs.c:cifs_alloc_rdelem(char *full_path, struct dentry *rdentry,
fs/cifs/cifsfs.c- struct inode *rdinode)
fs/cifs/cifsfs.c-{
fs/cifs/cifsfs.c- struct cifs_rdelem *rdelem;
--
fs/cifs/cifsfs.c- if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
fs/cifs/cifsfs.c-
fs/cifs/cifsfs.c- rdelem =
fs/cifs/cifsfs.c: cifs_alloc_rdelem(fpath, dentry, rinode);
fs/cifs/cifsfs.c- if (IS_ERR(rdelem))
fs/cifs/cifsfs.c- goto rdelem_ret;
fs/cifs/cifsfs.c- spin_lock(&cifs_sb->rtdislock);

To properly build a path from a child dentry of a disconnected root you need to know the path of the disconnected root and use it as a prefix. This prefix is only stored in the rdelem struct which is only used with noserverino. See above.

My patch was able to fix several SUSE customers problems and people contacted me off-list to tell me it also fixed his problem. I cannot reproduce the issue you're describing.

We can talk on irc if you want, I'm aaptel on freenode.

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)

So only time you will have NULL dir (inode) is when lookup_one_len() in cifs_get_root() returned a negative dentry. If we call create_root_dis_dentry()
for a path object which already has a negative dentry, we will create another dentry (disconnected) for the same path object. So for the same path object, we could possibly have two (or more) dentries in the dentry cache, one negative and one
(or more) disconnected. So I think if we are going to call create_root_dis_dentry()
in case error is ENONET, we should delete the (negative) dentry before the call.