From a1275c3b21e433888994f7b979cd1129d384ec9c Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Thu, 27 May 2010 11:19:30 -0400 Subject: ecryptfs: Fix warning in ecryptfs_process_response() Fix warning seen with "make -j24 CONFIG_DEBUG_SECTION_MISMATCH=y V=1": fs/ecryptfs/messaging.c: In function 'ecryptfs_process_response': fs/ecryptfs/messaging.c:276: warning: 'daemon' may be used uninitialized in this function Signed-off-by: Prarit Bhargava Signed-off-by: Tyler Hicks --- fs/ecryptfs/messaging.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ecryptfs/messaging.c b/fs/ecryptfs/messaging.c index 46c4dd8dfcc3..bcb68c0cb1f0 100644 --- a/fs/ecryptfs/messaging.c +++ b/fs/ecryptfs/messaging.c @@ -274,7 +274,7 @@ int ecryptfs_process_response(struct ecryptfs_message *msg, uid_t euid, struct user_namespace *user_ns, struct pid *pid, u32 seq) { - struct ecryptfs_daemon *daemon; + struct ecryptfs_daemon *uninitialized_var(daemon); struct ecryptfs_msg_ctx *msg_ctx; size_t msg_size; struct nsproxy *nsproxy; -- cgit v1.2.3 From c43f7b8fb03be8bcc579bfc4e6ab70eac887ab55 Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Tue, 3 Nov 2009 11:45:11 -0600 Subject: eCryptfs: Handle ioctl calls with unlocked and compat functions Lower filesystems that only implemented unlocked_ioctl weren't being passed ioctl calls because eCryptfs only checked for lower_file->f_op->ioctl and returned -ENOTTY if it was NULL. eCryptfs shouldn't implement ioctl(), since it doesn't require the BKL. This patch introduces ecryptfs_unlocked_ioctl() and ecryptfs_compat_ioctl(), which passes the calls on to the lower file system. https://bugs.launchpad.net/ecryptfs/+bug/469664 Reported-by: James Dupin Cc: stable@kernel.org Signed-off-by: Tyler Hicks --- fs/ecryptfs/file.c | 56 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index e8fcf4e2ed7d..6e4f84cf73e8 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -292,12 +292,40 @@ static int ecryptfs_fasync(int fd, struct file *file, int flag) return rc; } -static int ecryptfs_ioctl(struct inode *inode, struct file *file, - unsigned int cmd, unsigned long arg); +static long +ecryptfs_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct file *lower_file = NULL; + long rc = -ENOTTY; + + if (ecryptfs_file_to_private(file)) + lower_file = ecryptfs_file_to_lower(file); + if (lower_file && lower_file->f_op && lower_file->f_op->unlocked_ioctl) + rc = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg); + return rc; +} + +#ifdef CONFIG_COMPAT +static long +ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct file *lower_file = NULL; + long rc = -ENOIOCTLCMD; + + if (ecryptfs_file_to_private(file)) + lower_file = ecryptfs_file_to_lower(file); + if (lower_file && lower_file->f_op && lower_file->f_op->compat_ioctl) + rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg); + return rc; +} +#endif const struct file_operations ecryptfs_dir_fops = { .readdir = ecryptfs_readdir, - .ioctl = ecryptfs_ioctl, + .unlocked_ioctl = ecryptfs_unlocked_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = ecryptfs_compat_ioctl, +#endif .open = ecryptfs_open, .flush = ecryptfs_flush, .release = ecryptfs_release, @@ -313,7 +341,10 @@ const struct file_operations ecryptfs_main_fops = { .write = do_sync_write, .aio_write = generic_file_aio_write, .readdir = ecryptfs_readdir, - .ioctl = ecryptfs_ioctl, + .unlocked_ioctl = ecryptfs_unlocked_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = ecryptfs_compat_ioctl, +#endif .mmap = generic_file_mmap, .open = ecryptfs_open, .flush = ecryptfs_flush, @@ -322,20 +353,3 @@ const struct file_operations ecryptfs_main_fops = { .fasync = ecryptfs_fasync, .splice_read = generic_file_splice_read, }; - -static int -ecryptfs_ioctl(struct inode *inode, struct file *file, unsigned int cmd, - unsigned long arg) -{ - int rc = 0; - struct file *lower_file = NULL; - - if (ecryptfs_file_to_private(file)) - lower_file = ecryptfs_file_to_lower(file); - if (lower_file && lower_file->f_op && lower_file->f_op->ioctl) - rc = lower_file->f_op->ioctl(ecryptfs_inode_to_lower(inode), - lower_file, cmd, arg); - else - rc = -ENOTTY; - return rc; -} -- cgit v1.2.3 From 31f73bee3e170b7cabb35db9e2f4bf7919b9d036 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Thu, 29 Jul 2010 13:01:36 +0200 Subject: ecryptfs: release reference to lower mount if interpose fails In ecryptfs_lookup_and_interpose_lower() the lower mount is not decremented if allocation of a dentry info struct failed. As a result the lower filesystem cant be unmounted any more (since it is considered busy). This patch corrects the reference counting. Signed-off-by: Lino Sanfilippo Cc: stable@kernel.org Signed-off-by: Tyler Hicks --- fs/ecryptfs/inode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 31ef5252f0fe..8cd617b66baa 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -264,7 +264,7 @@ int ecryptfs_lookup_and_interpose_lower(struct dentry *ecryptfs_dentry, printk(KERN_ERR "%s: Out of memory whilst attempting " "to allocate ecryptfs_dentry_info struct\n", __func__); - goto out_dput; + goto out_put; } ecryptfs_set_dentry_lower(ecryptfs_dentry, lower_dentry); ecryptfs_set_dentry_lower_mnt(ecryptfs_dentry, lower_mnt); @@ -339,8 +339,9 @@ int ecryptfs_lookup_and_interpose_lower(struct dentry *ecryptfs_dentry, out_free_kmem: kmem_cache_free(ecryptfs_header_cache_2, page_virt); goto out; -out_dput: +out_put: dput(lower_dentry); + mntput(lower_mnt); d_drop(ecryptfs_dentry); out: return rc; -- cgit v1.2.3 From ceeab92971e8af05c1e81a4ff2c271124b55bb9b Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Fri, 6 Aug 2010 22:58:49 +0200 Subject: fs/ecryptfs/file.c: introduce missing free The comments in the code indicate that file_info should be released if the function fails. This releasing is done at the label out_free, not out. The semantic match that finds this problem is as follows: (http://www.emn.fr/x-info/coccinelle/) // @r exists@ local idexpression x; statement S; expression E; identifier f,f1,l; position p1,p2; expression *ptr != NULL; @@ x@p1 = kmem_cache_zalloc(...); ... if (x == NULL) S <... when != x when != if (...) { <+...x...+> } ( x->f1 = E | (x->f1 == NULL || ...) | f(...,x->f1,...) ) ...> ( return <+...x...+>; | return@p2 ...; ) @script:python@ p1 << r.p1; p2 << r.p2; @@ print "* file: %s kmem_cache_zalloc %s" % (p1[0].file,p1[0].line) // Signed-off-by: Julia Lawall Cc: stable@kernel.org Signed-off-by: Tyler Hicks --- fs/ecryptfs/file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index 6e4f84cf73e8..622c95140802 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -199,7 +199,7 @@ static int ecryptfs_open(struct inode *inode, struct file *file) "the persistent file for the dentry with name " "[%s]; rc = [%d]\n", __func__, ecryptfs_dentry->d_name.name, rc); - goto out; + goto out_free; } } if ((ecryptfs_inode_to_private(inode)->lower_file->f_flags & O_RDONLY) @@ -207,7 +207,7 @@ static int ecryptfs_open(struct inode *inode, struct file *file) rc = -EPERM; printk(KERN_WARNING "%s: Lower persistent file is RO; eCryptfs " "file must hence be opened RO\n", __func__); - goto out; + goto out_free; } ecryptfs_set_file_lower( file, ecryptfs_inode_to_private(inode)->lower_file); -- cgit v1.2.3 From 21edad32205e97dc7ccb81a85234c77e760364c8 Mon Sep 17 00:00:00 2001 From: Lino Sanfilippo Date: Thu, 15 Jul 2010 15:11:55 +0200 Subject: ecryptfs: dont call lookup_one_len to avoid NULL nameidata I have encountered the same problem that Eric Sandeen described in this post http://lkml.org/lkml/fancy/2010/4/23/467 while experimenting with stackable filesystems. The reason seems to be that ecryptfs calls lookup_one_len() to get the lower dentry, which in turn calls the lower parent dirs d_revalidate() with a NULL nameidata object. If ecryptfs is the underlaying filesystem, the NULL pointer dereference occurs, since ecryptfs is not prepared to handle a NULL nameidata. I know that this cant happen any more, since it is no longer allowed to mount ecryptfs upon itself. But maybe this patch it useful nevertheless, since the problem would still apply for an underlaying filesystem that implements d_revalidate() and is not prepared to handle a NULL nameidata (I dont know if there actually is such a fs). With this patch (against 2.6.35-rc5) ecryptfs uses the vfs_lookup_path() function instead of lookup_one_len() which ensures that the nameidata passed to the lower filesystems d_revalidate(). Signed-off-by: Lino Sanfilippo Signed-off-by: Tyler Hicks --- fs/ecryptfs/inode.c | 89 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 12 deletions(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 8cd617b66baa..a8acfc5d3476 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -347,6 +347,76 @@ out: return rc; } +/** + * ecryptfs_new_lower_dentry + * @ename: The name of the new dentry. + * @lower_dir_dentry: Parent directory of the new dentry. + * @nd: nameidata from last lookup. + * + * Create a new dentry or get it from lower parent dir. + */ +static struct dentry * +ecryptfs_new_lower_dentry(struct qstr *name, struct dentry *lower_dir_dentry, + struct nameidata *nd) +{ + struct dentry *new_dentry; + struct dentry *tmp; + struct inode *lower_dir_inode; + + lower_dir_inode = lower_dir_dentry->d_inode; + + tmp = d_alloc(lower_dir_dentry, name); + if (!tmp) + return ERR_PTR(-ENOMEM); + + mutex_lock(&lower_dir_inode->i_mutex); + new_dentry = lower_dir_inode->i_op->lookup(lower_dir_inode, tmp, nd); + mutex_unlock(&lower_dir_inode->i_mutex); + + if (!new_dentry) + new_dentry = tmp; + else + dput(tmp); + + return new_dentry; +} + + +/** + * ecryptfs_lookup_one_lower + * @ecryptfs_dentry: The eCryptfs dentry that we are looking up + * @lower_dir_dentry: lower parent directory + * + * Get the lower dentry from vfs. If lower dentry does not exist yet, + * create it. + */ +static struct dentry * +ecryptfs_lookup_one_lower(struct dentry *ecryptfs_dentry, + struct dentry *lower_dir_dentry) +{ + struct nameidata nd; + struct vfsmount *lower_mnt; + struct qstr *name; + int err; + + name = &ecryptfs_dentry->d_name; + lower_mnt = mntget(ecryptfs_dentry_to_lower_mnt( + ecryptfs_dentry->d_parent)); + err = vfs_path_lookup(lower_dir_dentry, lower_mnt, name->name , 0, &nd); + mntput(lower_mnt); + + if (!err) { + /* we dont need the mount */ + mntput(nd.path.mnt); + return nd.path.dentry; + } + if (err != -ENOENT) + return ERR_PTR(err); + + /* create a new lower dentry */ + return ecryptfs_new_lower_dentry(name, lower_dir_dentry, &nd); +} + /** * ecryptfs_lookup * @ecryptfs_dir_inode: The eCryptfs directory inode @@ -374,14 +444,12 @@ static struct dentry *ecryptfs_lookup(struct inode *ecryptfs_dir_inode, goto out_d_drop; } lower_dir_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry->d_parent); - mutex_lock(&lower_dir_dentry->d_inode->i_mutex); - lower_dentry = lookup_one_len(ecryptfs_dentry->d_name.name, - lower_dir_dentry, - ecryptfs_dentry->d_name.len); - mutex_unlock(&lower_dir_dentry->d_inode->i_mutex); + + lower_dentry = ecryptfs_lookup_one_lower(ecryptfs_dentry, + lower_dir_dentry); if (IS_ERR(lower_dentry)) { rc = PTR_ERR(lower_dentry); - ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_len() returned " + ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_lower() returned " "[%d] on lower_dentry = [%s]\n", __func__, rc, encrypted_and_encoded_name); goto out_d_drop; @@ -403,14 +471,11 @@ static struct dentry *ecryptfs_lookup(struct inode *ecryptfs_dir_inode, "filename; rc = [%d]\n", __func__, rc); goto out_d_drop; } - mutex_lock(&lower_dir_dentry->d_inode->i_mutex); - lower_dentry = lookup_one_len(encrypted_and_encoded_name, - lower_dir_dentry, - encrypted_and_encoded_name_size - 1); - mutex_unlock(&lower_dir_dentry->d_inode->i_mutex); + lower_dentry = ecryptfs_lookup_one_lower(ecryptfs_dentry, + lower_dir_dentry); if (IS_ERR(lower_dentry)) { rc = PTR_ERR(lower_dentry); - ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_len() returned " + ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_lower() returned " "[%d] on lower_dentry = [%s]\n", __func__, rc, encrypted_and_encoded_name); goto out_d_drop; -- cgit v1.2.3