linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] Enable atomic inode security labeling
@ 2005-07-08 13:25 Stephen Smalley
  2005-07-08 13:48 ` [RFC][PATCH 1/3] security: " Stephen Smalley
                   ` (5 more replies)
  0 siblings, 6 replies; 25+ messages in thread
From: Stephen Smalley @ 2005-07-08 13:25 UTC (permalink / raw)
  To: linuxfs
  Cc: Alexander Viro, Ext2-devel, Andreas Gruenbacher, Andreas Dilger,
	Andrew Morton, Stephen Tweedie, James Morris, Chris Wright

The following patch set enables atomic security labeling of newly
created inodes by altering the fs code to invoke a new LSM hook to
obtain the security attribute to apply to a newly created inode and to
set up the incore inode security state during the inode creation
transaction.  This parallels the existing processing for setting ACLs
on newly created inodes.  Otherwise, it is possible for new inodes to
be accessed by another thread via the dcache prior to complete
security setup (presently handled by the post_create/mkdir/... LSM
hooks in the VFS) and a newly created inode may be left unlabeled on
the disk in the event of a crash.  SELinux presently works around the
issue by ensuring that the incore inode security label is initialized
to a special SID that is inaccessible to unprivileged processes (in
accordance with policy), thereby preventing inappropriate access but
potentially causing false denials on legitimate accesses.  A simple test
program demonstrates such false denials on SELinux, and the patch solves the
problem.  Similar such false denials have been encountered in real applications.

Please let me know if you have any comments or suggestions for improvement.

The patches are split up for the LSM/SELinux changes, the ext2
changes, and the ext3 changes.  Similar changes would be desirable for
other filesystems that support security attributes as well, such as
jfs and xfs.  A full diffstat of all three patches is below.

 fs/ext2/ialloc.c                  |    5 +++
 fs/ext2/xattr.h                   |    1 
 fs/ext2/xattr_security.c          |   22 +++++++++++++
 fs/ext3/ialloc.c                  |    5 +++
 fs/ext3/xattr.h                   |    1 
 fs/ext3/xattr_security.c          |   22 +++++++++++++
 include/linux/security.h          |   41 +++++++++++++++++++++++++
 security/dummy.c                  |    7 ++++
 security/selinux/hooks.c          |   60 ++++++++++++++++++++++++++++++++++++++
 security/selinux/include/objsec.h |    1 
 10 files changed, 165 insertions(+)

-- 
Stephen Smalley
National Security Agency



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 1/3] security:  Enable atomic inode security labeling
  2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
@ 2005-07-08 13:48 ` Stephen Smalley
  2005-07-08 13:55 ` [RFC][PATCH 2/3] ext2: " Stephen Smalley
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 25+ messages in thread
From: Stephen Smalley @ 2005-07-08 13:48 UTC (permalink / raw)
  To: linuxfs
  Cc: Alexander Viro, Ext2-devel, Andreas Gruenbacher, Andreas Dilger,
	Andrew Morton, Stephen Tweedie, James Morris, Chris Wright

This patch defines a new inode_init_security LSM hook to obtain the
security attribute to apply to a newly created inode and to set up the
incore inode security state for it, and adds a corresponding hook
function implementation to SELinux.

Please let me know if you have any comments or suggestions for improvement.

 include/linux/security.h          |   41 +++++++++++++++++++++++++
 security/dummy.c                  |    7 ++++
 security/selinux/hooks.c          |   60 ++++++++++++++++++++++++++++++++++++++
 security/selinux/include/objsec.h |    1 
 4 files changed, 109 insertions(+)

diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/include/linux/security.h linux-2.6.13/include/linux/security.h
--- linux-2.6.13.clean/include/linux/security.h	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/include/linux/security.h	2005-06-30 13:24:23.000000000 -0400
@@ -250,6 +250,25 @@ struct swap_info_struct;
  *	@inode contains the inode structure.
  *	Deallocate the inode security structure and set @inode->i_security to
  *	NULL. 
+ * @inode_init_security:
+ * 	Obtain the security attribute name suffix and value to set on a newly
+ *	created inode and set up the incore security field for the new inode.
+ *	This hook is called by the fs code as part of the inode creation 
+ *	transaction and provides for atomic labeling of the inode, unlike
+ *	the post_create/mkdir/... hooks called by the VFS.  The hook function
+ *	is expected to allocate the name and value via kmalloc, with the caller
+ *	being responsible for calling kfree after using them.  
+ *	If the security module does not use security attributes or does
+ *	not wish to put a security attribute on this particular inode, 
+ *	then it should return -EOPNOTSUPP to skip this processing.
+ *	@inode contains the inode structure of the newly created inode.
+ *	@dir contains the inode structure of the parent directory.
+ *	@name will be set to the allocated name suffix (e.g. selinux).
+ *	@value will be set to the allocated attribute value.
+ *	@len will be set to the length of the value.
+ *	Returns 0 if @name and @value have been successfully set,
+ *		-EOPNOTSUPP if no security attribute is needed, or
+ *		-ENOMEM on memory allocation failure.
  * @inode_create:
  *	Check permission to create a regular file.
  *	@dir contains inode structure of the parent of the new file.
