integrity: fs hook placement

RFC PATCH In review Mimi Zohar 5 participants 10 messages Mar 8, 2007 VFS

Share & tools

Mimi Zohar

author

2007-03-08 15:51 UTC #0 lore raw

RFC PATCH 2/6 integrity: fs hook placement

This patch places calls to the new integrity hooks in the appropriate places in the fs directory. It is not meant in any way to be viewed as a complete set, but used as a basis for an initial discussion.

fs/ext3/xattr_security.c +8 -0

Index: linux-2.6.21-rc3-mm2/fs/ext3/xattr_security.c
===================================================================
@@ -10,6 +10,7 @@
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include "xattr.h"
 
 static size_t

@@ -58,12 +59,19 @@ ext3_init_security(handle_t *handle, str
 
 	err = security_inode_init_security(inode, dir, &name, &value, &len);
 	if (err) {
+		/* Even if creation of the security xattr fails, must
+		 * indicate this is a new inode. */
+		integrity_inode_init_integrity(inode, dir, NULL, NULL, NULL);
 		if (err == -EOPNOTSUPP)
 			return 0;
 		return err;
 	}
 	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
 				    name, value, len, 0);
+
+	integrity_inode_init_integrity(inode, dir, &name, &value, &len);
+	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
+				    name, value, len, 0);
 	kfree(name);
 	kfree(value);
 	return err;

include/linux/fs.h +3 -0

Index: linux-2.6.21-rc3-mm2/include/linux/fs.h
===================================================================
@@ -591,6 +591,9 @@ struct inode {
 #ifdef CONFIG_SECURITY
 	void			*i_security;
 #endif
+#ifdef CONFIG_INTEGRITY
+	void			*i_integrity;
+#endif
 	void			*i_private; /* fs or device private pointer */
 };

fs/dcache.c +6 -0

Index: linux-2.6.21-rc3-mm2/fs/dcache.c
===================================================================
@@ -29,6 +29,7 @@
 #include <linux/file.h>
 #include <asm/uaccess.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/seqlock.h>
 #include <linux/swap.h>
 #include <linux/bootmem.h>

@@ -986,6 +987,7 @@ void d_instantiate(struct dentry *entry,
 	entry->d_inode = inode;
 	fsnotify_d_instantiate(entry, inode);
 	spin_unlock(&dcache_lock);
+	integrity_d_instantiate(entry, inode);
 	security_d_instantiate(entry, inode);
 }

@@ -1050,6 +1052,7 @@ struct dentry *d_instantiate_unique(stru
 	spin_unlock(&dcache_lock);
 
 	if (!result) {
+		integrity_d_instantiate(entry, inode);
 		security_d_instantiate(entry, inode);
 		return NULL;
 	}

@@ -1187,6 +1190,7 @@ struct dentry *d_splice_alias(struct ino
 			BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
 			fsnotify_d_instantiate(new, inode);
 			spin_unlock(&dcache_lock);
+			integrity_d_instantiate(new, inode);
 			security_d_instantiate(new, inode);
 			d_rehash(dentry);
 			d_move(new, dentry);

@@ -1197,6 +1201,7 @@ struct dentry *d_splice_alias(struct ino
 			dentry->d_inode = inode;
 			fsnotify_d_instantiate(dentry, inode);
 			spin_unlock(&dcache_lock);
+			integrity_d_instantiate(dentry, inode);
 			security_d_instantiate(dentry, inode);
 			d_rehash(dentry);
 		}

@@ -1748,6 +1753,7 @@ found:
 	spin_unlock(&dcache_lock);
 out_nolock:
 	if (actual == dentry) {
+		integrity_d_instantiate(dentry, inode);
 		security_d_instantiate(dentry, inode);
 		return NULL;
 	}

fs/file_table.c +3 -0

Index: linux-2.6.21-rc3-mm2/fs/file_table.c
===================================================================
@@ -13,6 +13,7 @@
 #include <linux/smp_lock.h>
 #include <linux/fs.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/eventpoll.h>
 #include <linux/rcupdate.h>
 #include <linux/mount.h>

@@ -169,6 +170,7 @@ void fastcall __fput(struct file *file)
 	if (file->f_op && file->f_op->release)
 		file->f_op->release(inode, file);
 	security_file_free(file);
+	integrity_file_free(file);
 	if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL))
 		cdev_put(inode->i_cdev);
 	fops_put(file->f_op);

