[PATCH V2 1/1] ecryptfs: call vfs_setxattr() in ecryptfs_setxattr()

[PATCH V2 1/1] ecryptfs: call vfs_setxattr() in ecryptfs_setxattr()

[Roberto Sassu]

[-- Attachment #1: Type: Text/Plain, Size: 1906 bytes --]

Ecryptfs is a stackable filesystem which relies on lower filesystems the
ability of setting/getting extended attributes.

If there is a security module enabled on the system it updates the
'security' field of inodes according to the owned extended attribute set
with the function vfs_setxattr().  When this function is performed on a
ecryptfs filesystem the 'security' field is not updated for the lower
filesystem since the call security_inode_post_setxattr() is missing for
the lower inode.
Further, the call security_inode_setxattr() is missing for the lower inode,
leading to policy violations in the security module because specific
checks for this hook are not performed (i. e. filesystem 
'associate' permission on SELinux is not checked for the lower filesystem).

This patch replaces the call of the setxattr() method of the lower inode
in the function ecryptfs_setxattr() with vfs_setxattr().


Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Reviewed-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
---
 fs/ecryptfs/inode.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 8cd617b..9c0cc4b 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -32,6 +32,7 @@
 #include <linux/crypto.h>
 #include <linux/fs_stack.h>
 #include <linux/slab.h>
+#include <linux/xattr.h>
 #include <asm/unaligned.h>
 #include "ecryptfs_kernel.h"
 
@@ -1016,10 +1017,8 @@ ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 		rc = -EOPNOTSUPP;
 		goto out;
 	}
-	mutex_lock(&lower_dentry->d_inode->i_mutex);
-	rc = lower_dentry->d_inode->i_op->setxattr(lower_dentry, name, value,
-						   size, flags);
-	mutex_unlock(&lower_dentry->d_inode->i_mutex);
+
+	rc = vfs_setxattr(lower_dentry, name, value, size, flags);
 out:
 	return rc;
 }
-- 
1.7.2.3

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4707 bytes --]

Re: [PATCH V2 1/1] ecryptfs: call vfs_setxattr() in ecryptfs_setxattr()

[James Morris]

On Tue, 5 Oct 2010, Roberto Sassu wrote:

> Ecryptfs is a stackable filesystem which relies on lower filesystems the
> ability of setting/getting extended attributes.
> 
> If there is a security module enabled on the system it updates the
> 'security' field of inodes according to the owned extended attribute set
> with the function vfs_setxattr().  When this function is performed on a
> ecryptfs filesystem the 'security' field is not updated for the lower
> filesystem since the call security_inode_post_setxattr() is missing for
> the lower inode.
> Further, the call security_inode_setxattr() is missing for the lower inode,
> leading to policy violations in the security module because specific
> checks for this hook are not performed (i. e. filesystem 
> 'associate' permission on SELinux is not checked for the lower filesystem).
> 
> This patch replaces the call of the setxattr() method of the lower inode
> in the function ecryptfs_setxattr() with vfs_setxattr().
> 
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> Reviewed-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>

Acked-by: James Morris <jmorris@namei.org>

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH V2 1/1] ecryptfs: call vfs_setxattr() in ecryptfs_setxattr()

[Tyler Hicks]

On Tue Oct 05, 2010 at 06:53:45PM +0200, Roberto Sassu <roberto.sassu@polito.it> wrote:
> Ecryptfs is a stackable filesystem which relies on lower filesystems the
> ability of setting/getting extended attributes.
> 
> If there is a security module enabled on the system it updates the
> 'security' field of inodes according to the owned extended attribute set
> with the function vfs_setxattr().  When this function is performed on a
> ecryptfs filesystem the 'security' field is not updated for the lower
> filesystem since the call security_inode_post_setxattr() is missing for
> the lower inode.
> Further, the call security_inode_setxattr() is missing for the lower inode,
> leading to policy violations in the security module because specific
> checks for this hook are not performed (i. e. filesystem 
> 'associate' permission on SELinux is not checked for the lower filesystem).
> 
> This patch replaces the call of the setxattr() method of the lower inode
> in the function ecryptfs_setxattr() with vfs_setxattr().
> 
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
> Reviewed-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
> ---

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6.git#next

Thanks!

>  fs/ecryptfs/inode.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 8cd617b..9c0cc4b 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -32,6 +32,7 @@
>  #include <linux/crypto.h>
>  #include <linux/fs_stack.h>
>  #include <linux/slab.h>
> +#include <linux/xattr.h>
>  #include <asm/unaligned.h>
>  #include "ecryptfs_kernel.h"
>  
> @@ -1016,10 +1017,8 @@ ecryptfs_setxattr(struct dentry *dentry, const char *name, const void *value,
>  		rc = -EOPNOTSUPP;
>  		goto out;
>  	}
> -	mutex_lock(&lower_dentry->d_inode->i_mutex);
> -	rc = lower_dentry->d_inode->i_op->setxattr(lower_dentry, name, value,
> -						   size, flags);
> -	mutex_unlock(&lower_dentry->d_inode->i_mutex);
> +
> +	rc = vfs_setxattr(lower_dentry, name, value, size, flags);
>  out:
>  	return rc;
>  }
> -- 
> 1.7.2.3