J. R. Okajima wrote: > > When two inodes refers to a single inode_security_struct (i_security), > won't it cause a "double free" problem? > Thanks for your feedback. Below another patch that set the inode_security_struct correctly with help of security API. Your review are welcome. Best Regards, Zakaria ElQotbi >From 934602df4e9d26e77cf04f8514079c02fe592d4a Mon Sep 17 00:00:00 2001 From: Zakaria ElQotbi <zakaria.elqotbi@xxxxxxxxxxx> Date: Wed, 19 Mar 2014 11:59:09 +0100 Subject: [PATCH] overlayfs: copyup security inode field SELinux (and maybe other security frameworks) relies on inode->i_security field to perform audit of security contexts. I think this field must be the same as the underlying filesystem, instead of creating new fresh one at ovl_new_inode() which give an UNLABELED sid. The issue rised when certain process (for instance Zygote) fails to perform some actions (for instance getxattr) on Android using SEAndroid and overlyafs with empty uppdir mounted on /system, but it succeeds in case there is not overlayfs. Signed-off-by: Zakaria ElQotbi <zakaria.elqotbi@xxxxxxxxxxx> --- fs/overlayfs/inode.c | 25 +++++++++++++++++++++++++ fs/overlayfs/overlayfs.h | 7 ++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index dbdf58e..a6f4813 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -11,6 +11,7 @@ #include <linux/slab.h> #include <linux/xattr.h> #include "overlayfs.h" +#include <linux/security.h> int ovl_setattr(struct dentry *dentry, struct iattr *attr) { @@ -332,6 +333,30 @@ static const struct inode_operations ovl_symlink_inode_operations = { .removexattr = ovl_removexattr, }; +void ovl_copyattr(struct inode *from, struct inode *to) +{ +#ifdef CONFIG_SECURITY + void *secctx; + size_t ctxlen; + int err = -1; + + err = security_inode_getsecctx(from, &secctx, &ctxlen); + if (!err) { + /* + * replace the fresh inode_security_struct because it should be + * the same as the real underlying inode. + */ + err = security_inode_notifysecctx(to, secctx, ctxlen); + security_release_secctx(secctx, ctxlen); + } + if (err) { + WARN(1, "cannot set security context err:%d \n", err); + } +#endif + to->i_uid = from->i_uid; + to->i_gid = from->i_gid; +} + struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, struct ovl_entry *oe) { diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 3495a55..0a1bac3 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -56,11 +56,8 @@ int ovl_removexattr(struct dentry *dentry, const char *name); struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, struct ovl_entry *oe); -static inline void ovl_copyattr(struct inode *from, struct inode *to) -{ - to->i_uid = from->i_uid; - to->i_gid = from->i_gid; -} + +void ovl_copyattr(struct inode *from, struct inode *to); /* dir.c */ extern const struct inode_operations ovl_dir_inode_operations; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html