@@ -1080,6 +1099,8 @@ struct security_operations {
 
 	int (*inode_alloc_security) (struct inode *inode);	
 	void (*inode_free_security) (struct inode *inode);
+	int (*inode_init_security) (struct inode *inode, struct inode *dir,
+				    char **name, void **value, size_t *len);
 	int (*inode_create) (struct inode *dir,
 	                     struct dentry *dentry, int mode);
 	void (*inode_post_create) (struct inode *dir,
@@ -1442,6 +1463,17 @@ static inline void security_inode_free (
 		return;
 	security_ops->inode_free_security (inode);
 }
+
+static inline int security_inode_init_security (struct inode *inode, 
+						struct inode *dir,
+						char **name,
+						void **value,
+						size_t *len)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return -EOPNOTSUPP;
+	return security_ops->inode_init_security (inode, dir, name, value, len);
+}
 	
 static inline int security_inode_create (struct inode *dir,
 					 struct dentry *dentry,
@@ -2171,6 +2203,15 @@ static inline int security_inode_alloc (
 
 static inline void security_inode_free (struct inode *inode)
 { }
+
+static inline int security_inode_init_security (struct inode *inode, 
+						struct inode *dir,
+						char **name,
+						void **value,
+						size_t *len)
+{
+	return -EOPNOTSUPP;
+}
 	
 static inline int security_inode_create (struct inode *dir,
 					 struct dentry *dentry,
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/security/dummy.c linux-2.6.13/security/dummy.c
--- linux-2.6.13.clean/security/dummy.c	2005-06-30 13:24:05.000000000 -0400
+++ linux-2.6.13/security/dummy.c	2005-06-30 13:24:23.000000000 -0400
@@ -258,6 +258,12 @@ static void dummy_inode_free_security (s
 	return;
 }
 
+static int dummy_inode_init_security (struct inode *inode, struct inode *dir,
+				      char **name, void **value, size_t *len)
+{
+	return -EOPNOTSUPP;
+}
+
 static int dummy_inode_create (struct inode *inode, struct dentry *dentry,
 			       int mask)
 {
@@ -886,6 +892,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, sb_post_pivotroot);
 	set_to_dummy_if_null(ops, inode_alloc_security);
 	set_to_dummy_if_null(ops, inode_free_security);
+	set_to_dummy_if_null(ops, inode_init_security);
 	set_to_dummy_if_null(ops, inode_create);
 	set_to_dummy_if_null(ops, inode_post_create);
 	set_to_dummy_if_null(ops, inode_link);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/security/selinux/hooks.c linux-2.6.13/security/selinux/hooks.c
--- linux-2.6.13.clean/security/selinux/hooks.c	2005-06-30 13:24:05.000000000 -0400
+++ linux-2.6.13/security/selinux/hooks.c	2005-06-30 13:24:23.000000000 -0400
@@ -1272,6 +1272,7 @@ static int post_create(struct inode *dir
 	struct inode *inode;
 	struct inode_security_struct *dsec;
 	struct superblock_security_struct *sbsec;
+	struct inode_security_struct *isec;
 	u32 newsid;
 	char *context;
 	unsigned int len;
@@ -1291,6 +1292,11 @@ static int post_create(struct inode *dir
 		return 0;
 	}
 
+	isec = inode->i_security;
+
+	if (isec->security_attr_init)
+		return 0;	
+
 	if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
 		newsid = tsec->create_sid;
 	} else {
@@ -2016,6 +2022,59 @@ static void selinux_inode_free_security(
 	inode_free_security(inode);
 }
 
+static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
+				       char **name, void **value, 
+				       size_t *len)
+{
+	struct task_security_struct *tsec;
+	struct inode_security_struct *dsec;
+	struct superblock_security_struct *sbsec;
+	struct inode_security_struct *isec;
+	u32 newsid;
+	int rc;
+	char *namep, *context;
+ 
+	tsec = current->security;
+	dsec = dir->i_security;
+	sbsec = dir->i_sb->s_security;
+	isec = inode->i_security;
+	
+	if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
+		newsid = tsec->create_sid;
+	} else {
+		rc = security_transition_sid(tsec->sid, dsec->sid,
+					     inode_mode_to_security_class(inode->i_mode),
+					     &newsid);
+		if (rc) {
+			printk(KERN_WARNING "%s:  "
+			       "security_transition_sid failed, rc=%d (dev=%s "
+			       "ino=%ld)\n",
+			       __FUNCTION__,
+			       -rc, inode->i_sb->s_id, inode->i_ino);
+			return rc;
+		}
+	}
+
+	inode_security_set_sid(inode, newsid);
+	
+	namep = kmalloc(sizeof(XATTR_SELINUX_SUFFIX), GFP_KERNEL);
+	if (!namep)
+		return -ENOMEM;
+	strcpy(namep, XATTR_SELINUX_SUFFIX);
+	*name = namep;
+	
+	rc = security_sid_to_context(newsid, &context, len);
+	if (rc) {
+		kfree(namep);
+		return rc;
+	}
+	*value = context;
+
+	isec->security_attr_init = 1;
+
+	return 0;
+}
+
 static int selinux_inode_create(struct inode *dir, struct dentry *dentry, int mask)
 {
 	return may_create(dir, dentry, SECCLASS_FILE);
@@ -4296,6 +4355,7 @@ static struct security_operations selinu
 
 	.inode_alloc_security =		selinux_inode_alloc_security,
 	.inode_free_security =		selinux_inode_free_security,
+	.inode_init_security =		selinux_inode_init_security,
 	.inode_create =			selinux_inode_create,
 	.inode_post_create =		selinux_inode_post_create,
 	.inode_link =			selinux_inode_link,
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/security/selinux/include/objsec.h linux-2.6.13/security/selinux/include/objsec.h
--- linux-2.6.13.clean/security/selinux/include/objsec.h	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/security/selinux/include/objsec.h	2005-06-30 13:24:23.000000000 -0400
@@ -46,6 +46,7 @@ struct inode_security_struct {
 	unsigned char initialized;     /* initialization flag */
 	struct semaphore sem;
 	unsigned char inherit;         /* inherit SID from parent entry */
+	unsigned char security_attr_init; /* security attributes init flag */
 };
 
 struct file_security_struct {

-- 
Stephen Smalley
National Security Agency



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
  2005-07-08 13:48 ` [RFC][PATCH 1/3] security: " Stephen Smalley
@ 2005-07-08 13:55 ` Stephen Smalley
  2005-07-10 23:39   ` Christoph Hellwig
  2005-07-13 20:37   ` Dave Kleikamp
  2005-07-08 13:58 ` [RFC][PATCH 3/3] ext3: " Stephen Smalley
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 25+ messages in thread
From: Stephen Smalley @ 2005-07-08 13:55 UTC (permalink / raw)
  To: linuxfs
  Cc: Alexander Viro, Ext2-devel, Andreas Gruenbacher, Andreas Dilger,
	Andrew Morton, Stephen Tweedie, James Morris, Chris Wright

This patch modifies ext2 to call the inode_init_security LSM hook to
obtain the security attribute for a newly created inode and to set the
resulting attribute on the new inode.  This parallels the existing
processing for setting ACLs on newly created inodes.

Please let me know if you have any comments or suggestions for improvement.

 fs/ext2/ialloc.c         |    5 +++++
 fs/ext2/xattr.h          |    1 +
 fs/ext2/xattr_security.c |   22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext2/ialloc.c linux-2.6.13/fs/ext2/ialloc.c
--- linux-2.6.13.clean/fs/ext2/ialloc.c	2005-06-17 15:48:29.000000000 -0400
+++ linux-2.6.13/fs/ext2/ialloc.c	2005-07-06 10:55:37.000000000 -0400
@@ -614,6 +614,11 @@ got:
 		DQUOT_FREE_INODE(inode);
 		goto fail2;
 	}
+	err = ext2_init_security(inode,dir);
+	if (err) {
+		DQUOT_FREE_INODE(inode);
+		goto fail2;
+	}
 	mark_inode_dirty(inode);
 	ext2_debug("allocating inode %lu\n", inode->i_ino);
 	ext2_preread_inode(inode);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext2/xattr.h linux-2.6.13/fs/ext2/xattr.h
--- linux-2.6.13.clean/fs/ext2/xattr.h	2005-06-17 15:48:29.000000000 -0400
+++ linux-2.6.13/fs/ext2/xattr.h	2005-07-06 10:55:37.000000000 -0400
@@ -64,6 +64,7 @@ extern struct xattr_handler ext2_xattr_s
 
 extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
 
+extern int ext2_init_security(struct inode *inode, struct inode *dir);
 extern int ext2_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext2_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
 
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext2/xattr_security.c linux-2.6.13/fs/ext2/xattr_security.c
--- linux-2.6.13.clean/fs/ext2/xattr_security.c	2005-06-17 15:48:29.000000000 -0400
+++ linux-2.6.13/fs/ext2/xattr_security.c	2005-07-06 10:56:20.000000000 -0400
@@ -8,6 +8,7 @@
 #include <linux/fs.h>
 #include <linux/smp_lock.h>
 #include <linux/ext2_fs.h>
+#include <linux/security.h>
 #include "xattr.h"
 
 static size_t
@@ -45,6 +46,27 @@ ext2_xattr_security_set(struct inode *in
 			      value, size, flags);
 }
 
+int
+ext2_init_security(struct inode *inode, struct inode *dir)
+{
+	int err;
+	size_t len;
+	void *value;
+	char *name;
+
+	err = security_inode_init_security(inode, dir, &name, &value, &len);
+	if (err) {
+		if (err == -EOPNOTSUPP)
+			return 0;
+		return err;
+	}
+	err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY, 
+			     name, value, len, 0);
+	kfree(name);
+	kfree(value);
+	return err;
+}
+
 struct xattr_handler ext2_xattr_security_handler = {
 	.prefix	= XATTR_SECURITY_PREFIX,
 	.list	= ext2_xattr_security_list,

-- 
Stephen Smalley
National Security Agency



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 3/3] ext3:  Enable atomic inode security labeling
  2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
  2005-07-08 13:48 ` [RFC][PATCH 1/3] security: " Stephen Smalley
  2005-07-08 13:55 ` [RFC][PATCH 2/3] ext2: " Stephen Smalley
@ 2005-07-08 13:58 ` Stephen Smalley
  2005-07-11 16:07   ` Stephen C. Tweedie
  2005-07-10 23:40 ` [RFC][PATCH 0/3] " Christoph Hellwig
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 25+ messages in thread
From: Stephen Smalley @ 2005-07-08 13:58 UTC (permalink / raw)
  To: linuxfs
  Cc: Alexander Viro, Ext2-devel, Andreas Gruenbacher, Andreas Dilger,
	Andrew Morton, Stephen Tweedie, James Morris, Chris Wright

This patch modifies ext3 to call the inode_init_security LSM hook to
obtain the security attribute for a newly created inode and to set the
resulting attribute on the new inode as part of the same transaction.
This parallels the existing processing for setting ACLs on newly
created inodes.

Please let me know if you have any comments or suggestions for improvement.

 fs/ext3/ialloc.c         |    5 +++++
 fs/ext3/xattr.h          |    1 +
 fs/ext3/xattr_security.c |   22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext3/ialloc.c linux-2.6.13/fs/ext3/ialloc.c
--- linux-2.6.13.clean/fs/ext3/ialloc.c	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/fs/ext3/ialloc.c	2005-06-30 13:24:23.000000000 -0400
@@ -606,6 +606,11 @@ got:
 		DQUOT_FREE_INODE(inode);
 		goto fail2;
   	}
+	err = ext3_init_security(handle,inode, dir);
+	if (err) {
+		DQUOT_FREE_INODE(inode);
+		goto fail2;
+	}
 	err = ext3_mark_inode_dirty(handle, inode);
 	if (err) {
 		ext3_std_error(sb, err);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext3/xattr.h linux-2.6.13/fs/ext3/xattr.h
--- linux-2.6.13.clean/fs/ext3/xattr.h	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/fs/ext3/xattr.h	2005-06-30 13:24:23.000000000 -0400
@@ -67,6 +67,7 @@ extern struct xattr_handler ext3_xattr_s
 
 extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
 
+extern int ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir);
 extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext3_xattr_list(struct inode *, char *, size_t);
 extern int ext3_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext3/xattr_security.c linux-2.6.13/fs/ext3/xattr_security.c
--- linux-2.6.13.clean/fs/ext3/xattr_security.c	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/fs/ext3/xattr_security.c	2005-06-30 13:24:23.000000000 -0400
@@ -9,6 +9,7 @@
 #include <linux/smp_lock.h>
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
+#include <linux/security.h>
 #include "xattr.h"
 
 static size_t
@@ -47,6 +48,27 @@ ext3_xattr_security_set(struct inode *in
 			      value, size, flags);
 }
 
+int
+ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir)
+{
+	int err;
+	size_t len;
+	void *value;
+	char *name;
+
+	err = security_inode_init_security(inode, dir, &name, &value, &len);
+	if (err) {
+		if (err == -EOPNOTSUPP)
+			return 0;
+		return err;
+	}
+	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
+				    name, value, len, 0);
+	kfree(name);
+	kfree(value);
+	return err;
+}
+
 struct xattr_handler ext3_xattr_security_handler = {
 	.prefix	= XATTR_SECURITY_PREFIX,
 	.list	= ext3_xattr_security_list,


-- 
Stephen Smalley
National Security Agency



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-08 13:55 ` [RFC][PATCH 2/3] ext2: " Stephen Smalley
@ 2005-07-10 23:39   ` Christoph Hellwig
  2005-07-11 12:53     ` Stephen Smalley
  2005-07-13 20:37   ` Dave Kleikamp
  1 sibling, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2005-07-10 23:39 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Fri, Jul 08, 2005 at 09:55:14AM -0400, Stephen Smalley wrote:
> +int
> +ext2_init_security(struct inode *inode, struct inode *dir)
> +{
> +	int err;
> +	size_t len;
> +	void *value;
> +	char *name;
> +
> +	err = security_inode_init_security(inode, dir, &name, &value, &len);
> +	if (err) {
> +		if (err == -EOPNOTSUPP)
> +			return 0;
> +		return err;
> +	}
> +	err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY, 
> +			     name, value, len, 0);
> +	kfree(name);
> +	kfree(value);

Please set the xattr from security_inode_init_security by using ->setxattr, that
way we don't need to duplicate this code everywhere.



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 0/3] Enable atomic inode security labeling
  2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
                   ` (2 preceding siblings ...)
  2005-07-08 13:58 ` [RFC][PATCH 3/3] ext3: " Stephen Smalley
@ 2005-07-10 23:40 ` Christoph Hellwig
  2005-07-11 13:31   ` Stephen Smalley
  2005-07-13 15:05 ` [RFC][PATCH 2.6.13-rc2-mm2] tmpfs: " Stephen Smalley
  2005-07-14 16:16 ` [RFC][PATCH 0/2] JFS atomic inode security labeling Dave Kleikamp
  5 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2005-07-10 23:40 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Fri, Jul 08, 2005 at 09:25:21AM -0400, Stephen Smalley wrote:
> The following patch set enables atomic security labeling of newly
> created inodes by altering the fs code to invoke a new LSM hook to
> obtain the security attribute to apply to a newly created inode and to
> set up the incore inode security state during the inode creation
> transaction.  This parallels the existing processing for setting ACLs
> on newly created inodes.  Otherwise, it is possible for new inodes to
> be accessed by another thread via the dcache prior to complete
> security setup (presently handled by the post_create/mkdir/... LSM
> hooks in the VFS)

Please also kill these hooks now that they've been replaced with something
more useful and make sure selinux doesn't work on filesystem not converted
to the new method.



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-10 23:39   ` Christoph Hellwig
@ 2005-07-11 12:53     ` Stephen Smalley
  2005-07-12  2:29       ` Christoph Hellwig
  0 siblings, 1 reply; 25+ messages in thread
From: Stephen Smalley @ 2005-07-11 12:53 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Mon, 2005-07-11 at 00:39 +0100, Christoph Hellwig wrote:
> On Fri, Jul 08, 2005 at 09:55:14AM -0400, Stephen Smalley wrote:
> > +int
> > +ext2_init_security(struct inode *inode, struct inode *dir)
> > +{
> > +	int err;
> > +	size_t len;
> > +	void *value;
> > +	char *name;
> > +
> > +	err = security_inode_init_security(inode, dir, &name, &value, &len);
> > +	if (err) {
> > +		if (err == -EOPNOTSUPP)
> > +			return 0;
> > +		return err;
> > +	}
> > +	err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY, 
> > +			     name, value, len, 0);
> > +	kfree(name);
> > +	kfree(value);
> 
> Please set the xattr from security_inode_init_security by using ->setxattr, that
> way we don't need to duplicate this code everywhere.

That doesn't allow us to ensure that the setting of the xattr occurs in
the same transaction as the create (in the ext3 case, doesn't matter for
ext2), so you can still have a crash and leave an unlabeled file around.
Just followed the example of the ACL code here, except that it doesn't
need to call to a security module to determine the ACL of the new inode.

-- 
Stephen Smalley
National Security Agency


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 0/3] Enable atomic inode security labeling
  2005-07-10 23:40 ` [RFC][PATCH 0/3] " Christoph Hellwig
@ 2005-07-11 13:31   ` Stephen Smalley
  2005-07-12  2:32     ` Christoph Hellwig
  0 siblings, 1 reply; 25+ messages in thread
From: Stephen Smalley @ 2005-07-11 13:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Mon, 2005-07-11 at 00:40 +0100, Christoph Hellwig wrote:
> On Fri, Jul 08, 2005 at 09:25:21AM -0400, Stephen Smalley wrote:
> > The following patch set enables atomic security labeling of newly
> > created inodes by altering the fs code to invoke a new LSM hook to
> > obtain the security attribute to apply to a newly created inode and to
> > set up the incore inode security state during the inode creation
> > transaction.  This parallels the existing processing for setting ACLs
> > on newly created inodes.  Otherwise, it is possible for new inodes to
> > be accessed by another thread via the dcache prior to complete
> > security setup (presently handled by the post_create/mkdir/... LSM
> > hooks in the VFS)
> 
> Please also kill these hooks now that they've been replaced with something
> more useful and make sure selinux doesn't work on filesystem not converted
> to the new method.

I was planning on leaving the security_inode_post* hooks intact at least
until the other filesystem types that support security xattrs have all
been converted to use the new hook, and even then only after a separate
RFC to confirm that it is ok to kill those hooks.  Otherwise, we'd be
breaking any existing systems that are using SELinux with xfs, jfs, or
reiserfs.  Also likely need to update tmpfs for the incore inode
security setup of new inodes prior to killing those hooks.

-- 
Stephen Smalley
National Security Agency


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 3/3] ext3:  Enable atomic inode security labeling
  2005-07-08 13:58 ` [RFC][PATCH 3/3] ext3: " Stephen Smalley
@ 2005-07-11 16:07   ` Stephen C. Tweedie
  2005-07-11 16:14     ` Jan Kara
  0 siblings, 1 reply; 25+ messages in thread
From: Stephen C. Tweedie @ 2005-07-11 16:07 UTC (permalink / raw)
  To: Stephen Smalley, Jan Kara
  Cc: linuxfs, Al Viro, ext2-devel@lists.sourceforge.net,
	Andreas Gruenbacher, Andreas Dilger, Andrew Morton, James Morris,
	Chris Wright, Stephen Tweedie

Hi,

On Fri, 2005-07-08 at 14:58, Stephen Smalley wrote:
> This patch modifies ext3 to call the inode_init_security LSM hook to
> obtain the security attribute for a newly created inode and to set the
> resulting attribute on the new inode as part of the same transaction.
> This parallels the existing processing for setting ACLs on newly
> created inodes.
> 
> Please let me know if you have any comments or suggestions for improvement.

> +	err = ext3_init_security(handle,inode, dir);
> +	if (err) {
> +		DQUOT_FREE_INODE(inode);
> +		goto fail2;
> +	}

This looks fine but it lead me to something else...

I wanted to just double-check that the above code gets the inode cleanup
right if we get EDQUOT when allocating the EA block during inode
initialisation.  I tried it with the equivalent ACL block of code, not
actually using SELinux, and it worked as expected (set a default ACL on
a directory, set quota to prevent further block allocations, and try to
allocate a new inode; you get EDQUOT back.)

Trouble is, on unmounting the fs I got a 

Kernel BUG at "fs/dquot.c":422

in invalidate_dquots:
#ifdef __DQUOT_PARANOIA
		if (atomic_read(&dquot->dq_count))
			BUG();
#endif

backtrace
RIP: 0010:[<ffffffff801a62e0>] <ffffffff801a62e0>{vfs_quota_off+512}
Call Trace:<ffffffff80190ed5>{sys_umount+533} <ffffffff8010f31d>{error_exit+0}
       <ffffffff8010e99e>{system_call+126} 

It seems repeatable to me --- it hit twice out of two attempts.  Jan,
recognise this problem?

Cheers,
 Stephen



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 3/3] ext3:  Enable atomic inode security labeling
  2005-07-11 16:07   ` Stephen C. Tweedie
@ 2005-07-11 16:14     ` Jan Kara
  2005-07-11 16:50       ` Stephen C. Tweedie
  0 siblings, 1 reply; 25+ messages in thread