@@ -240,6 +242,7 @@ void put_filp(struct file *file)
 {
 	if (atomic_dec_and_test(&file->f_count)) {
 		security_file_free(file);
+		integrity_file_free(file);
 		file_kill(file);
 		file_free(file);
 	}

fs/inode.c +10 -0

Index: linux-2.6.21-rc3-mm2/fs/inode.c
===================================================================
@@ -17,6 +17,7 @@
 #include <linux/hash.h>
 #include <linux/swap.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/pagemap.h>
 #include <linux/cdev.h>
 #include <linux/bootmem.h>

@@ -142,6 +143,14 @@ static struct inode *alloc_inode(struct 
 			return NULL;
 		}
 
+		if (integrity_inode_alloc(inode)) {
+			if (inode->i_sb->s_op->destroy_inode)
+				inode->i_sb->s_op->destroy_inode(inode);
+			else
+				kmem_cache_free(inode_cachep, (inode));
+			return NULL;
+		}
+
 		mapping->a_ops = &empty_aops;
  		mapping->host = inode;
 		mapping->flags = 0;

@@ -172,6 +181,7 @@ void destroy_inode(struct inode *inode) 
 {
 	BUG_ON(inode_has_buffers(inode));
 	security_inode_free(inode);
+	integrity_inode_free(inode);
 	if (inode->i_sb->s_op->destroy_inode)
 		inode->i_sb->s_op->destroy_inode(inode);
 	else

fs/xattr.c +9 -0

Index: linux-2.6.21-rc3-mm2/fs/xattr.c
===================================================================
@@ -14,6 +14,7 @@
 #include <linux/xattr.h>
 #include <linux/namei.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/syscalls.h>
 #include <linux/module.h>
 #include <linux/fsnotify.h>

@@ -84,11 +85,17 @@ vfs_setxattr(struct dentry *dentry, char
 	error = security_inode_setxattr(dentry, name, value, size, flags);
 	if (error)
 		goto out;
+
+	error = integrity_inode_setxattr(dentry, name, value, size, flags);
+	if (error)
+		goto out;
+
 	error = -EOPNOTSUPP;
 	if (inode->i_op->setxattr) {
 		error = inode->i_op->setxattr(dentry, name, value, size, flags);
 		if (!error) {
 			fsnotify_xattr(dentry);
+			integrity_inode_post_setxattr(dentry, name);
 			security_inode_post_setxattr(dentry, name, value,
 						     size, flags);
 		}

@@ -181,6 +188,8 @@ vfs_removexattr(struct dentry *dentry, c
 
 	mutex_lock(&inode->i_mutex);
 	error = inode->i_op->removexattr(dentry, name);
+	if (!error)
+		integrity_inode_post_setxattr(dentry, name);
 	mutex_unlock(&inode->i_mutex);
 
 	if (!error)

Chris Wright

2007-03-08 16:42 UTC #1 lore raw

Quoted 31 lines · * Mimi Zohar ([email protected]) wrote:

(quoted diff) +7 -0

+	integrity_d_instantiate(entry, inode);
 	security_d_instantiate(entry, inode);
 
+		integrity_d_instantiate(entry, inode);
 		security_d_instantiate(entry, inode);
 
 			spin_unlock(&dcache_lock);
+			integrity_d_instantiate(new, inode);
 			security_d_instantiate(new, inode);
 			d_rehash(dentry);
 
 			d_move(new, dentry);
+			integrity_d_instantiate(dentry, inode);
 			security_d_instantiate(dentry, inode);
 			d_rehash(dentry);
 
 	if (actual == dentry) {
+		integrity_d_instantiate(dentry, inode);
 		security_d_instantiate(dentry, inode);
 
 
 		file->f_op->release(inode, file);
 	security_file_free(file);
+	integrity_file_free(file);
 
 
 		security_file_free(file);
+		integrity_file_free(file);


		if (security_inode_alloc(inode)) {

...

Quoted 6 lines

(quoted diff) +4 -0

+		if (integrity_inode_alloc(inode)) {
+			if (inode->i_sb->s_op->destroy_inode)
+				inode->i_sb->s_op->destroy_inode(inode);
 
 	security_inode_free(inode);
+	integrity_inode_free(inode);

Review

Quoted 10 lines

(quoted diff) +3 -0

 
 	error = security_inode_setxattr(dentry, name, value, size, flags);
 	if (error)
 		goto out;
+
+	error = integrity_inode_setxattr(dentry, name, value, size, flags);
 
 			fsnotify_xattr(dentry);
+			integrity_inode_post_setxattr(dentry, name);
 			security_inode_post_setxattr(dentry, name, value,

I know there's some slightly different goals, but this just doesn't make sense. Need to get back to defining and expressing just the differences.

Serge E. Hallyn

2007-03-08 17:07 UTC #2 lore raw

Quoted 46 lines · Chris Wright

Quoted 31 lines · * Mimi Zohar ([email protected]) wrote:

(quoted diff) +7 -0

+	integrity_d_instantiate(entry, inode);
 	security_d_instantiate(entry, inode);
 
+		integrity_d_instantiate(entry, inode);
 		security_d_instantiate(entry, inode);
 
 			spin_unlock(&dcache_lock);
+			integrity_d_instantiate(new, inode);
 			security_d_instantiate(new, inode);
 			d_rehash(dentry);
 
 			d_move(new, dentry);
+			integrity_d_instantiate(dentry, inode);
 			security_d_instantiate(dentry, inode);
 			d_rehash(dentry);
 
 	if (actual == dentry) {
+		integrity_d_instantiate(dentry, inode);
 		security_d_instantiate(dentry, inode);
 
 
 		file->f_op->release(inode, file);
 	security_file_free(file);
+	integrity_file_free(file);
 
 
 		security_file_free(file);
+		integrity_file_free(file);


		if (security_inode_alloc(inode)) {

...

Quoted 12 lines

(quoted diff) +6 -0

+		if (integrity_inode_alloc(inode)) {
+			if (inode->i_sb->s_op->destroy_inode)
+				inode->i_sb->s_op->destroy_inode(inode);
 
 	security_inode_free(inode);
+	integrity_inode_free(inode);
 
 	error = security_inode_setxattr(dentry, name, value, size, flags);
 	if (error)
 		goto out;
+
+	error = integrity_inode_setxattr(dentry, name, value, size, flags);

Quoted 6 lines

fs/xattr.c vfs_setxattr(struct dentry *dentry, char · line 99
Quoted 3 lines
fs/xattr.c +1 -0
 			fsnotify_xattr(dentry);
+			integrity_inode_post_setxattr(dentry, name);
 			security_inode_post_setxattr(dentry, name, value,
I know there's some slightly different goals, but this just doesn't make sense. Need to get back to defining and expressing just the differences.

Are you objecting only to the duplication at the callsites, so that an fsnotify-type of consolidation of security and integrity hooks would be ok? Or are you complaining that the security_inode_setxattr and integrity_inode_setxattr hooks are too similar anyway, and integrity modules should just use some lsm hooks for anything which will be authoritative?

(I could see an argument that integirty subsystem should be purely for measuring and hence its hooks should never return a value. Only hitch there is that if integrity subsystem hits ENOMEM it should be able to refuse the action...)

-serge

Chris Wright

2007-03-08 17:42 UTC #3 lore raw

Quoted 6 lines · * Serge E. Hallyn ([email protected]) wrote:

Are you objecting only to the duplication at the callsites, so that an fsnotify-type of consolidation of security and integrity hooks would be ok? Or are you complaining that the security_inode_setxattr and integrity_inode_setxattr hooks are too similar anyway, and integrity modules should just use some lsm hooks for anything which will be authoritative?

It's duplication of callsites with many identical implementations that's the problem.

Quoted 4 lines

(I could see an argument that integirty subsystem should be purely for measuring and hence its hooks should never return a value. Only hitch there is that if integrity subsystem hits ENOMEM it should be able to refuse the action...)

Right, that's what I was expecting to see, just the measurement infrastructure.

thanks,
-chris

Serge E. Hallyn

2007-03-08 18:01 UTC #4 lore raw

Quoted 10 lines · Chris Wright

Quoted 6 lines · * Serge E. Hallyn ([email protected]) wrote:
Are you objecting only to the duplication at the callsites, so that an fsnotify-type of consolidation of security and integrity hooks would be ok? Or are you complaining that the security_inode_setxattr and integrity_inode_setxattr hooks are too similar anyway, and integrity modules should just use some lsm hooks for anything which will be authoritative?
It's duplication of callsites with many identical implementations that's the problem.

Yes it's ugly...

But I guess it gets a point across :)

Quoted 7 lines

Quoted 4 lines
(I could see an argument that integirty subsystem should be purely for measuring and hence its hooks should never return a value. Only hitch there is that if integrity subsystem hits ENOMEM it should be able to refuse the action...)
Right, that's what I was expecting to see, just the measurement infrastructure.

So what you are saying is EVM would stay an LSM, with a cooperating integrity subsystem *just* doing measurements?

That's kind of what i was expecting too, however that doesn't fit as well with the idea that an integrity subsystem prevents the need for lsm stacking. I think the idea was that evm would still be able to enforce integrity of selinux xattrs without it stack with selinux. So I can see where this approach came from.

-serge

Stephen Smalley

2007-03-08 19:28 UTC #5 lore raw

Quoted 24 lines · Serge E. Hallyn

Quoted 10 lines · Chris Wright
Quoted 6 lines · * Serge E. Hallyn ([email protected]) wrote:
Are you objecting only to the duplication at the callsites, so that an fsnotify-type of consolidation of security and integrity hooks would be ok? Or are you complaining that the security_inode_setxattr and integrity_inode_setxattr hooks are too similar anyway, and integrity modules should just use some lsm hooks for anything which will be authoritative?
It's duplication of callsites with many identical implementations that's the problem.
Yes it's ugly...
But I guess it gets a point across :)
Quoted 7 lines
Quoted 4 lines
(I could see an argument that integirty subsystem should be purely for measuring and hence its hooks should never return a value. Only hitch there is that if integrity subsystem hits ENOMEM it should be able to refuse the action...)
Right, that's what I was expecting to see, just the measurement infrastructure.

Quoted 8 lines

So what you are saying is EVM would stay an LSM, with a cooperating integrity subsystem *just* doing measurements?
That's kind of what i was expecting too, however that doesn't fit as well with the idea that an integrity subsystem prevents the need for lsm stacking. I think the idea was that evm would still be able to enforce integrity of selinux xattrs without it stack with selinux. So I can see where this approach came from.

The enforcement mechanism should be directly integrated into SELinux, not stacked as a separate module.

Signature · 2 lines

Stephen Smalley
National Security Agency

Serge E. Hallyn

2007-03-08 21:07 UTC #6 lore raw

Quoted 28 lines · Stephen Smalley

Quoted 27 lines · Serge E. Hallyn
Quoted 10 lines · Chris Wright
Quoted 6 lines · * Serge E. Hallyn ([email protected]) wrote:
Are you objecting only to the duplication at the callsites, so that an fsnotify-type of consolidation of security and integrity hooks would be ok? Or are you complaining that the security_inode_setxattr and integrity_inode_setxattr hooks are too similar anyway, and integrity modules should just use some lsm hooks for anything which will be authoritative?
It's duplication of callsites with many identical implementations that's the problem.
Yes it's ugly...
But I guess it gets a point across :)
Quoted 7 lines
Quoted 4 lines
(I could see an argument that integirty subsystem should be purely for measuring and hence its hooks should never return a value. Only hitch there is that if integrity subsystem hits ENOMEM it should be able to refuse the action...)
Right, that's what I was expecting to see, just the measurement infrastructure.
So what you are saying is EVM would stay an LSM, with a cooperating integrity subsystem *just* doing measurements?

Quoted 8 lines

Quoted 5 lines
That's kind of what i was expecting too, however that doesn't fit as well with the idea that an integrity subsystem prevents the need for lsm stacking. I think the idea was that evm would still be able to enforce integrity of selinux xattrs without it stack with selinux. So I can see where this approach came from.
The enforcement mechanism should be directly integrated into SELinux, not stacked as a separate module.

And a big plus of splitting it up as I was describing is that anyone who wanted could make use of the measurement module to add such functionality to SELinux, as an alternative to the EVM LSM module.

-serge

Mimi Zohar

author

2007-03-08 18:22 UTC #7 lore raw

In reply to Chris Wright

Quoted 11 lines · Chris Wright

Quoted 6 lines · * Serge E. Hallyn ([email protected]) wrote:
Are you objecting only to the duplication at the callsites, so that an fsnotify-type of consolidation of security and integrity hooks would be ok? Or are you complaining that the security_inode_setxattr and integrity_inode_setxattr hooks are too similar anyway, and integrity modules should just use some lsm hooks for anything which will be authoritative?
It's duplication of callsites with many identical implementations that's the problem.

Quoted 7 lines

Quoted 4 lines
(I could see an argument that integirty subsystem should be purely for measuring and hence its hooks should never return a value. Only hitch there is that if integrity subsystem hits ENOMEM it should be able to refuse the action...)
Right, that's what I was expecting to see, just the measurement infrastructure.

There are a total of 10 Linux Integrity Module(LIM) hooks. Seven of which parallel the LSM hooks, out of the ~150 LSM hooks. 3 of the LIM hooks are for initializing, allocating, and freeing the inode-

Quoted 1 line

i_integrity, used for caching integrity information. As the integrity

information is stored as extended attributes, 2 hooks are for catching changes to the extended attributes, one is for updating the extended attributes when the file closes, and d_instantiate is used for initialization. Is this excessive? How else would you design integrity, without using the LSM hooks?

Mimi Zohar

Dmitriy Monakhov

2007-03-08 18:16 UTC #8 lore raw

In reply to Mimi Zohar

Quoted 17 lines · Mimi Zohar

This patch places calls to the new integrity hooks in the appropriate places in the fs directory. It is not meant in any way to be viewed as a complete set, but used as a basis for an initial discussion.
Index: linux-2.6.21-rc3-mm2/fs/ext3/xattr_security.c
===================================================================
fs/ext3/xattr_security.c +1 -0
@@ -10,6 +10,7 @@
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include "xattr.h"
 
 static size_t

fs/ext3/xattr_security.c ext3_init_security(handle_t *handle, str · line 68

Quoted 10 lines

fs/ext3/xattr_security.c +3 -0

@@ -58,12 +59,19 @@ ext3_init_security(handle_t *handle, str
 
 	err = security_inode_init_security(inode, dir, &name, &value, &len);
 	if (err) {
+		/* Even if creation of the security xattr fails, must
+		 * indicate this is a new inode. */
+		integrity_inode_init_integrity(inode, dir, NULL, NULL, NULL);
 		if (err == -EOPNOTSUPP)
 			return 0;
 		return err;
 	}

<<<< block begin

fs/ext3/xattr_security.c ext3_init_security(handle_t *handle, str · line 74

Quoted 6 lines

fs/ext3/xattr_security.c +4 -0

 	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
 				    name, value, len, 0);
+
+	integrity_inode_init_integrity(inode, dir, &name, &value, &len);
+	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
+				    name, value, len, 0);

<<<< block end
May be i've missed something, but i don't get last block. Why you call ext3_xattr_set_handle() twise?,
or you just mistyped and it has to look like this:
<<<<block_begin

+	integrity_inode_init_integrity(inode, dir, &name, &value, &len);
	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
				    name, value, len, 0);

<<<block_end

Quoted 179 lines

fs/ext3/xattr_security.c +0 -0

 	kfree(name);
 	kfree(value);
 	return err;

Index: linux-2.6.21-rc3-mm2/include/linux/fs.h
===================================================================

include/linux/fs.h +3 -0

@@ -591,6 +591,9 @@ struct inode {
 #ifdef CONFIG_SECURITY
 	void			*i_security;
 #endif
+#ifdef CONFIG_INTEGRITY
+	void			*i_integrity;
+#endif
 	void			*i_private; /* fs or device private pointer */
 };

Index: linux-2.6.21-rc3-mm2/fs/dcache.c
===================================================================

fs/dcache.c +6 -0

@@ -29,6 +29,7 @@
 #include <linux/file.h>
 #include <asm/uaccess.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/seqlock.h>
 #include <linux/swap.h>
 #include <linux/bootmem.h>

@@ -986,6 +987,7 @@ void d_instantiate(struct dentry *entry,
 	entry->d_inode = inode;
 	fsnotify_d_instantiate(entry, inode);
 	spin_unlock(&dcache_lock);
+	integrity_d_instantiate(entry, inode);
 	security_d_instantiate(entry, inode);
 }

@@ -1050,6 +1052,7 @@ struct dentry *d_instantiate_unique(stru
 	spin_unlock(&dcache_lock);
 
 	if (!result) {
+		integrity_d_instantiate(entry, inode);
 		security_d_instantiate(entry, inode);
 		return NULL;
 	}

@@ -1187,6 +1190,7 @@ struct dentry *d_splice_alias(struct ino
 			BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
 			fsnotify_d_instantiate(new, inode);
 			spin_unlock(&dcache_lock);
+			integrity_d_instantiate(new, inode);
 			security_d_instantiate(new, inode);
 			d_rehash(dentry);
 			d_move(new, dentry);

@@ -1197,6 +1201,7 @@ struct dentry *d_splice_alias(struct ino
 			dentry->d_inode = inode;
 			fsnotify_d_instantiate(dentry, inode);
 			spin_unlock(&dcache_lock);
+			integrity_d_instantiate(dentry, inode);
 			security_d_instantiate(dentry, inode);
 			d_rehash(dentry);
 		}

@@ -1748,6 +1753,7 @@ found:
 	spin_unlock(&dcache_lock);
 out_nolock:
 	if (actual == dentry) {
+		integrity_d_instantiate(dentry, inode);
 		security_d_instantiate(dentry, inode);
 		return NULL;
 	}

Index: linux-2.6.21-rc3-mm2/fs/file_table.c
===================================================================

fs/file_table.c +3 -0

@@ -13,6 +13,7 @@
 #include <linux/smp_lock.h>
 #include <linux/fs.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/eventpoll.h>
 #include <linux/rcupdate.h>
 #include <linux/mount.h>

@@ -169,6 +170,7 @@ void fastcall __fput(struct file *file)
 	if (file->f_op && file->f_op->release)
 		file->f_op->release(inode, file);
 	security_file_free(file);
+	integrity_file_free(file);
 	if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL))
 		cdev_put(inode->i_cdev);
 	fops_put(file->f_op);

@@ -240,6 +242,7 @@ void put_filp(struct file *file)
 {
 	if (atomic_dec_and_test(&file->f_count)) {
 		security_file_free(file);
+		integrity_file_free(file);
 		file_kill(file);
 		file_free(file);
 	}

Index: linux-2.6.21-rc3-mm2/fs/inode.c
===================================================================

fs/inode.c +10 -0

@@ -17,6 +17,7 @@
 #include <linux/hash.h>
 #include <linux/swap.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/pagemap.h>
 #include <linux/cdev.h>
 #include <linux/bootmem.h>

@@ -142,6 +143,14 @@ static struct inode *alloc_inode(struct 
 			return NULL;
 		}
 
+		if (integrity_inode_alloc(inode)) {
+			if (inode->i_sb->s_op->destroy_inode)
+				inode->i_sb->s_op->destroy_inode(inode);
+			else
+				kmem_cache_free(inode_cachep, (inode));
+			return NULL;
+		}
+
 		mapping->a_ops = &empty_aops;
  		mapping->host = inode;
 		mapping->flags = 0;

@@ -172,6 +181,7 @@ void destroy_inode(struct inode *inode) 
 {
 	BUG_ON(inode_has_buffers(inode));
 	security_inode_free(inode);
+	integrity_inode_free(inode);
 	if (inode->i_sb->s_op->destroy_inode)
 		inode->i_sb->s_op->destroy_inode(inode);
 	else

Index: linux-2.6.21-rc3-mm2/fs/xattr.c
===================================================================

fs/xattr.c +9 -1

@@ -14,6 +14,7 @@
 #include <linux/xattr.h>
 #include <linux/namei.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include <linux/syscalls.h>
 #include <linux/module.h>
 #include <linux/fsnotify.h>

@@ -84,11 +85,17 @@ vfs_setxattr(struct dentry *dentry, char
 	error = security_inode_setxattr(dentry, name, value, size, flags);
 	if (error)
 		goto out;
+
+	error = integrity_inode_setxattr(dentry, name, value, size, flags);
+	if (error)
+		goto out;
+
 	error = -EOPNOTSUPP;
 	if (inode->i_op->setxattr) {
 		error = inode->i_op->setxattr(dentry, name, value, size, flags);
 		if (!error) {
 			fsnotify_xattr(dentry);
+			integrity_inode_post_setxattr(dentry, name);
 			security_inode_post_setxattr(dentry, name, value,
 						     size, flags);
 		}

@@ -181,6 +188,8 @@ vfs_removexattr(struct dentry *dentry, c
 
 	mutex_lock(&inode->i_mutex);
 	error = inode->i_op->removexattr(dentry, name);
+	if (!error)
+		integrity_inode_post_setxattr(dentry, name);
 	mutex_unlock(&inode->i_mutex);
 
 	if (!error)
 
 
 
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Mimi Zohar

author

2007-03-08 18:33 UTC #9 lore raw

Quoted 30 lines · Dmitriy Monakhov

Quoted 17 lines · Mimi Zohar
This patch places calls to the new integrity hooks in the appropriate places in the fs directory. It is not meant in any way to be viewed as a complete set, but used as a basis for an initial discussion.
Index: linux-2.6.21-rc3-mm2/fs/ext3/xattr_security.c
===================================================================
fs/ext3/xattr_security.c +1 -0
@@ -10,6 +10,7 @@
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/security.h>
+#include <linux/integrity.h>
 #include "xattr.h"
 
 static size_t
fs/ext3/xattr_security.c ext3_init_security(handle_t *handle, str · line 68
Quoted 10 lines
fs/ext3/xattr_security.c +3 -0
@@ -58,12 +59,19 @@ ext3_init_security(handle_t *handle, str
 
 	err = security_inode_init_security(inode, dir, &name, &value, &len);
 	if (err) {
+		/* Even if creation of the security xattr fails, must
+		 * indicate this is a new inode. */
+		integrity_inode_init_integrity(inode, dir, NULL, NULL, NULL);
 		if (err == -EOPNOTSUPP)
 			return 0;
 		return err;
 	}
<<<< block begin

Quoted 11 lines

fs/ext3/xattr_security.c ext3_init_security(handle_t *handle, str · line 74
Quoted 6 lines
fs/ext3/xattr_security.c +4 -0
 	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
 				    name, value, len, 0);
+
+	integrity_inode_init_integrity(inode, dir, &name, &value, &len);
+	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
+				    name, value, len, 0);
<<<< block end
May be i've missed something, but i don't get last block. Why you call ext3_xattr_set_handle() twise?,
or you just mistyped and it has to look like this:
<<<<block_begin

Both security_inode_init_security and integrity_inode_init_integrity create extended attributes. The first ext3_xattr_set_handle creates the security extended attribute and the second one creates the integrity extended attribute. In the case of EVM, EVM calculates an HMAC on a set of extended attributes, possibly including the initial security extended attribute.

Mimi Zohar