From patchwork Thu Nov 5 14:48:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Aur=C3=A9lien_Aptel?= X-Patchwork-Id: 7561961 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C71AEBEEA4 for ; Thu, 5 Nov 2015 14:49:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1A4572073F for ; Thu, 5 Nov 2015 14:49:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 230A520457 for ; Thu, 5 Nov 2015 14:49:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1162153AbbKEOtH (ORCPT ); Thu, 5 Nov 2015 09:49:07 -0500 Received: from smtp.nue.novell.com ([195.135.221.5]:50736 "EHLO smtp.nue.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1162162AbbKEOtB (ORCPT ); Thu, 5 Nov 2015 09:49:01 -0500 Received: from gwmail.emea.novell.com ([10.120.13.103]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Thu, 05 Nov 2015 15:48:59 +0100 Received: from aaptelpc ([10.120.13.201]) by gwmail.emea.novell.com with ESMTP (TLS encrypted); Thu, 05 Nov 2015 14:48:41 +0000 Date: Thu, 5 Nov 2015 15:48:36 +0100 From: =?UTF-8?B?QXVyw6lsaWVu?= Aptel To: linux-cifs@vger.kernel.org, Shirish Pargaonkar Subject: [PATCH v1] cifs: make shares unaccessible at root level mountable Message-ID: <20151105154836.1851c30c@aaptelpc> Organization: SUSE X-Mailer: Claws Mail 3.11.0 (GTK+ 2.24.28; x86_64-suse-linux-gnu) MIME-Version: 1.0 Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 From 92520bbeef256957c150df01f946ad8890140a78 Mon Sep 17 00:00:00 2001 From: Aurelien Aptel 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 --- 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