On Tuesday, October 05, 2010 08:23:53 am Tyler Hicks wrote: > On Fri Oct 01, 2010 at 02:14:00PM -0700, akpm@xxxxxxxxxxxxxxxxxxxx <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > From: Roberto Sassu <roberto.sassu@xxxxxxxxx> > > Andrew - thanks for not letting this one slip through. > > > > > 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. > > > > This patch makes the function __vfs_setxattr_noperm() available for > > modules and replaces the call to the setxattr() method of the lower inode > > with the exported function. > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxx> > > Cc: Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx> > > Cc: Dustin Kirkland <kirkland@xxxxxxxxxxxxx> > > Cc: James Morris <jmorris@xxxxxxxxx> > > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > --- > > > > fs/ecryptfs/inode.c | 5 +++-- > > fs/xattr.c | 1 + > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff -puN fs/ecryptfs/inode.c~ecryptfs-call-__vfs_setxattr_noperm-in-ecryptfs_setxattr fs/ecryptfs/inode.c > > --- a/fs/ecryptfs/inode.c~ecryptfs-call-__vfs_setxattr_noperm-in-ecryptfs_setxattr > > +++ a/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" > > > > @@ -1109,8 +1110,8 @@ ecryptfs_setxattr(struct dentry *dentry, > > goto out; > > } > > mutex_lock(&lower_dentry->d_inode->i_mutex); > > - rc = lower_dentry->d_inode->i_op->setxattr(lower_dentry, name, value, > > - size, flags); > > + rc = __vfs_setxattr_noperm(lower_dentry, name, value, > > + size, flags); > > Hi Roberto - Thanks for the fix. However, it seems to me like we should > call vfs_setxattr(). We can't guarantee consistency among the eCryptfs > inode and the lower inode, so the extra call to xattr_permission() > isn't a waste. > Hi Tyler i look in deep at the code and i made the following considerations: first, i think calling vfs_setxattr() instead of __vfs_setxattr is not necessary when ecryptfs is mounted on the top of a directory in the lower filesystem: in this case the only way to modify the extended attributes of lower inodes is to call ecryptfs_setxattr(). Second, the use of vfs_setxattr() causes functions xattr_permission() and security_inode_setxattr() to be called twice. While the former is not needed the second time because it calls ecryptfs_permission() which in turn invokes inode_permission() on the lower inode, i noted the second is required at least for SELinux in this situation: suppose that ecryptfs and the lower filesystem have different security contexts for the filesystem object; in this case if the security_inode_setxattr() is missing for the lower inode, the filesystem 'associate' permission is not checked (SELinux verifies if an inode with a given label can be stored on a filesystem bound to a specific security context). I will post a second version of the patch which has the function __vfs_setxattr() replaced with vfs_setxattr(). > > mutex_unlock(&lower_dentry->d_inode->i_mutex); > > out: > > return rc; > > diff -puN fs/xattr.c~ecryptfs-call-__vfs_setxattr_noperm-in-ecryptfs_setxattr fs/xattr.c > > --- a/fs/xattr.c~ecryptfs-call-__vfs_setxattr_noperm-in-ecryptfs_setxattr > > +++ a/fs/xattr.c > > @@ -106,6 +106,7 @@ int __vfs_setxattr_noperm(struct dentry > > > > return error; > > } > > +EXPORT_SYMBOL_GPL(__vfs_setxattr_noperm); > > > > > > int > > _ > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
