Re: [PATCH] overlayfs: copyup security inode field

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux