[RFC,11/11] gfs2: Invalide security labels of inodes that go invalid

Commit Message

Andreas Grünbacher Aug. 20, 2015, 6:19 p.m. UTC

Invalide security labels of inodes when they go invalid.  SELinux will reload
the "security.selinux" xattr via iop->igetxattr on the next access.

Signed-off-by: Andreas Gruenbacher <[e-mail]>
---
 fs/gfs2/glops.c |  2 ++
 fs/gfs2/inode.c | 16 ++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Aug. 21, 2015, 6:49 a.m. UTC | #1

On Thu, Aug 20, 2015 at 08:19:58PM +0200, Andreas Gruenbacher wrote:
> Invalide security labels of inodes when they go invalid.  SELinux will reload
> the "security.selinux" xattr via iop->igetxattr on the next access.

And if that's all that you needed just skip the detour trough the VFS
interfaces.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andreas Gruenbacher Aug. 21, 2015, 9:25 a.m. UTC | #2

Christoph,

thanks for your review.

2015-08-21 8:49 GMT+02:00 Christoph Hellwig <[e-mail]>:
> On Thu, Aug 20, 2015 at 08:19:58PM +0200, Andreas Gruenbacher wrote:
>> Invalide security labels of inodes when they go invalid.  SELinux will reload
>> the "security.selinux" xattr via iop->igetxattr on the next access.
>
> And if that's all that you needed just skip the detour trough the VFS
> interfaces.

We have more than one file system that has a similar consistency model
as gfs2 so it's not really a one-off hack, and we have selinux as well
as smack, with several different xattr names.

Assuming that selinux or smack only have an inode available, how would
they (re)load an inode's security label other than going through the
vfs?

I could imagine bypassing the xattr handler abstraction in the
affected file systems, but that doesn't seem much better either.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index fa3fa5e..29be6d3 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -13,6 +13,7 @@ 
 #include <linux/gfs2_ondisk.h>
 #include <linux/bio.h>
 #include <linux/posix_acl.h>
+#include <linux/security.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -260,6 +261,7 @@  static void inode_go_inval(struct gfs2_glock *gl, int flags)
 		if (ip) {
 			set_bit(GIF_INVALID, &ip->i_flags);
 			forget_all_cached_acls(&ip->i_inode);
+			security_inode_invalidate_secctx(&ip->i_inode);
 			gfs2_dir_hash_inval(ip);
 		}
 	}
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 063fdfc..3474976 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2011,28 +2011,33 @@  static int gfs2_setxattr(struct dentry *dentry, const char *name,
 	return ret;
 }
 
-static ssize_t gfs2_getxattr(struct dentry *dentry, const char *name,
+static ssize_t gfs2_igetxattr(struct inode *inode, const char *name,
 			     void *data, size_t size)
 {
-	struct inode *inode = d_inode(dentry);
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 	int ret;
 
 	/* For selinux during lookup */
 	if (gfs2_glock_is_locked_by_me(ip->i_gl))
-		return generic_getxattr(dentry, name, data, size);
+		return generic_igetxattr(inode, name, data, size);
 
 	gfs2_holder_init(ip->i_gl, LM_ST_SHARED, LM_FLAG_ANY, &gh);
 	ret = gfs2_glock_nq(&gh);
 	if (ret == 0) {
-		ret = generic_getxattr(dentry, name, data, size);
+		ret = generic_igetxattr(inode, name, data, size);
 		gfs2_glock_dq(&gh);
 	}
 	gfs2_holder_uninit(&gh);
 	return ret;
 }
 
+static ssize_t gfs2_getxattr(struct dentry *dentry, const char *name,
+			     void *data, size_t size)
+{
+	return gfs2_igetxattr(d_inode(dentry), name, data, size);
+}
+
 static int gfs2_removexattr(struct dentry *dentry, const char *name)
 {
 	struct inode *inode = d_inode(dentry);
@@ -2099,6 +2104,7 @@  const struct inode_operations gfs2_file_iops = {
 	.setattr = gfs2_setattr,
 	.getattr = gfs2_getattr,
 	.setxattr = gfs2_setxattr,
+	.igetxattr = gfs2_igetxattr,
 	.getxattr = gfs2_getxattr,
 	.listxattr = gfs2_listxattr,
 	.removexattr = gfs2_removexattr,
@@ -2121,6 +2127,7 @@  const struct inode_operations gfs2_dir_iops = {
 	.setattr = gfs2_setattr,
 	.getattr = gfs2_getattr,
 	.setxattr = gfs2_setxattr,
+	.igetxattr = gfs2_igetxattr,
 	.getxattr = gfs2_getxattr,
 	.listxattr = gfs2_listxattr,
 	.removexattr = gfs2_removexattr,
@@ -2138,6 +2145,7 @@  const struct inode_operations gfs2_symlink_iops = {
 	.setattr = gfs2_setattr,
 	.getattr = gfs2_getattr,
 	.setxattr = gfs2_setxattr,
+	.igetxattr = gfs2_igetxattr,
 	.getxattr = gfs2_getxattr,
 	.listxattr = gfs2_listxattr,
 	.removexattr = gfs2_removexattr,