From: Jan Kara @ 2005-07-11 16:14 UTC (permalink / raw)
  To: Stephen C. Tweedie
  Cc: Stephen Smalley, linuxfs, Al Viro,
	ext2-devel@lists.sourceforge.net, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, James Morris, Chris Wright

  Hello,

> On Fri, 2005-07-08 at 14:58, Stephen Smalley wrote:
> > This patch modifies ext3 to call the inode_init_security LSM hook to
> > obtain the security attribute for a newly created inode and to set the
> > resulting attribute on the new inode as part of the same transaction.
> > This parallels the existing processing for setting ACLs on newly
> > created inodes.
> > 
> > Please let me know if you have any comments or suggestions for improvement.
> 
> > +	err = ext3_init_security(handle,inode, dir);
> > +	if (err) {
> > +		DQUOT_FREE_INODE(inode);
> > +		goto fail2;
> > +	}
> 
> This looks fine but it lead me to something else...
> 
> I wanted to just double-check that the above code gets the inode cleanup
> right if we get EDQUOT when allocating the EA block during inode
> initialisation.  I tried it with the equivalent ACL block of code, not
> actually using SELinux, and it worked as expected (set a default ACL on
> a directory, set quota to prevent further block allocations, and try to
> allocate a new inode; you get EDQUOT back.)
> 
> Trouble is, on unmounting the fs I got a 
> 
> Kernel BUG at "fs/dquot.c":422
> 
> in invalidate_dquots:
> #ifdef __DQUOT_PARANOIA
> 		if (atomic_read(&dquot->dq_count))
> 			BUG();
> #endif
> 
> backtrace
> RIP: 0010:[<ffffffff801a62e0>] <ffffffff801a62e0>{vfs_quota_off+512}
> Call Trace:<ffffffff80190ed5>{sys_umount+533} <ffffffff8010f31d>{error_exit+0}
>        <ffffffff8010e99e>{system_call+126} 
> 
> It seems repeatable to me --- it hit twice out of two attempts.  Jan,
> recognise this problem?
  Which kernel was that? We had a similar bug in 2.6.11 kernel but it
should be fixed in 2.6.12. I suppose you get the same BUG if you run
just quotaoff(8), right?

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs


-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 3/3] ext3:  Enable atomic inode security labeling
  2005-07-11 16:14     ` Jan Kara
@ 2005-07-11 16:50       ` Stephen C. Tweedie
  2005-07-12 14:15         ` [Ext2-devel] " Jan Kara
  0 siblings, 1 reply; 25+ messages in thread
From: Stephen C. Tweedie @ 2005-07-11 16:50 UTC (permalink / raw)
  To: Jan Kara
  Cc: Stephen Smalley, linuxfs, Al Viro,
	ext2-devel@lists.sourceforge.net, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, James Morris, Chris Wright,
	Stephen Tweedie

Hi,

On Mon, 2005-07-11 at 17:14, Jan Kara wrote:

>   Which kernel was that? We had a similar bug in 2.6.11 kernel but it
> should be fixed in 2.6.12.

This was 2.6.13-rc2 from today's git.

>  I suppose you get the same BUG if you run
> just quotaoff(8), right?

I just tried --- yes.

Cheers,
 Stephen


-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-11 12:53     ` Stephen Smalley
@ 2005-07-12  2:29       ` Christoph Hellwig
  0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2005-07-12  2:29 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Christoph Hellwig, linuxfs, Alexander Viro, Ext2-devel,
	Andreas Gruenbacher, Andreas Dilger, Andrew Morton,
	Stephen Tweedie, James Morris, Chris Wright

On Mon, Jul 11, 2005 at 08:53:02AM -0400, Stephen Smalley wrote:
> > Please set the xattr from security_inode_init_security by using ->setxattr, that
> > way we don't need to duplicate this code everywhere.
> 
> That doesn't allow us to ensure that the setting of the xattr occurs in
> the same transaction as the create (in the ext3 case, doesn't matter for
> ext2), so you can still have a crash and leave an unlabeled file around.
> Just followed the example of the ACL code here, except that it doesn't
> need to call to a security module to determine the ACL of the new inode.

Makes sense.  As unfortunate as the code duplicate is we'll have to live
with it it seems.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 0/3] Enable atomic inode security labeling
  2005-07-11 13:31   ` Stephen Smalley
@ 2005-07-12  2:32     ` Christoph Hellwig
  0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2005-07-12  2:32 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Mon, Jul 11, 2005 at 09:31:39AM -0400, Stephen Smalley wrote:
> I was planning on leaving the security_inode_post* hooks intact at least
> until the other filesystem types that support security xattrs have all
> been converted to use the new hook,

Having these transactional guarantees just for some filesystem and not
others is really bad, we should provide a coherent interface.

> and even then only after a separate
> RFC to confirm that it is ok to kill those hooks.  Otherwise, we'd be
> breaking any existing systems that are using SELinux with xfs, jfs, or
> reiserfs.

Partial transitions are always bad things, and we should avoid them
for one that only affects these few users of the interface.  Just keep
the patch in -mm until people have updated these filesystems.
I'll look at XFS ASAP.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Ext2-devel] Re: [RFC][PATCH 3/3] ext3:  Enable atomic inode security labeling
  2005-07-11 16:50       ` Stephen C. Tweedie
@ 2005-07-12 14:15         ` Jan Kara
  0 siblings, 0 replies; 25+ messages in thread
From: Jan Kara @ 2005-07-12 14:15 UTC (permalink / raw)
  To: Stephen C. Tweedie
  Cc: Stephen Smalley, linuxfs, Al Viro,
	ext2-devel@lists.sourceforge.net, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, James Morris, Chris Wright

  Hi,

> On Mon, 2005-07-11 at 17:14, Jan Kara wrote:
> 
> >   Which kernel was that? We had a similar bug in 2.6.11 kernel but it
> > should be fixed in 2.6.12.
> 
> This was 2.6.13-rc2 from today's git.
> 
> >  I suppose you get the same BUG if you run
> > just quotaoff(8), right?
> 
> I just tried --- yes.
  OK, I'll have a look why this happens... It seems like we forget to
drop quota reference somewhere.

										Honza

-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 2.6.13-rc2-mm2] tmpfs:  Enable atomic inode security labeling
  2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
                   ` (3 preceding siblings ...)
  2005-07-10 23:40 ` [RFC][PATCH 0/3] " Christoph Hellwig
@ 2005-07-13 15:05 ` Stephen Smalley
  2005-07-14 19:29   ` [RFC][PATCH] Remove security_inode_post_create/mkdir/symlink/mknod hooks Stephen Smalley
  2005-07-14 16:16 ` [RFC][PATCH 0/2] JFS atomic inode security labeling Dave Kleikamp
  5 siblings, 1 reply; 25+ messages in thread
From: Stephen Smalley @ 2005-07-13 15:05 UTC (permalink / raw)
  To: linuxfs
  Cc: Christoph Hellwig, Alexander Viro, Andreas Gruenbacher,
	Andrew Morton, Stephen Tweedie, James Morris, Chris Wright

This patch modifies tmpfs to call the inode_init_security LSM hook to
set up the incore inode security state for new inodes before the inode
becomes accessible via the dcache.  As there is no underlying storage
of security xattrs in this case, it is not necessary for the hook to
return the (name, value, len) triple to the tmpfs code, so this patch
also modifies the SELinux hook function to correctly handle the case where
the (name, value, len) pointers are NULL.  The hook call is needed in tmpfs in 
order to support proper security labeling of tmpfs inodes (e.g. for udev with tmpfs 
/dev in Fedora).  With this change in place, we should then be able to remove 
the security_inode_post_create/mkdir/... hooks safely.

Signed-off-by:  Stephen Smalley <sds@tycho.nsa.gov>

 mm/shmem.c               |   20 +++++++++++++++++++-
 security/selinux/hooks.c |   24 ++++++++++++++----------
 2 files changed, 33 insertions(+), 11 deletions(-)

diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/mm/shmem.c linux-2.6.13-rc2-mm2-sds/mm/shmem.c
--- linux-2.6.13-rc2-mm2/mm/shmem.c	2005-07-12 09:09:34.000000000 -0400
+++ linux-2.6.13-rc2-mm2-sds/mm/shmem.c	2005-07-13 09:30:53.000000000 -0400
@@ -1606,6 +1606,15 @@ shmem_mknod(struct inode *dir, struct de
 	int error = -ENOSPC;
 
 	if (inode) {
+		error = security_inode_init_security(inode, dir, NULL, NULL,
+						     NULL);
+		if (error) {
+			if (error != -EOPNOTSUPP) {
+				iput(inode);
+				return error;
+			}
+			error = 0;
+		}
 		if (dir->i_mode & S_ISGID) {
 			inode->i_gid = dir->i_gid;
 			if (S_ISDIR(mode))
@@ -1615,7 +1624,6 @@ shmem_mknod(struct inode *dir, struct de
 		dir->i_ctime = dir->i_mtime = CURRENT_TIME;
 		d_instantiate(dentry, inode);
 		dget(dentry); /* Extra count - pin the dentry in core */
-		error = 0;
 	}
 	return error;
 }
@@ -1745,6 +1753,16 @@ static int shmem_symlink(struct inode *d
 	if (!inode)
 		return -ENOSPC;
 
+	error = security_inode_init_security(inode, dir, NULL, NULL,
+					     NULL);
+	if (error) {
+		if (error != -EOPNOTSUPP) {
+			iput(inode);
+			return error;
+		}
+		error = 0;
+	}
+
 	info = SHMEM_I(inode);
 	inode->i_size = len-1;
 	if (len <= (char *)inode - (char *)info) {
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/security/selinux/hooks.c linux-2.6.13-rc2-mm2-sds/security/selinux/hooks.c
--- linux-2.6.13-rc2-mm2/security/selinux/hooks.c	2005-07-12 09:10:32.000000000 -0400
+++ linux-2.6.13-rc2-mm2-sds/security/selinux/hooks.c	2005-07-13 09:36:22.000000000 -0400
@@ -2035,7 +2035,7 @@ static int selinux_inode_init_security(s
 	struct inode_security_struct *isec;
 	u32 newsid;
 	int rc;
-	char *namep, *context;
+	char *namep = NULL, *context;
 
 	tsec = current->security;
 	dsec = dir->i_security;
@@ -2060,17 +2060,21 @@ static int selinux_inode_init_security(s
 
 	inode_security_set_sid(inode, newsid);
 
-	namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_KERNEL);
-	if (!namep)
-		return -ENOMEM;
-	*name = namep;
+	if (name) {
+		namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_KERNEL);
+		if (!namep)
+			return -ENOMEM;
+		*name = namep;
+	}
 
-	rc = security_sid_to_context(newsid, &context, len);
-	if (rc) {
-		kfree(namep);
-		return rc;
+	if (value && len) {
+		rc = security_sid_to_context(newsid, &context, len);
+		if (rc) {
+			kfree(namep);
+			return rc;
+		}
+		*value = context;
 	}
-	*value = context;
 
 	isec->security_attr_init = 1;
 

-- 
Stephen Smalley
National Security Agency


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-08 13:55 ` [RFC][PATCH 2/3] ext2: " Stephen Smalley
  2005-07-10 23:39   ` Christoph Hellwig
@ 2005-07-13 20:37   ` Dave Kleikamp
  2005-07-13 20:41     ` Stephen Smalley
  2005-07-13 20:50     ` Andrew Morton
  1 sibling, 2 replies; 25+ messages in thread
From: Dave Kleikamp @ 2005-07-13 20:37 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Fri, 2005-07-08 at 09:55 -0400, Stephen Smalley wrote:
> This patch modifies ext2 to call the inode_init_security LSM hook to
> obtain the security attribute for a newly created inode and to set the
> resulting attribute on the new inode.  This parallels the existing
> processing for setting ACLs on newly created inodes.
> 
> Please let me know if you have any comments or suggestions for improvement.
> 
>  fs/ext2/ialloc.c         |    5 +++++
>  fs/ext2/xattr.h          |    1 +
>  fs/ext2/xattr_security.c |   22 ++++++++++++++++++++++
>  3 files changed, 28 insertions(+)
> 
> diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext2/ialloc.c linux-2.6.13/fs/ext2/ialloc.c
> --- linux-2.6.13.clean/fs/ext2/ialloc.c	2005-06-17 15:48:29.000000000 -0400
> +++ linux-2.6.13/fs/ext2/ialloc.c	2005-07-06 10:55:37.000000000 -0400
> @@ -614,6 +614,11 @@ got:
>  		DQUOT_FREE_INODE(inode);
>  		goto fail2;
>  	}
> +	err = ext2_init_security(inode,dir);

Won't this be unresolved if CONFIG_EXT2_FS_SECURITY is unset?
xattr_security.c won't be compiled at all.  The ext3 patch has the same
problem.

BTW, I'm working on a patch for jfs.  It's a little more complicated
since jfs's xattr code creates a transaction down in the lower level
code.  I'm going to have to restructure it so the caller creates the
transaction and passes down the tid.

> +	if (err) {
> +		DQUOT_FREE_INODE(inode);
> +		goto fail2;
> +	}
>  	mark_inode_dirty(inode);
>  	ext2_debug("allocating inode %lu\n", inode->i_ino);
>  	ext2_preread_inode(inode);
> diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext2/xattr.h linux-2.6.13/fs/ext2/xattr.h
> --- linux-2.6.13.clean/fs/ext2/xattr.h	2005-06-17 15:48:29.000000000 -0400
> +++ linux-2.6.13/fs/ext2/xattr.h	2005-07-06 10:55:37.000000000 -0400
> @@ -64,6 +64,7 @@ extern struct xattr_handler ext2_xattr_s
>  
>  extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
>  
> +extern int ext2_init_security(struct inode *inode, struct inode *dir);
>  extern int ext2_xattr_get(struct inode *, int, const char *, void *, size_t);
>  extern int ext2_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
>  
> diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext2/xattr_security.c linux-2.6.13/fs/ext2/xattr_security.c
> --- linux-2.6.13.clean/fs/ext2/xattr_security.c	2005-06-17 15:48:29.000000000 -0400
> +++ linux-2.6.13/fs/ext2/xattr_security.c	2005-07-06 10:56:20.000000000 -0400
> @@ -8,6 +8,7 @@
>  #include <linux/fs.h>
>  #include <linux/smp_lock.h>
>  #include <linux/ext2_fs.h>
> +#include <linux/security.h>
>  #include "xattr.h"
>  
>  static size_t
> @@ -45,6 +46,27 @@ ext2_xattr_security_set(struct inode *in
>  			      value, size, flags);
>  }
>  
> +int
> +ext2_init_security(struct inode *inode, struct inode *dir)
> +{
> +	int err;
> +	size_t len;
> +	void *value;
> +	char *name;
> +
> +	err = security_inode_init_security(inode, dir, &name, &value, &len);
> +	if (err) {
> +		if (err == -EOPNOTSUPP)
> +			return 0;
> +		return err;
> +	}
> +	err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY, 
> +			     name, value, len, 0);
> +	kfree(name);
> +	kfree(value);
> +	return err;
> +}
> +
>  struct xattr_handler ext2_xattr_security_handler = {
>  	.prefix	= XATTR_SECURITY_PREFIX,
>  	.list	= ext2_xattr_security_list,
> 
-- 
David Kleikamp
IBM Linux Technology Center



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-13 20:37   ` Dave Kleikamp
@ 2005-07-13 20:41     ` Stephen Smalley
  2005-07-13 20:50     ` Andrew Morton
  1 sibling, 0 replies; 25+ messages in thread
From: Stephen Smalley @ 2005-07-13 20:41 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

On Wed, 2005-07-13 at 15:37 -0500, Dave Kleikamp wrote:
> Won't this be unresolved if CONFIG_EXT2_FS_SECURITY is unset?
> xattr_security.c won't be compiled at all.  The ext3 patch has the same
> problem.

Yes, Andrew already caught that and fixed it in -mm (the fixed patches
are in 2.6.13-rc2-mm2).  Sorry.

> BTW, I'm working on a patch for jfs.  It's a little more complicated
> since jfs's xattr code creates a transaction down in the lower level
> code.  I'm going to have to restructure it so the caller creates the
> transaction and passes down the tid.

Ok, thanks for working on it.

-- 
Stephen Smalley
National Security Agency


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 2/3] ext2: Enable atomic inode security labeling
  2005-07-13 20:37   ` Dave Kleikamp
  2005-07-13 20:41     ` Stephen Smalley
@ 2005-07-13 20:50     ` Andrew Morton
  1 sibling, 0 replies; 25+ messages in thread
From: Andrew Morton @ 2005-07-13 20:50 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: sds, linux-fsdevel, viro, Ext2-devel, agruen, adilger, sct,
	jmorris, chrisw

Dave Kleikamp <shaggy@austin.ibm.com> wrote:
>
> > +	err = ext2_init_security(inode,dir);
> 
> Won't this be unresolved if CONFIG_EXT2_FS_SECURITY is unset?
> xattr_security.c won't be compiled at all.  The ext3 patch has the same
> problem.

Yeah.  I think I kicked this into shape in rc2-mm2:

--- 25/fs/ext3/xattr.h~ext3-enable-atomic-inode-security-labeling-fix	2005-07-12 05:02:43.000000000 -0600
+++ 25-akpm/fs/ext3/xattr.h	2005-07-12 05:18:06.000000000 -0600
@@ -67,7 +67,6 @@ extern struct xattr_handler ext3_xattr_s
 
 extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
 
-extern int ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir);
 extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext3_xattr_list(struct inode *, char *, size_t);
 extern int ext3_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
@@ -134,3 +133,14 @@ exit_ext3_xattr(void)
 #define ext3_xattr_handlers	NULL
 
 # endif  /* CONFIG_EXT3_FS_XATTR */
+
+#ifdef CONFIG_EXT3_FS_SECURITY
+extern int ext3_init_security(handle_t *handle, struct inode *inode,
+				struct inode *dir);
+#else
+static inline int ext3_init_security(handle_t *handle, struct inode *inode,
+				struct inode *dir)
+{
+	return 0;
+}
+#endif
_



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 0/2] JFS atomic inode security labeling
  2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
                   ` (4 preceding siblings ...)
  2005-07-13 15:05 ` [RFC][PATCH 2.6.13-rc2-mm2] tmpfs: " Stephen Smalley
@ 2005-07-14 16:16 ` Dave Kleikamp
  2005-07-14 16:19   ` [RFC][PATCH 1/2] JFS atomic xattr/acl handling Dave Kleikamp
                     ` (2 more replies)
  5 siblings, 3 replies; 25+ messages in thread
From: Dave Kleikamp @ 2005-07-14 16:16 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

Here's the jfs implementation.  The first patch fixes the transaction
layering in the xattr code to allow xattrs to be added to an inode as a
part of an existing transaction, and correctly calls jfs_init_acl()
within the same transaction that creates a file/directory/device.

The second patch actually implements jfs_init_security() similarly to
the ext2/3 patches.

The first patch was lightly tested, and the second was compile tested
only.
-- 
David Kleikamp
IBM Linux Technology Center



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 1/2] JFS atomic xattr/acl handling
  2005-07-14 16:16 ` [RFC][PATCH 0/2] JFS atomic inode security labeling Dave Kleikamp
@ 2005-07-14 16:19   ` Dave Kleikamp
  2005-07-14 16:20   ` [RFC][PATCH 2/2] JFS atomic inode security labeling Dave Kleikamp
  2005-07-14 16:26   ` [RFC][PATCH 0/2] " Dave Kleikamp
  2 siblings, 0 replies; 25+ messages in thread
From: Dave Kleikamp @ 2005-07-14 16:19 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

__jfs_setxattr should allow extended attributes to be set within an
existing transaction

diff -urp linux-2.6.13-rc3/fs/jfs/acl.c linux-1/fs/jfs/acl.c
--- linux-2.6.13-rc3/fs/jfs/acl.c	2005-07-13 10:03:40.000000000 -0500
+++ linux-1/fs/jfs/acl.c	2005-07-14 09:19:50.000000000 -0500
@@ -23,6 +23,7 @@
 #include <linux/quotaops.h>
 #include <linux/posix_acl_xattr.h>
 #include "jfs_incore.h"
+#include "jfs_txnmgr.h"
 #include "jfs_xattr.h"
 #include "jfs_acl.h"
 
@@ -75,7 +76,8 @@ static struct posix_acl *jfs_get_acl(str
 	return acl;
 }
 
-static int jfs_set_acl(struct inode *inode, int type, struct posix_acl *acl)
+static int jfs_set_acl(tid_t tid, struct inode *inode, int type,
+		       struct posix_acl *acl)
 {
 	char *ea_name;
 	struct jfs_inode_info *ji = JFS_IP(inode);
@@ -110,7 +112,7 @@ static int jfs_set_acl(struct inode *ino
 		if (rc < 0)
 			goto out;
 	}
-	rc = __jfs_setxattr(inode, ea_name, value, size, 0);
+	rc = __jfs_setxattr(tid, inode, ea_name, value, size, 0);
 out:
 	kfree(value);
 
@@ -143,7 +145,7 @@ int jfs_permission(struct inode *inode, 
 	return generic_permission(inode, mask, jfs_check_acl);
 }
 
-int jfs_init_acl(struct inode *inode, struct inode *dir)
+int jfs_init_acl(tid_t tid, struct inode *inode, struct inode *dir)
 {
 	struct posix_acl *acl = NULL;
 	struct posix_acl *clone;
@@ -159,7 +161,7 @@ int jfs_init_acl(struct inode *inode, st
 
 	if (acl) {
 		if (S_ISDIR(inode->i_mode)) {
-			rc = jfs_set_acl(inode, ACL_TYPE_DEFAULT, acl);
+			rc = jfs_set_acl(tid, inode, ACL_TYPE_DEFAULT, acl);
 			if (rc)
 				goto cleanup;
 		}
@@ -173,7 +175,8 @@ int jfs_init_acl(struct inode *inode, st
 		if (rc >= 0) {
 			inode->i_mode = mode;
 			if (rc > 0)
-				rc = jfs_set_acl(inode, ACL_TYPE_ACCESS, clone);
+				rc = jfs_set_acl(tid, inode, ACL_TYPE_ACCESS,
+						 clone);
 		}
 		posix_acl_release(clone);
 cleanup:
@@ -202,8 +205,15 @@ static int jfs_acl_chmod(struct inode *i
 		return -ENOMEM;
 
 	rc = posix_acl_chmod_masq(clone, inode->i_mode);
-	if (!rc)
-		rc = jfs_set_acl(inode, ACL_TYPE_ACCESS, clone);
+	if (!rc) {
+		tid_t tid = txBegin(inode->i_sb, 0);
+		down(&JFS_IP(inode)->commit_sem);
+		rc = jfs_set_acl(tid, inode, ACL_TYPE_ACCESS, clone);
+		if (!rc)
+			rc = txCommit(tid, 1, &inode, 0);
+		txEnd(tid);
+		up(&JFS_IP(inode)->commit_sem);
+	}
 
 	posix_acl_release(clone);
 	return rc;
diff -urp linux-2.6.13-rc3/fs/jfs/jfs_acl.h linux-1/fs/jfs/jfs_acl.h
--- linux-2.6.13-rc3/fs/jfs/jfs_acl.h	2005-07-13 10:03:40.000000000 -0500
+++ linux-1/fs/jfs/jfs_acl.h	2005-07-14 08:45:01.000000000 -0500
@@ -21,8 +21,16 @@
 #ifdef CONFIG_JFS_POSIX_ACL
 
 int jfs_permission(struct inode *, int, struct nameidata *);
-int jfs_init_acl(struct inode *, struct inode *);
+int jfs_init_acl(tid_t, struct inode *, struct inode *);
 int jfs_setattr(struct dentry *, struct iattr *);
 
-#endif		/* CONFIG_JFS_POSIX_ACL */
+#else
+
+static inline int jfs_init_acl(tid_t tid, struct inode *inode,
+			       struct inode *dir)
+{
+	return 0;
+}
+
+#endif
 #endif		/* _H_JFS_ACL */
diff -urp linux-2.6.13-rc3/fs/jfs/jfs_xattr.h linux-1/fs/jfs/jfs_xattr.h
--- linux-2.6.13-rc3/fs/jfs/jfs_xattr.h	2005-06-17 14:48:29.000000000 -0500
+++ linux-1/fs/jfs/jfs_xattr.h	2005-07-13 16:59:04.000000000 -0500
@@ -52,8 +52,8 @@ struct jfs_ea_list {
 #define	END_EALIST(ealist) \
 	((struct jfs_ea *) (((char *) (ealist)) + EALIST_SIZE(ealist)))
 
-extern int __jfs_setxattr(struct inode *, const char *, const void *, size_t,
-			  int);
+extern int __jfs_setxattr(tid_t, struct inode *, const char *, const void *,
+			  size_t, int);
 extern int jfs_setxattr(struct dentry *, const char *, const void *, size_t,
 			int);
 extern ssize_t __jfs_getxattr(struct inode *, const char *, void *, size_t);
diff -urp linux-2.6.13-rc3/fs/jfs/namei.c linux-1/fs/jfs/namei.c
--- linux-2.6.13-rc3/fs/jfs/namei.c	2005-07-13 10:03:40.000000000 -0500
+++ linux-1/fs/jfs/namei.c	2005-07-14 09:41:21.000000000 -0500
@@ -39,6 +39,24 @@ struct dentry_operations jfs_ci_dentry_o
 static s64 commitZeroLink(tid_t, struct inode *);
 
 /*
+ * NAME:	free_ea_wmap(inode)
+ *
+ * FUNCTION:	free uncommitted extended attributes from working map 
+ *
+ */
+static inline void free_ea_wmap(struct inode *inode)
+{
+	dxd_t *ea = &JFS_IP(inode)->ea;
+
+	if (ea->flag & DXD_EXTENT) {
+		/* free EA pages from cache */
+		invalidate_dxd_metapages(inode, *ea);
+		dbFree(inode, addressDXD(ea), lengthDXD(ea));
+	}
+	ea->flag = 0;
+}
+
+/*
  * NAME:	jfs_create(dip, dentry, mode)
  *
  * FUNCTION:	create a regular file in the parent directory <dip>
@@ -89,8 +107,13 @@ static int jfs_create(struct inode *dip,
 	down(&JFS_IP(dip)->commit_sem);
 	down(&JFS_IP(ip)->commit_sem);
 
+	rc = jfs_init_acl(tid, ip, dip);
+	if (rc)
+		goto out3;
+
 	if ((rc = dtSearch(dip, &dname, &ino, &btstack, JFS_CREATE))) {
 		jfs_err("jfs_create: dtSearch returned %d", rc);
+		txAbort(tid, 0);
 		goto out3;
 	}
 
@@ -139,6 +162,7 @@ static int jfs_create(struct inode *dip,
 	up(&JFS_IP(dip)->commit_sem);
 	up(&JFS_IP(ip)->commit_sem);
 	if (rc) {
+		free_ea_wmap(ip);
 		ip->i_nlink = 0;
 		iput(ip);
 	} else
@@ -147,11 +171,6 @@ static int jfs_create(struct inode *dip,
       out2:
 	free_UCSname(&dname);
 
-#ifdef CONFIG_JFS_POSIX_ACL
-	if (rc == 0)
-		jfs_init_acl(ip, dip);
-#endif
-
       out1:
 
 	jfs_info("jfs_create: rc:%d", rc);
@@ -216,8 +235,13 @@ static int jfs_mkdir(struct inode *dip, 
 	down(&JFS_IP(dip)->commit_sem);
 	down(&JFS_IP(ip)->commit_sem);
 
+	rc = jfs_init_acl(tid, ip, dip);
+	if (rc)
+		goto out3;
+
 	if ((rc = dtSearch(dip, &dname, &ino, &btstack, JFS_CREATE))) {
 		jfs_err("jfs_mkdir: dtSearch returned %d", rc);
+		txAbort(tid, 0);
 		goto out3;
 	}
 
@@ -267,6 +291,7 @@ static int jfs_mkdir(struct inode *dip, 
 	up(&JFS_IP(dip)->commit_sem);
 	up(&JFS_IP(ip)->commit_sem);
 	if (rc) {
+		free_ea_wmap(ip);
 		ip->i_nlink = 0;
 		iput(ip);
 	} else
@@ -275,10 +300,6 @@ static int jfs_mkdir(struct inode *dip, 
       out2:
 	free_UCSname(&dname);
 
-#ifdef CONFIG_JFS_POSIX_ACL
-	if (rc == 0)
-		jfs_init_acl(ip, dip);
-#endif
 
       out1:
 
@@ -1000,6 +1021,7 @@ static int jfs_symlink(struct inode *dip
 	up(&JFS_IP(dip)->commit_sem);
 	up(&JFS_IP(ip)->commit_sem);
 	if (rc) {
+		free_ea_wmap(ip);
 		ip->i_nlink = 0;
 		iput(ip);
 	} else
@@ -1008,11 +1030,6 @@ static int jfs_symlink(struct inode *dip
       out2:
 	free_UCSname(&dname);
 
-#ifdef CONFIG_JFS_POSIX_ACL
-	if (rc == 0)
-		jfs_init_acl(ip, dip);
-#endif
-
       out1:
 	jfs_info("jfs_symlink: rc:%d", rc);
 	return rc;
@@ -1328,17 +1345,25 @@ static int jfs_mknod(struct inode *dir, 
 	down(&JFS_IP(dir)->commit_sem);
 	down(&JFS_IP(ip)->commit_sem);
 
-	if ((rc = dtSearch(dir, &dname, &ino, &btstack, JFS_CREATE)))
+	rc = jfs_init_acl(tid, ip, dir);
+	if (rc)
 		goto out3;
 
+	if ((rc = dtSearch(dir, &dname, &ino, &btstack, JFS_CREATE))) {
+		txAbort(tid, 0);
+		goto out3;
+	}
+
 	tblk = tid_to_tblock(tid);
 	tblk->xflag |= COMMIT_CREATE;
 	tblk->ino = ip->i_ino;
 	tblk->u.ixpxd = JFS_IP(ip)->ixpxd;
 
 	ino = ip->i_ino;
-	if ((rc = dtInsert(tid, dir, &dname, &ino, &btstack)))
+	if ((rc = dtInsert(tid, dir, &dname, &ino, &btstack))) {
+		txAbort(tid, 0);
 		goto out3;
+	}
 
 	ip->i_op = &jfs_file_inode_operations;
 	jfs_ip->dev = new_encode_dev(rdev);
@@ -1360,6 +1385,7 @@ static int jfs_mknod(struct inode *dir, 
 	up(&JFS_IP(ip)->commit_sem);
 	up(&JFS_IP(dir)->commit_sem);
 	if (rc) {
+		free_ea_wmap(ip);
 		ip->i_nlink = 0;
 		iput(ip);
 	} else
@@ -1368,11 +1394,6 @@ static int jfs_mknod(struct inode *dir, 
       out1:
 	free_UCSname(&dname);
 
-#ifdef CONFIG_JFS_POSIX_ACL
-	if (rc == 0)
-		jfs_init_acl(ip, dir);
-#endif
-
       out:
 	jfs_info("jfs_mknod: returning %d", rc);
 	return rc;
diff -urp linux-2.6.13-rc3/fs/jfs/xattr.c linux-1/fs/jfs/xattr.c
--- linux-2.6.13-rc3/fs/jfs/xattr.c	2005-07-13 14:07:30.000000000 -0500
+++ linux-1/fs/jfs/xattr.c	2005-07-13 16:59:58.000000000 -0500
@@ -633,12 +633,12 @@ static void ea_release(struct inode *ino
 	}
 }
 
-static int ea_put(struct inode *inode, struct ea_buffer *ea_buf, int new_size)
+static int ea_put(tid_t tid, struct inode *inode, struct ea_buffer *ea_buf,
+		  int new_size)
 {
 	struct jfs_inode_info *ji = JFS_IP(inode);
 	unsigned long old_blocks, new_blocks;
 	int rc = 0;
-	tid_t tid;
 
 	if (new_size == 0) {
 		ea_release(inode, ea_buf);
@@ -664,9 +664,6 @@ static int ea_put(struct inode *inode, s
 	if (rc)
 		return rc;
 
-	tid = txBegin(inode->i_sb, 0);
-	down(&ji->commit_sem);
-
 	old_blocks = new_blocks = 0;
 
 	if (ji->ea.flag & DXD_EXTENT) {
@@ -695,11 +692,8 @@ static int ea_put(struct inode *inode, s
 		DQUOT_FREE_BLOCK(inode, old_blocks);
 
 	inode->i_ctime = CURRENT_TIME;
-	rc = txCommit(tid, 1, &inode, 0);
-	txEnd(tid);
-	up(&ji->commit_sem);
 
-	return rc;
+	return 0;
 }
 
 /*
@@ -810,8 +804,8 @@ static int can_set_xattr(struct inode *i
 	return permission(inode, MAY_WRITE, NULL);
 }
 
-int __jfs_setxattr(struct inode *inode, const char *name, const void *value,
-		   size_t value_len, int flags)
+int __jfs_setxattr(tid_t tid, struct inode *inode, const char *name,
+		   const void *value, size_t value_len, int flags)
 {
 	struct jfs_ea_list *ealist;
 	struct jfs_ea *ea, *old_ea = NULL, *next_ea = NULL;
@@ -825,9 +819,6 @@ int __jfs_setxattr(struct inode *inode, 
 	int rc;
 	int length;
 
-	if ((rc = can_set_xattr(inode, name, value, value_len)))
-		return rc;
-
 	if (strncmp(name, XATTR_OS2_PREFIX, XATTR_OS2_PREFIX_LEN) == 0) {
 		os2name = kmalloc(namelen - XATTR_OS2_PREFIX_LEN + 1,
 				  GFP_KERNEL);
@@ -939,7 +930,7 @@ int __jfs_setxattr(struct inode *inode, 
 
 	ealist->size = cpu_to_le32(new_size);
 
-	rc = ea_put(inode, &ea_buf, new_size);
+	rc = ea_put(tid, inode, &ea_buf, new_size);
 
 	goto out;
       release:
@@ -955,12 +946,29 @@ int __jfs_setxattr(struct inode *inode, 
 int jfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 		 size_t value_len, int flags)
 {
+	struct inode *inode = dentry->d_inode;
+	struct jfs_inode_info *ji = JFS_IP(inode);
+	int rc;
+	tid_t tid;
+
+	if ((rc = can_set_xattr(inode, name, value, value_len)))
+		return rc;
+
 	if (value == NULL) {	/* empty EA, do not remove */
 		value = "";
 		value_len = 0;
 	}
 
-	return __jfs_setxattr(dentry->d_inode, name, value, value_len, flags);
+	tid = txBegin(inode->i_sb, 0);
+	down(&ji->commit_sem);
+	rc = __jfs_setxattr(tid, dentry->d_inode, name, value, value_len,
+			    flags);
+	if (!rc)
+		rc = txCommit(tid, 1, &inode, 0);
+	txEnd(tid);
+	up(&ji->commit_sem);
+
+	return rc;
 }
 
 static int can_get_xattr(struct inode *inode, const char *name)
@@ -1122,5 +1130,21 @@ ssize_t jfs_listxattr(struct dentry * de
 
 int jfs_removexattr(struct dentry *dentry, const char *name)
 {
-	return __jfs_setxattr(dentry->d_inode, name, NULL, 0, XATTR_REPLACE);
+	struct inode *inode = dentry->d_inode;
+	struct jfs_inode_info *ji = JFS_IP(inode);
+	int rc;
+	tid_t tid;
+
+	if ((rc = can_set_xattr(inode, name, NULL, 0)))
+		return rc;
+
+	tid = txBegin(inode->i_sb, 0);
+	down(&ji->commit_sem);
+	rc = __jfs_setxattr(tid, dentry->d_inode, name, NULL, 0, XATTR_REPLACE);
+	if (!rc)
+		rc = txCommit(tid, 1, &inode, 0);
+	txEnd(tid);
+	up(&ji->commit_sem);
+
+	return rc;
 }



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH 2/2] JFS atomic inode security labeling
  2005-07-14 16:16 ` [RFC][PATCH 0/2] JFS atomic inode security labeling Dave Kleikamp
  2005-07-14 16:19   ` [RFC][PATCH 1/2] JFS atomic xattr/acl handling Dave Kleikamp
@ 2005-07-14 16:20   ` Dave Kleikamp
  2005-07-14 16:26   ` [RFC][PATCH 0/2] " Dave Kleikamp
  2 siblings, 0 replies; 25+ messages in thread
From: Dave Kleikamp @ 2005-07-14 16:20 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Andrew Morton, Stephen Tweedie, James Morris,
	Chris Wright

Implement jfs_init_security()

diff -Nurp linux-1/fs/jfs/jfs_xattr.h linux/fs/jfs/jfs_xattr.h
--- linux-1/fs/jfs/jfs_xattr.h	2005-07-13 16:59:04.000000000 -0500
+++ linux/fs/jfs/jfs_xattr.h	2005-07-14 10:27:23.000000000 -0500
@@ -61,4 +61,14 @@ extern ssize_t jfs_getxattr(struct dentr
 extern ssize_t jfs_listxattr(struct dentry *, char *, size_t);
 extern int jfs_removexattr(struct dentry *, const char *);
 
+#ifdef CONFIG_JFS_SECURITY
+extern int jfs_init_security(tid_t, struct inode *, struct inode *);
+#else
+static inline int jfs_init_security(tid_t tid, struct inode *inode,
+				    struct inode *dir)
+{
+	return 0;
+}
+#endif
+
 #endif	/* H_JFS_XATTR */
diff -Nurp linux-1/fs/jfs/namei.c linux/fs/jfs/namei.c
--- linux-1/fs/jfs/namei.c	2005-07-14 09:41:21.000000000 -0500
+++ linux/fs/jfs/namei.c	2005-07-14 10:18:21.000000000 -0500
@@ -111,6 +111,12 @@ static int jfs_create(struct inode *dip,
 	if (rc)
 		goto out3;
 
+	rc = jfs_init_security(tid, ip, dip);
+	if (rc) {
+		txAbort(tid, 0);
+		goto out3;
+	}
+
 	if ((rc = dtSearch(dip, &dname, &ino, &btstack, JFS_CREATE))) {
 		jfs_err("jfs_create: dtSearch returned %d", rc);
 		txAbort(tid, 0);
@@ -239,6 +245,12 @@ static int jfs_mkdir(struct inode *dip, 
 	if (rc)
 		goto out3;
 
+	rc = jfs_init_security(tid, ip, dip);
+	if (rc) {
+		txAbort(tid, 0);
+		goto out3;
+	}
+
 	if ((rc = dtSearch(dip, &dname, &ino, &btstack, JFS_CREATE))) {
 		jfs_err("jfs_mkdir: dtSearch returned %d", rc);
 		txAbort(tid, 0);
@@ -906,6 +918,10 @@ static int jfs_symlink(struct inode *dip
 	down(&JFS_IP(dip)->commit_sem);
 	down(&JFS_IP(ip)->commit_sem);
 
+	rc = jfs_init_security(tid, ip, dip);
+	if (rc)
+		goto out3;
+
 	tblk = tid_to_tblock(tid);
 	tblk->xflag |= COMMIT_CREATE;
 	tblk->ino = ip->i_ino;
@@ -1349,6 +1365,12 @@ static int jfs_mknod(struct inode *dir, 
 	if (rc)
 		goto out3;
 
+	rc = jfs_init_security(tid, ip, dir);
+	if (rc) {
+		txAbort(tid, 0);
+		goto out3;
+	}
+
 	if ((rc = dtSearch(dir, &dname, &ino, &btstack, JFS_CREATE))) {
 		txAbort(tid, 0);
 		goto out3;
diff -Nurp linux-1/fs/jfs/xattr.c linux/fs/jfs/xattr.c
--- linux-1/fs/jfs/xattr.c	2005-07-13 16:59:58.000000000 -0500
+++ linux/fs/jfs/xattr.c	2005-07-14 10:54:11.000000000 -0500
@@ -21,6 +21,7 @@
 #include <linux/xattr.h>
 #include <linux/posix_acl_xattr.h>
 #include <linux/quotaops.h>
+#include <linux/security.h>
 #include "jfs_incore.h"
 #include "jfs_superblock.h"
 #include "jfs_dmap.h"
@@ -1148,3 +1149,38 @@ int jfs_removexattr(struct dentry *dentr
 
 	return rc;
 }
+
+#ifdef CONFIG_JFS_SECURITY
+int jfs_init_security(tid_t tid, struct inode *inode, struct inode *dir)
+{
+	int rc;
+	size_t len;
+	void *value;
+	char *suffix;
+	char *name;
+
+	rc = security_inode_init_security(inode, dir, &suffix, &value, &len);
+	if (rc) {
+		if (rc == -EOPNOTSUPP)
+			return 0;
+		return rc;
+	}
+	name = kmalloc(XATTR_SECURITY_PREFIX_LEN + 1 + strlen(suffix),
+		       GFP_NOFS);
+	if (!name) {
+		rc = -ENOMEM;
+		goto kmalloc_failed;
+	}
+	strcpy(name, XATTR_SECURITY_PREFIX);
+	strcpy(name + XATTR_SECURITY_PREFIX_LEN, suffix);
+
+	rc = __jfs_setxattr(tid, inode, name, value, len, 0);
+
+	kfree(name);
+kmalloc_failed:
+	kfree(suffix);
+	kfree(value);
+
+	return rc;
+}
+#endif




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH 0/2] JFS atomic inode security labeling
  2005-07-14 16:16 ` [RFC][PATCH 0/2] JFS atomic inode security labeling Dave Kleikamp
  2005-07-14 16:19   ` [RFC][PATCH 1/2] JFS atomic xattr/acl handling Dave Kleikamp
  2005-07-14 16:20   ` [RFC][PATCH 2/2] JFS atomic inode security labeling Dave Kleikamp
@ 2005-07-14 16:26   ` Dave Kleikamp
  2 siblings, 0 replies; 25+ messages in thread
From: Dave Kleikamp @ 2005-07-14 16:26 UTC (permalink / raw)
  To: Andrew Morton, Stephen Smalley
  Cc: linuxfs, Alexander Viro, Ext2-devel, Andreas Gruenbacher,
	Andreas Dilger, Stephen Tweedie, James Morris, Chris Wright

Andrew,
I'll push these to you via git after any feedback.

On Thu, 2005-07-14 at 11:16 -0500, Dave Kleikamp wrote:
> Here's the jfs implementation.  The first patch fixes the transaction
> layering in the xattr code to allow xattrs to be added to an inode as a
> part of an existing transaction, and correctly calls jfs_init_acl()
> within the same transaction that creates a file/directory/device.
> 
> The second patch actually implements jfs_init_security() similarly to
> the ext2/3 patches.
> 
> The first patch was lightly tested, and the second was compile tested
> only.
-- 
David Kleikamp
IBM Linux Technology Center


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC][PATCH] Remove security_inode_post_create/mkdir/symlink/mknod hooks
  2005-07-13 15:05 ` [RFC][PATCH 2.6.13-rc2-mm2] tmpfs: " Stephen Smalley
@ 2005-07-14 19:29   ` Stephen Smalley
  2005-07-14 19:41     ` Chris Wright
  0 siblings, 1 reply; 25+ messages in thread
From: Stephen Smalley @ 2005-07-14 19:29 UTC (permalink / raw)
  To: linuxfs; +Cc: Andrew Morton, Alexander Viro, lsm, Christoph Hellwig,
	Chris Wright

This patch removes the inode_post_create/mkdir/mknod/symlink LSM hooks
as they are obsoleted by the new inode_init_security hook that enables
atomic inode security labeling.  If anyone sees any reason to retain these hooks,
please speak now.  Also, is anyone using the post_rename/link hooks; if not,
those could also be removed.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---

 fs/namei.c                        |   16 +----
 include/linux/security.h          |   90 ------------------------------
 security/dummy.c                  |   28 ---------
 security/selinux/hooks.c          |  111 --------------------------------------
 security/selinux/include/objsec.h |    1 
 6 files changed, 4 insertions(+), 242 deletions(-)

diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/fs/namei.c linux-2.6.13-rc2-mm2-killpost/fs/namei.c
--- linux-2.6.13-rc2-mm2/fs/namei.c	2005-07-14 11:26:59.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost/fs/namei.c	2005-07-14 10:53:01.000000000 -0400
@@ -1311,10 +1311,8 @@ int vfs_create(struct inode *dir, struct
 		return error;
 	DQUOT_INIT(dir);
 	error = dir->i_op->create(dir, dentry, mode, nd);
-	if (!error) {
+	if (!error)
 		fsnotify_create(dir, dentry->d_name.name);
-		security_inode_post_create(dir, dentry, mode);
-	}
 	return error;
 }
 
@@ -1636,10 +1634,8 @@ int vfs_mknod(struct inode *dir, struct 
 
 	DQUOT_INIT(dir);
 	error = dir->i_op->mknod(dir, dentry, mode, dev);
-	if (!error) {
+	if (!error)
 		fsnotify_create(dir, dentry->d_name.name);
-		security_inode_post_mknod(dir, dentry, mode, dev);
-	}
 	return error;
 }
 
@@ -1709,10 +1705,8 @@ int vfs_mkdir(struct inode *dir, struct 
 
 	DQUOT_INIT(dir);
 	error = dir->i_op->mkdir(dir, dentry, mode);
-	if (!error) {
+	if (!error)
 		fsnotify_mkdir(dir, dentry->d_name.name);
-		security_inode_post_mkdir(dir,dentry, mode);
-	}
 	return error;
 }
 
@@ -1950,10 +1944,8 @@ int vfs_symlink(struct inode *dir, struc
 
 	DQUOT_INIT(dir);
 	error = dir->i_op->symlink(dir, dentry, oldname);
-	if (!error) {
+	if (!error)
 		fsnotify_create(dir, dentry->d_name.name);
-		security_inode_post_symlink(dir, dentry, oldname);
-	}
 	return error;
 }
 
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/include/linux/security.h linux-2.6.13-rc2-mm2-killpost/include/linux/security.h
--- linux-2.6.13-rc2-mm2/include/linux/security.h	2005-07-14 11:27:05.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost/include/linux/security.h	2005-07-14 10:53:01.000000000 -0400
@@ -275,12 +275,6 @@ struct swap_info_struct;
  *	@dentry contains the dentry structure for the file to be created.
  *	@mode contains the file mode of the file to be created.
  *	Return 0 if permission is granted.
- * @inode_post_create:
- *	Set the security attributes on a newly created regular file.  This hook
- *	is called after a file has been successfully created.
- *	@dir contains the inode structure of the parent directory of the new file.
- *	@dentry contains the the dentry structure for the newly created file.
- *	@mode contains the file mode.
  * @inode_link:
  *	Check permission before creating a new hard link to a file.
  *	@old_dentry contains the dentry structure for an existing link to the file.
@@ -303,13 +297,6 @@ struct swap_info_struct;
  *	@dentry contains the dentry structure of the symbolic link.
  *	@old_name contains the pathname of file.
  *	Return 0 if permission is granted.
- * @inode_post_symlink:
- *	@dir contains the inode structure of the parent directory of the new link.
- *	@dentry contains the dentry structure of new symbolic link.
- *	@old_name contains the pathname of file.
- *	Set security attributes for a newly created symbolic link.  Note that
- *	@dentry->d_inode may be NULL, since the filesystem might not
- *	instantiate the dentry (e.g. NFS).
  * @inode_mkdir:
  *	Check permissions to create a new directory in the existing directory
  *	associated with inode strcture @dir. 
@@ -317,11 +304,6 @@ struct swap_info_struct;
  *	@dentry contains the dentry structure of new directory.
  *	@mode contains the mode of new directory.
  *	Return 0 if permission is granted.
- * @inode_post_mkdir:
- *	Set security attributes on a newly created directory.
- *	@dir contains the inode structure of parent of the directory to be created.
- *	@dentry contains the dentry structure of new directory.
- *	@mode contains the mode of new directory.
  * @inode_rmdir:
  *	Check the permission to remove a directory.
  *	@dir contains the inode structure of parent of the directory to be removed.
@@ -337,13 +319,6 @@ struct swap_info_struct;
  *	@mode contains the mode of the new file.
  *	@dev contains the the device number.
  *	Return 0 if permission is granted.
- * @inode_post_mknod:
- *	Set security attributes on a newly created special file (or socket or
- *	fifo file created via the mknod system call).
- *	@dir contains the inode structure of parent of the new node.
- *	@dentry contains the dentry structure of the new node.
- *	@mode contains the mode of the new node.
- *	@dev contains the the device number.
  * @inode_rename:
  *	Check for permission to rename a file or directory.
  *	@old_dir contains the inode structure for parent of the old link.
@@ -1103,8 +1078,6 @@ struct security_operations {
 				    char **name, void **value, size_t *len);
 	int (*inode_create) (struct inode *dir,
 	                     struct dentry *dentry, int mode);
-	void (*inode_post_create) (struct inode *dir,
-	                           struct dentry *dentry, int mode);
 	int (*inode_link) (struct dentry *old_dentry,
 	                   struct inode *dir, struct dentry *new_dentry);
 	void (*inode_post_link) (struct dentry *old_dentry,
@@ -1112,17 +1085,10 @@ struct security_operations {
 	int (*inode_unlink) (struct inode *dir, struct dentry *dentry);
 	int (*inode_symlink) (struct inode *dir,
 	                      struct dentry *dentry, const char *old_name);
-	void (*inode_post_symlink) (struct inode *dir,
-	                            struct dentry *dentry,
-	                            const char *old_name);
 	int (*inode_mkdir) (struct inode *dir, struct dentry *dentry, int mode);
-	void (*inode_post_mkdir) (struct inode *dir, struct dentry *dentry, 
-			    int mode);
 	int (*inode_rmdir) (struct inode *dir, struct dentry *dentry);
 	int (*inode_mknod) (struct inode *dir, struct dentry *dentry,
 	                    int mode, dev_t dev);
-	void (*inode_post_mknod) (struct inode *dir, struct dentry *dentry,
-	                          int mode, dev_t dev);
 	int (*inode_rename) (struct inode *old_dir, struct dentry *old_dentry,
 	                     struct inode *new_dir, struct dentry *new_dentry);
 	void (*inode_post_rename) (struct inode *old_dir,
@@ -1484,15 +1450,6 @@ static inline int security_inode_create 
 	return security_ops->inode_create (dir, dentry, mode);
 }
 
-static inline void security_inode_post_create (struct inode *dir,
-					       struct dentry *dentry,
-					       int mode)
-{
-	if (dentry->d_inode && unlikely (IS_PRIVATE (dentry->d_inode)))
-		return;
-	security_ops->inode_post_create (dir, dentry, mode);
-}
-
 static inline int security_inode_link (struct dentry *old_dentry,
 				       struct inode *dir,
 				       struct dentry *new_dentry)
@@ -1528,15 +1485,6 @@ static inline int security_inode_symlink
 	return security_ops->inode_symlink (dir, dentry, old_name);
 }
 
-static inline void security_inode_post_symlink (struct inode *dir,
-						struct dentry *dentry,
-						const char *old_name)
-{
-	if (dentry->d_inode && unlikely (IS_PRIVATE (dentry->d_inode)))
-		return;
-	security_ops->inode_post_symlink (dir, dentry, old_name);
-}
-
 static inline int security_inode_mkdir (struct inode *dir,
 					struct dentry *dentry,
 					int mode)
@@ -1546,15 +1494,6 @@ static inline int security_inode_mkdir (
 	return security_ops->inode_mkdir (dir, dentry, mode);
 }
 
-static inline void security_inode_post_mkdir (struct inode *dir,
-					      struct dentry *dentry,
-					      int mode)
-{
-	if (dentry->d_inode && unlikely (IS_PRIVATE (dentry->d_inode)))
-		return;
-	security_ops->inode_post_mkdir (dir, dentry, mode);
-}
-
 static inline int security_inode_rmdir (struct inode *dir,
 					struct dentry *dentry)
 {
@@ -1572,15 +1511,6 @@ static inline int security_inode_mknod (
 	return security_ops->inode_mknod (dir, dentry, mode, dev);
 }
 
-static inline void security_inode_post_mknod (struct inode *dir,
-					      struct dentry *dentry,
-					      int mode, dev_t dev)
-{
-	if (dentry->d_inode && unlikely (IS_PRIVATE (dentry->d_inode)))
-		return;
-	security_ops->inode_post_mknod (dir, dentry, mode, dev);
-}
-
 static inline int security_inode_rename (struct inode *old_dir,
 					 struct dentry *old_dentry,
 					 struct inode *new_dir,
@@ -2225,11 +2155,6 @@ static inline int security_inode_create 
 	return 0;
 }
 
-static inline void security_inode_post_create (struct inode *dir,
-					       struct dentry *dentry,
-					       int mode)
-{ }
-
 static inline int security_inode_link (struct dentry *old_dentry,
 				       struct inode *dir,
 				       struct dentry *new_dentry)
@@ -2255,11 +2180,6 @@ static inline int security_inode_symlink
 	return 0;
 }
 
-static inline void security_inode_post_symlink (struct inode *dir,
-						struct dentry *dentry,
-						const char *old_name)
-{ }
-
 static inline int security_inode_mkdir (struct inode *dir,
 					struct dentry *dentry,
 					int mode)
@@ -2267,11 +2187,6 @@ static inline int security_inode_mkdir (
 	return 0;
 }
 
-static inline void security_inode_post_mkdir (struct inode *dir,
-					      struct dentry *dentry,
-					      int mode)
-{ }
-
 static inline int security_inode_rmdir (struct inode *dir,
 					struct dentry *dentry)
 {
@@ -2285,11 +2200,6 @@ static inline int security_inode_mknod (
 	return 0;
 }
 
-static inline void security_inode_post_mknod (struct inode *dir,
-					      struct dentry *dentry,
-					      int mode, dev_t dev)
-{ }
-
 static inline int security_inode_rename (struct inode *old_dir,
 					 struct dentry *old_dentry,
 					 struct inode *new_dir,
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/security/dummy.c linux-2.6.13-rc2-mm2-killpost/security/dummy.c
--- linux-2.6.13-rc2-mm2/security/dummy.c	2005-07-14 11:27:06.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost/security/dummy.c	2005-07-14 10:53:01.000000000 -0400
@@ -270,12 +270,6 @@ static int dummy_inode_create (struct in
 	return 0;
 }
 
-static void dummy_inode_post_create (struct inode *inode, struct dentry *dentry,
-				     int mask)
-{
-	return;
-}
-
 static int dummy_inode_link (struct dentry *old_dentry, struct inode *inode,
 			     struct dentry *new_dentry)
 {
@@ -300,24 +294,12 @@ static int dummy_inode_symlink (struct i
 	return 0;
 }
 
-static void dummy_inode_post_symlink (struct inode *inode,
-				      struct dentry *dentry, const char *name)
-{
-	return;
-}
-
 static int dummy_inode_mkdir (struct inode *inode, struct dentry *dentry,
 			      int mask)
 {
 	return 0;
 }
 
-static void dummy_inode_post_mkdir (struct inode *inode, struct dentry *dentry,
-				    int mask)
-{
-	return;
-}
-
 static int dummy_inode_rmdir (struct inode *inode, struct dentry *dentry)
 {
 	return 0;
@@ -329,12 +311,6 @@ static int dummy_inode_mknod (struct ino
 	return 0;
 }
 
-static void dummy_inode_post_mknod (struct inode *inode, struct dentry *dentry,
-				    int mode, dev_t dev)
-{
-	return;
-}
-
 static int dummy_inode_rename (struct inode *old_inode,
 			       struct dentry *old_dentry,
 			       struct inode *new_inode,
@@ -894,17 +870,13 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, inode_free_security);
 	set_to_dummy_if_null(ops, inode_init_security);
 	set_to_dummy_if_null(ops, inode_create);
-	set_to_dummy_if_null(ops, inode_post_create);
 	set_to_dummy_if_null(ops, inode_link);
 	set_to_dummy_if_null(ops, inode_post_link);
 	set_to_dummy_if_null(ops, inode_unlink);
 	set_to_dummy_if_null(ops, inode_symlink);
-	set_to_dummy_if_null(ops, inode_post_symlink);
 	set_to_dummy_if_null(ops, inode_mkdir);
-	set_to_dummy_if_null(ops, inode_post_mkdir);
 	set_to_dummy_if_null(ops, inode_rmdir);
 	set_to_dummy_if_null(ops, inode_mknod);
-	set_to_dummy_if_null(ops, inode_post_mknod);
 	set_to_dummy_if_null(ops, inode_rename);
 	set_to_dummy_if_null(ops, inode_post_rename);
 	set_to_dummy_if_null(ops, inode_readlink);
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/security/selinux/hooks.c linux-2.6.13-rc2-mm2-killpost/security/selinux/hooks.c
--- linux-2.6.13-rc2-mm2/security/selinux/hooks.c	2005-07-14 11:46:30.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost/security/selinux/hooks.c	2005-07-14 11:02:55.000000000 -0400
@@ -1264,91 +1264,6 @@ static int inode_security_set_sid(struct
 	return 0;
 }
 
-/* Set the security attributes on a newly created file. */
-static int post_create(struct inode *dir,
-		       struct dentry *dentry)
-{
-
-	struct task_security_struct *tsec;
-	struct inode *inode;
-	struct inode_security_struct *dsec;
-	struct superblock_security_struct *sbsec;
-	struct inode_security_struct *isec;
-	u32 newsid;
-	char *context;
-	unsigned int len;
-	int rc;
-
-	tsec = current->security;
-	dsec = dir->i_security;
-	sbsec = dir->i_sb->s_security;
-
-	inode = dentry->d_inode;
-	if (!inode) {
-		/* Some file system types (e.g. NFS) may not instantiate
-		   a dentry for all create operations (e.g. symlink),
-		   so we have to check to see if the inode is non-NULL. */
-		printk(KERN_WARNING "post_create:  no inode, dir (dev=%s, "
-		       "ino=%ld)\n", dir->i_sb->s_id, dir->i_ino);
-		return 0;
-	}
-
-	isec = inode->i_security;
-
-	if (isec->security_attr_init)
-		return 0;
-
-	if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
-		newsid = tsec->create_sid;
-	} else {
-		rc = security_transition_sid(tsec->sid, dsec->sid,
-					     inode_mode_to_security_class(inode->i_mode),
-					     &newsid);
-		if (rc) {
-			printk(KERN_WARNING "post_create:  "
-			       "security_transition_sid failed, rc=%d (dev=%s "
-			       "ino=%ld)\n",
-			       -rc, inode->i_sb->s_id, inode->i_ino);
-			return rc;
-		}
-	}
-
-	rc = inode_security_set_sid(inode, newsid);
-	if (rc) {
-		printk(KERN_WARNING "post_create:  inode_security_set_sid "
-		       "failed, rc=%d (dev=%s ino=%ld)\n",
-		       -rc, inode->i_sb->s_id, inode->i_ino);
-		return rc;
-	}
-
-	if (sbsec->behavior == SECURITY_FS_USE_XATTR &&
-	    inode->i_op->setxattr) {
-		/* Use extended attributes. */
-		rc = security_sid_to_context(newsid, &context, &len);
-		if (rc) {
-			printk(KERN_WARNING "post_create:  sid_to_context "
-			       "failed, rc=%d (dev=%s ino=%ld)\n",
-			       -rc, inode->i_sb->s_id, inode->i_ino);
-			return rc;
-		}
-		down(&inode->i_sem);
-		rc = inode->i_op->setxattr(dentry,
-					   XATTR_NAME_SELINUX,
-					   context, len, 0);
-		up(&inode->i_sem);
-		kfree(context);
-		if (rc < 0) {
-			printk(KERN_WARNING "post_create:  setxattr failed, "
-			       "rc=%d (dev=%s ino=%ld)\n",
-			       -rc, inode->i_sb->s_id, inode->i_ino);
-			return rc;
-		}
-	}
-
-	return 0;
-}
-
-
 /* Hook functions begin here. */
 
 static int selinux_ptrace(struct task_struct *parent, struct task_struct *child)
@@ -2076,8 +1991,6 @@ static int selinux_inode_init_security(s
 		*value = context;
 	}
 
-	isec->security_attr_init = 1;
-
 	return 0;
 }
 
@@ -2086,11 +1999,6 @@ static int selinux_inode_create(struct i
 	return may_create(dir, dentry, SECCLASS_FILE);
 }
 
-static void selinux_inode_post_create(struct inode *dir, struct dentry *dentry, int mask)
-{
-	post_create(dir, dentry);
-}
-
 static int selinux_inode_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
 {
 	int rc;
@@ -2121,21 +2029,11 @@ static int selinux_inode_symlink(struct 
 	return may_create(dir, dentry, SECCLASS_LNK_FILE);
 }
 
-static void selinux_inode_post_symlink(struct inode *dir, struct dentry *dentry, const char *name)
-{
-	post_create(dir, dentry);
-}
-
 static int selinux_inode_mkdir(struct inode *dir, struct dentry *dentry, int mask)
 {
 	return may_create(dir, dentry, SECCLASS_DIR);
 }
 
-static void selinux_inode_post_mkdir(struct inode *dir, struct dentry *dentry, int mask)
-{
-	post_create(dir, dentry);
-}
-
 static int selinux_inode_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	return may_link(dir, dentry, MAY_RMDIR);
@@ -2152,11 +2050,6 @@ static int selinux_inode_mknod(struct in
 	return may_create(dir, dentry, inode_mode_to_security_class(mode));
 }
 
-static void selinux_inode_post_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
-{
-	post_create(dir, dentry);
-}
-
 static int selinux_inode_rename(struct inode *old_inode, struct dentry *old_dentry,
                                 struct inode *new_inode, struct dentry *new_dentry)
 {
@@ -4363,17 +4256,13 @@ static struct security_operations selinu
 	.inode_free_security =		selinux_inode_free_security,
 	.inode_init_security =		selinux_inode_init_security,
 	.inode_create =			selinux_inode_create,
-	.inode_post_create =		selinux_inode_post_create,
 	.inode_link =			selinux_inode_link,
 	.inode_post_link =		selinux_inode_post_link,
 	.inode_unlink =			selinux_inode_unlink,
 	.inode_symlink =		selinux_inode_symlink,
-	.inode_post_symlink =		selinux_inode_post_symlink,
 	.inode_mkdir =			selinux_inode_mkdir,
-	.inode_post_mkdir =		selinux_inode_post_mkdir,
 	.inode_rmdir =			selinux_inode_rmdir,
 	.inode_mknod =			selinux_inode_mknod,
-	.inode_post_mknod =		selinux_inode_post_mknod,
 	.inode_rename =			selinux_inode_rename,
 	.inode_post_rename =		selinux_inode_post_rename,
 	.inode_readlink =		selinux_inode_readlink,
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2/security/selinux/include/objsec.h linux-2.6.13-rc2-mm2-killpost/security/selinux/include/objsec.h
--- linux-2.6.13-rc2-mm2/security/selinux/include/objsec.h	2005-07-14 11:27:06.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost/security/selinux/include/objsec.h	2005-07-14 10:53:01.000000000 -0400
@@ -46,7 +46,6 @@ struct inode_security_struct {
 	unsigned char initialized;     /* initialization flag */
 	struct semaphore sem;
 	unsigned char inherit;         /* inherit SID from parent entry */
-	unsigned char security_attr_init; /* security attributes init flag */
 };
 
 struct file_security_struct {

-- 
Stephen Smalley
National Security Agency

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] Remove security_inode_post_create/mkdir/symlink/mknod hooks
  2005-07-14 19:29   ` [RFC][PATCH] Remove security_inode_post_create/mkdir/symlink/mknod hooks Stephen Smalley
@ 2005-07-14 19:41     ` Chris Wright
  2005-07-14 20:51       ` Stephen Smalley
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Wright @ 2005-07-14 19:41 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Andrew Morton, Alexander Viro, lsm, Christoph Hellwig,
	Chris Wright, linuxfs

* Stephen Smalley (sds@tycho.nsa.gov) wrote:
> This patch removes the inode_post_create/mkdir/mknod/symlink LSM hooks
> as they are obsoleted by the new inode_init_security hook that enables
> atomic inode security labeling.  If anyone sees any reason to retain these hooks,
> please speak now.  Also, is anyone using the post_rename/link hooks; if not,
> those could also be removed.

Please remove post_rename.  The dentry args are garbage anyway.

thanks,
-chris

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] Remove security_inode_post_create/mkdir/symlink/mknod hooks
  2005-07-14 19:41     ` Chris Wright
@ 2005-07-14 20:51       ` Stephen Smalley
  0 siblings, 0 replies; 25+ messages in thread
From: Stephen Smalley @ 2005-07-14 20:51 UTC (permalink / raw)
  To: Chris Wright
  Cc: Andrew Morton, Alexander Viro, linuxfs, lsm, Christoph Hellwig

On Thu, 2005-07-14 at 12:41 -0700, Chris Wright wrote:
> * Stephen Smalley (sds@tycho.nsa.gov) wrote:
> > This patch removes the inode_post_create/mkdir/mknod/symlink LSM hooks
> > as they are obsoleted by the new inode_init_security hook that enables
> > atomic inode security labeling.  If anyone sees any reason to retain these hooks,
> > please speak now.  Also, is anyone using the post_rename/link hooks; if not,
> > those could also be removed.
> 
> Please remove post_rename.  The dentry args are garbage anyway.

This patch removes the inode_post_link and inode_post_rename LSM hooks
as they are unused (and likely useless). 

Signed-off-by:  Stephen Smalley <sds@tycho.nsa.gov>
---

 fs/namei.c               |   10 +--------
 include/linux/security.h |   49 -----------------------------------------------
 security/dummy.c         |   17 ----------------
 security/selinux/hooks.c |   13 ------------
 5 files changed, 2 insertions(+), 87 deletions(-)

diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2-killpost/fs/namei.c linux-2.6.13-rc2-mm2-killpost2/fs/namei.c
--- linux-2.6.13-rc2-mm2-killpost/fs/namei.c	2005-07-14 10:53:01.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost2/fs/namei.c	2005-07-14 16:21:49.000000000 -0400
@@ -2015,10 +2015,8 @@ int vfs_link(struct dentry *old_dentry, 
 	DQUOT_INIT(dir);
 	error = dir->i_op->link(old_dentry, dir, new_dentry);
 	up(&old_dentry->d_inode->i_sem);
-	if (!error) {
+	if (!error)
 		fsnotify_create(dir, new_dentry->d_name.name);
-		security_inode_post_link(old_dentry, dir, new_dentry);
-	}
 	return error;
 }
 
@@ -2137,11 +2135,8 @@ static int vfs_rename_dir(struct inode *
 			d_rehash(new_dentry);
 		dput(new_dentry);
 	}
-	if (!error) {
+	if (!error)
 		d_move(old_dentry,new_dentry);
-		security_inode_post_rename(old_dir, old_dentry,
-					   new_dir, new_dentry);
-	}
 	return error;
 }
 
@@ -2167,7 +2162,6 @@ static int vfs_rename_other(struct inode
 		/* The following d_move() should become unconditional */
 		if (!(old_dir->i_sb->s_type->fs_flags & FS_ODD_RENAME))
 			d_move(old_dentry, new_dentry);
-		security_inode_post_rename(old_dir, old_dentry, new_dir, new_dentry);
 	}
 	if (target)
 		up(&target->i_sem);
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2-killpost/include/linux/security.h linux-2.6.13-rc2-mm2-killpost2/include/linux/security.h
--- linux-2.6.13-rc2-mm2-killpost/include/linux/security.h	2005-07-14 10:53:01.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost2/include/linux/security.h	2005-07-14 16:36:07.000000000 -0400
@@ -281,11 +281,6 @@ struct swap_info_struct;
  *	@dir contains the inode structure of the parent directory of the new link.
  *	@new_dentry contains the dentry structure for the new link.
  *	Return 0 if permission is granted.
- * @inode_post_link:
- *	Set security attributes for a new hard link to a file.
- *	@old_dentry contains the dentry structure for the existing link.
- *	@dir contains the inode structure of the parent directory of the new file.
- *	@new_dentry contains the dentry structure for the new file link.
  * @inode_unlink:
  *	Check the permission to remove a hard link to a file. 
  *	@dir contains the inode structure of parent directory of the file.
@@ -326,12 +321,6 @@ struct swap_info_struct;
  *	@new_dir contains the inode structure for parent of the new link.
  *	@new_dentry contains the dentry structure of the new link.
  *	Return 0 if permission is granted.
- * @inode_post_rename:
- *	Set security attributes on a renamed file or directory.
- *	@old_dir contains the inode structure for parent of the old link.
- *	@old_dentry contains the dentry structure of the old link.
- *	@new_dir contains the inode structure for parent of the new link.
- *	@new_dentry contains the dentry structure of the new link.
  * @inode_readlink:
  *	Check the permission to read the symbolic link.
  *	@dentry contains the dentry structure for the file link.
@@ -1080,8 +1069,6 @@ struct security_operations {
 	                     struct dentry *dentry, int mode);
 	int (*inode_link) (struct dentry *old_dentry,
 	                   struct inode *dir, struct dentry *new_dentry);
-	void (*inode_post_link) (struct dentry *old_dentry,
-	                         struct inode *dir, struct dentry *new_dentry);
 	int (*inode_unlink) (struct inode *dir, struct dentry *dentry);
 	int (*inode_symlink) (struct inode *dir,
 	                      struct dentry *dentry, const char *old_name);
@@ -1091,10 +1078,6 @@ struct security_operations {
 	                    int mode, dev_t dev);
 	int (*inode_rename) (struct inode *old_dir, struct dentry *old_dentry,
 	                     struct inode *new_dir, struct dentry *new_dentry);
-	void (*inode_post_rename) (struct inode *old_dir,
-	                           struct dentry *old_dentry,
-	                           struct inode *new_dir,
-	                           struct dentry *new_dentry);
 	int (*inode_readlink) (struct dentry *dentry);
 	int (*inode_follow_link) (struct dentry *dentry, struct nameidata *nd);
 	int (*inode_permission) (struct inode *inode, int mask, struct nameidata *nd);
@@ -1459,15 +1442,6 @@ static inline int security_inode_link (s
 	return security_ops->inode_link (old_dentry, dir, new_dentry);
 }
 
-static inline void security_inode_post_link (struct dentry *old_dentry,
-					     struct inode *dir,
-					     struct dentry *new_dentry)
-{
-	if (new_dentry->d_inode && unlikely (IS_PRIVATE (new_dentry->d_inode)))
-		return;
-	security_ops->inode_post_link (old_dentry, dir, new_dentry);
-}
-
 static inline int security_inode_unlink (struct inode *dir,
 					 struct dentry *dentry)
 {
@@ -1523,18 +1497,6 @@ static inline int security_inode_rename 
 					   new_dir, new_dentry);
 }
 
-static inline void security_inode_post_rename (struct inode *old_dir,
-					       struct dentry *old_dentry,
-					       struct inode *new_dir,
-					       struct dentry *new_dentry)
-{
-	if (unlikely (IS_PRIVATE (old_dentry->d_inode) ||
-	    (new_dentry->d_inode && IS_PRIVATE (new_dentry->d_inode))))
-		return;
-	security_ops->inode_post_rename (old_dir, old_dentry,
-						new_dir, new_dentry);
-}
-
 static inline int security_inode_readlink (struct dentry *dentry)
 {
 	if (unlikely (IS_PRIVATE (dentry->d_inode)))
@@ -2162,11 +2124,6 @@ static inline int security_inode_link (s
 	return 0;
 }
 
-static inline void security_inode_post_link (struct dentry *old_dentry,
-					     struct inode *dir,
-					     struct dentry *new_dentry)
-{ }
-
 static inline int security_inode_unlink (struct inode *dir,
 					 struct dentry *dentry)
 {
@@ -2208,12 +2165,6 @@ static inline int security_inode_rename 
 	return 0;
 }
 
-static inline void security_inode_post_rename (struct inode *old_dir,
-					       struct dentry *old_dentry,
-					       struct inode *new_dir,
-					       struct dentry *new_dentry)
-{ }
-
 static inline int security_inode_readlink (struct dentry *dentry)
 {
 	return 0;
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2-killpost/security/dummy.c linux-2.6.13-rc2-mm2-killpost2/security/dummy.c
--- linux-2.6.13-rc2-mm2-killpost/security/dummy.c	2005-07-14 10:53:01.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost2/security/dummy.c	2005-07-14 16:36:28.000000000 -0400
@@ -276,13 +276,6 @@ static int dummy_inode_link (struct dent
 	return 0;
 }
 
-static void dummy_inode_post_link (struct dentry *old_dentry,
-				   struct inode *inode,
-				   struct dentry *new_dentry)
-{
-	return;
-}
-
 static int dummy_inode_unlink (struct inode *inode, struct dentry *dentry)
 {
 	return 0;
@@ -319,14 +312,6 @@ static int dummy_inode_rename (struct in
 	return 0;
 }
 
-static void dummy_inode_post_rename (struct inode *old_inode,
-				     struct dentry *old_dentry,
-				     struct inode *new_inode,
-				     struct dentry *new_dentry)
-{
-	return;
-}
-
 static int dummy_inode_readlink (struct dentry *dentry)
 {
 	return 0;
@@ -871,14 +856,12 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, inode_init_security);
 	set_to_dummy_if_null(ops, inode_create);
 	set_to_dummy_if_null(ops, inode_link);
-	set_to_dummy_if_null(ops, inode_post_link);
 	set_to_dummy_if_null(ops, inode_unlink);
 	set_to_dummy_if_null(ops, inode_symlink);
 	set_to_dummy_if_null(ops, inode_mkdir);
 	set_to_dummy_if_null(ops, inode_rmdir);
 	set_to_dummy_if_null(ops, inode_mknod);
 	set_to_dummy_if_null(ops, inode_rename);
-	set_to_dummy_if_null(ops, inode_post_rename);
 	set_to_dummy_if_null(ops, inode_readlink);
 	set_to_dummy_if_null(ops, inode_follow_link);
 	set_to_dummy_if_null(ops, inode_permission);
diff -X /home/sds/dontdiff -rup linux-2.6.13-rc2-mm2-killpost/security/selinux/hooks.c linux-2.6.13-rc2-mm2-killpost2/security/selinux/hooks.c
--- linux-2.6.13-rc2-mm2-killpost/security/selinux/hooks.c	2005-07-14 11:02:55.000000000 -0400
+++ linux-2.6.13-rc2-mm2-killpost2/security/selinux/hooks.c	2005-07-14 16:21:26.000000000 -0400
@@ -2009,11 +2009,6 @@ static int selinux_inode_link(struct den
 	return may_link(dir, old_dentry, MAY_LINK);
 }
 
-static void selinux_inode_post_link(struct dentry *old_dentry, struct inode *inode, struct dentry *new_dentry)
-{
-	return;
-}
-
 static int selinux_inode_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int rc;
@@ -2056,12 +2051,6 @@ static int selinux_inode_rename(struct i
 	return may_rename(old_inode, old_dentry, new_inode, new_dentry);
 }
 
-static void selinux_inode_post_rename(struct inode *old_inode, struct dentry *old_dentry,
-                                      struct inode *new_inode, struct dentry *new_dentry)
-{
-	return;
-}
-
 static int selinux_inode_readlink(struct dentry *dentry)
 {
 	return dentry_has_perm(current, NULL, dentry, FILE__READ);
@@ -4257,14 +4246,12 @@ static struct security_operations selinu
 	.inode_init_security =		selinux_inode_init_security,
 	.inode_create =			selinux_inode_create,
 	.inode_link =			selinux_inode_link,
-	.inode_post_link =		selinux_inode_post_link,
 	.inode_unlink =			selinux_inode_unlink,
 	.inode_symlink =		selinux_inode_symlink,
 	.inode_mkdir =			selinux_inode_mkdir,
 	.inode_rmdir =			selinux_inode_rmdir,
 	.inode_mknod =			selinux_inode_mknod,
 	.inode_rename =			selinux_inode_rename,
-	.inode_post_rename =		selinux_inode_post_rename,
 	.inode_readlink =		selinux_inode_readlink,
 	.inode_follow_link =		selinux_inode_follow_link,
 	.inode_permission =		selinux_inode_permission,

-- 
Stephen Smalley
National Security Agency

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2005-07-14 20:51 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-08 13:25 [RFC][PATCH 0/3] Enable atomic inode security labeling Stephen Smalley
2005-07-08 13:48 ` [RFC][PATCH 1/3] security: " Stephen Smalley
2005-07-08 13:55 ` [RFC][PATCH 2/3] ext2: " Stephen Smalley
2005-07-10 23:39   ` Christoph Hellwig
2005-07-11 12:53     ` Stephen Smalley
2005-07-12  2:29       ` Christoph Hellwig
2005-07-13 20:37   ` Dave Kleikamp
2005-07-13 20:41     ` Stephen Smalley
2005-07-13 20:50     ` Andrew Morton
2005-07-08 13:58 ` [RFC][PATCH 3/3] ext3: " Stephen Smalley
2005-07-11 16:07   ` Stephen C. Tweedie
2005-07-11 16:14     ` Jan Kara
2005-07-11 16:50       ` Stephen C. Tweedie
2005-07-12 14:15         ` [Ext2-devel] " Jan Kara
2005-07-10 23:40 ` [RFC][PATCH 0/3] " Christoph Hellwig
2005-07-11 13:31   ` Stephen Smalley
2005-07-12  2:32     ` Christoph Hellwig
2005-07-13 15:05 ` [RFC][PATCH 2.6.13-rc2-mm2] tmpfs: " Stephen Smalley
2005-07-14 19:29   ` [RFC][PATCH] Remove security_inode_post_create/mkdir/symlink/mknod hooks Stephen Smalley
2005-07-14 19:41     ` Chris Wright
2005-07-14 20:51       ` Stephen Smalley
2005-07-14 16:16 ` [RFC][PATCH 0/2] JFS atomic inode security labeling Dave Kleikamp
2005-07-14 16:19   ` [RFC][PATCH 1/2] JFS atomic xattr/acl handling Dave Kleikamp
2005-07-14 16:20   ` [RFC][PATCH 2/2] JFS atomic inode security labeling Dave Kleikamp
2005-07-14 16:26   ` [RFC][PATCH 0/2] " Dave Kleikamp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).