[PATCH v2 00/10] define new fs integrity_read method

With the introduction of IMA-appraisal and the need to write file
hashes as security xattrs, IMA needed to take the global i_mutex
lock.  process_measurement() took the iint->mutex first and then
the i_mutex, while setxattr, chmod and chown took the locks in
reverse order.  To resolve this potential deadlock, the iint->mutex
was removed.

Some filesystems have recently replaced their filesystem dependent
lock with the global i_rwsem (formerly the i_mutex) to read a file.
As a result, when IMA attempts to calculate the file hash, reading
the file attempts to take the i_rwsem again.

To resolve this locking problem, this patch set introduces a new
->integrity_read file operation method.  Until all filesystems
define the new ->integrity_read method, files that were previously
measured might not be currently measured and files that were
previously appraised might fail to be appraised properly.

Version 2 of this patch set, introduces measurement entries and
IMA-audit messages containing file hash values containing 0's,
instead of the actual file hash, for files which the file hash
could not be calculated.  Like for any other file signature
verification error, file access/execute permission will be denied,
for files in policy that the file hash could not be calculated.

To override the IMA policy, allowing unverified code to be
accessed/executed on filesystems not supported by IMA, version 2 of
this patch set defines a new pseudo policy "action" named
"dont_failsafe" and a new builtin policy named "fs_unsafe", which
can be specified on the boot command line.

Mimi

Changelog v1:
- Don't silently drop file measurements on failure to "collect" the
file hash, add an entry in the measurement list and IMA-audit log
the file.
- Define a pseudo policy action named "dont_failsafe".
- Define a new builtin IMA policy named "fs_unsafe".
- Instead of calling the existing read_iter method, when defined
as generic_file_read_iter(), define an ->integrity_read method
for each file system.
- Expanded/clarified motivation in the patch description for using
the ->read method.
- Use kvec, not iovec. (Reported by zero day testing)

Christoph Hellwig (1):
  ima: use fs method to read integrity data

Mimi Zohar (9):
  ima: always measure and audit files in policy
  ima: define "dont_failsafe" policy action rule
  ima: define "fs_unsafe" builtin policy
  tmpfs: define integrity_read method
  fs: define integrity_read method for ext2, gfs2, f2fs, jfs, ramfs
  ocfs2: define integrity_read method
  jffs2: define integrity_read method
  ubifs: define integrity_read method
  ima: use existing read file operation method to calculate file hash

 Documentation/ABI/testing/ima_policy            |  3 ++-
 Documentation/admin-guide/kernel-parameters.txt |  8 ++++++-
 fs/btrfs/file.c                                 |  1 +
 fs/ext2/file.c                                  |  1 +
 fs/ext4/file.c                                  |  1 +
 fs/f2fs/file.c                                  |  1 +
 fs/gfs2/file.c                                  |  2 ++
 fs/jffs2/file.c                                 |  1 +
 fs/jfs/file.c                                   |  1 +
 fs/ocfs2/file.c                                 |  1 +
 fs/ramfs/file-mmu.c                             |  1 +
 fs/ramfs/file-nommu.c                           |  1 +
 fs/ubifs/file.c                                 |  1 +
 fs/xfs/xfs_file.c                               | 21 +++++++++++++++++
 include/linux/fs.h                              |  1 +
 mm/shmem.c                                      |  1 +
 security/integrity/iint.c                       | 31 +++++++++++++++++++------
 security/integrity/ima/ima.h                    |  1 +
 security/integrity/ima/ima_api.c                |  7 ++++--
 security/integrity/ima/ima_main.c               | 15 +++++++++---
 security/integrity/ima/ima_policy.c             | 16 ++++++++++++-
 21 files changed, 101 insertions(+), 15 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Define an ->integrity_read file operation method to read data for
integrity hash collection.

Signed-off-by: Mimi Zohar <redacted>
---
 mm/shmem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index e67d6ba4e98e..16958b20946f 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c

@@ -3846,6 +3846,7 @@ static const struct file_operations shmem_file_operations = {
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
+	.integrity_read	= shmem_file_read_iter,
 #endif
 };
 

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

All files matching a "measure" rule must be included in the IMA
measurement list, even when the file hash cannot be calculated.
Similarly, all files matching an "audit" rule must be audited, even when
the file hash cannot be calculated.

The file data hash field contained in the IMA measurement list template
data will contain 0's instead of the actual file hash digest.

Mimi Zohar [off-list ref]
---
 security/integrity/ima/ima_api.c  | 7 +++++--
 security/integrity/ima/ima_main.c | 4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c2edba8de35e..174cf2e8dd45 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c

@@ -217,15 +217,18 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 
 		result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
 			ima_calc_buffer_hash(buf, size, &hash.hdr);
-		if (!result) {
+		if (!result || (result == -EBADF)) {
 			int length = sizeof(hash.hdr) + hash.hdr.length;
 			void *tmpbuf = krealloc(iint->ima_hash, length,
 						GFP_NOFS);
 			if (tmpbuf) {
 				iint->ima_hash = tmpbuf;
+				if (result == -EBADF)
+					memset(&hash.digest, 0, hash.hdr.length);
 				memcpy(iint->ima_hash, &hash, length);
 				iint->version = i_version;
-				iint->flags |= IMA_COLLECTED;
+				if (result != -EBADF)
+					iint->flags |= IMA_COLLECTED;
 			} else
 				result = -ENOMEM;
 		}

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..63777d1210b1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -235,7 +235,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
 	rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
-	if (rc != 0) {
+	if (rc != 0 && rc != -EBADF) {
 		if (file->f_flags & O_DIRECT)
 			rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;
 		goto out_digsig;

@@ -247,7 +247,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, pcr);
-	if (action & IMA_APPRAISE_SUBMASK)
+	if ((rc != -EBADF) && (action & IMA_APPRAISE_SUBMASK))
 		rc = ima_appraise_measurement(func, iint, file, pathname,
 					      xattr_value, xattr_len, opened);
 	if (action & IMA_AUDIT)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Christoph Hellwig <hch@lst.de>

Add a new ->integrity_read file operation to read data for integrity
hash collection.  This is defined to be equivalent to ->read_iter,
except that it will be called with the i_rwsem held exclusively.

Changelog v2:
- change iovec to kvec

Changelog v1:
- update the patch description, removing the concept that the presence of
->integrity_read indicates that the file system can support IMA. (Mimi)

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mimi Zohar <redacted>
---
 fs/btrfs/file.c           |  1 +
 fs/ext4/file.c            |  1 +
 fs/xfs/xfs_file.c         | 21 +++++++++++++++++++++
 include/linux/fs.h        |  1 +
 security/integrity/iint.c | 20 ++++++++++++++------
 5 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index da1096eb1a40..003e859b56c4 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c

@@ -3087,6 +3087,7 @@ const struct file_operations btrfs_file_operations = {
 #endif
 	.clone_file_range = btrfs_clone_file_range,
 	.dedupe_file_range = btrfs_dedupe_file_range,
+	.integrity_read = generic_file_read_iter,
 };
 
 void btrfs_auto_defrag_exit(void)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 831fd6beebf0..e7b2bd43cdc4 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c

@@ -753,6 +753,7 @@ const struct file_operations ext4_file_operations = {
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ext4_fallocate,
+	.integrity_read	= ext4_file_read_iter,
 };
 
 const struct inode_operations ext4_file_inode_operations = {

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 35703a801372..3d6ace2a79bc 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c

@@ -288,6 +288,26 @@ xfs_file_read_iter(
 	return ret;
 }
 
+static ssize_t
+xfs_integrity_read(
+	struct kiocb		*iocb,
+	struct iov_iter		*to)
+{
+	struct inode		*inode = file_inode(iocb->ki_filp);
+	struct xfs_mount	*mp = XFS_I(inode)->i_mount;
+
+	lockdep_assert_held(&inode->i_rwsem);
+
+	XFS_STATS_INC(mp, xs_read_calls);
+
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return -EIO;
+
+	if (IS_DAX(inode))
+		return dax_iomap_rw(iocb, to, &xfs_iomap_ops);
+	return generic_file_read_iter(iocb, to);
+}
+
 /*
  * Zero any on disk space between the current EOF and the new, larger EOF.
  *

@@ -1534,6 +1554,7 @@ const struct file_operations xfs_file_operations = {
 	.fallocate	= xfs_file_fallocate,
 	.clone_file_range = xfs_file_clone_range,
 	.dedupe_file_range = xfs_file_dedupe_range,
+	.integrity_read	= xfs_integrity_read,
 };
 
 const struct file_operations xfs_dir_file_operations = {

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 803e5a9b2654..36edfe84c4bf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -1690,6 +1690,7 @@ struct file_operations {
 			u64);
 	ssize_t (*dedupe_file_range)(struct file *, u64, u64, struct file *,
 			u64);
+	ssize_t (*integrity_read)(struct kiocb *, struct iov_iter *);
 };
 
 struct inode_operations {

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 6fc888ca468e..df04f35a1d40 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c

@@ -21,6 +21,7 @@
 #include <linux/rbtree.h>
 #include <linux/file.h>
 #include <linux/uaccess.h>
+#include <linux/uio.h>
 #include "integrity.h"
 
 static struct rb_root integrity_iint_tree = RB_ROOT;

@@ -184,18 +185,25 @@ security_initcall(integrity_iintcache_init);
 int integrity_kernel_read(struct file *file, loff_t offset,
 			  void *addr, unsigned long count)
 {
-	mm_segment_t old_fs;
-	char __user *buf = (char __user *)addr;
+	struct inode *inode = file_inode(file);
+	struct kvec iov = { .iov_base = addr, .iov_len = count };
+	struct kiocb kiocb;
+	struct iov_iter iter;
 	ssize_t ret;
 
+	lockdep_assert_held(&inode->i_rwsem);
+
 	if (!(file->f_mode & FMODE_READ))
 		return -EBADF;
+	if (!file->f_op->integrity_read)
+		return -EBADF;
 
-	old_fs = get_fs();
-	set_fs(get_ds());
-	ret = __vfs_read(file, buf, count, &offset);
-	set_fs(old_fs);
+	init_sync_kiocb(&kiocb, file);
+	kiocb.ki_pos = offset;
+	iov_iter_kvec(&iter, READ | ITER_KVEC, &iov, 1, count);
 
+	ret = file->f_op->integrity_read(&kiocb, &iter);
+	BUG_ON(ret == -EIOCBQUEUED);
 	return ret;
 }
 

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Permit normally denied access/execute permission for files in policy
on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
policy action rule.

Mimi Zohar [off-list ref]
---
 Documentation/ABI/testing/ima_policy |  3 ++-
 security/integrity/ima/ima.h         |  1 +
 security/integrity/ima/ima_main.c    | 11 ++++++++++-
 security/integrity/ima/ima_policy.c  | 14 +++++++++++++-
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index e76432b9954d..f271207743e5 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy

@@ -17,7 +17,8 @@ Description:
 
 		rule format: action [condition ...]
 
-		action: measure | dont_measure | appraise | dont_appraise | audit
+		action: measure | dont_meaure | appraise | dont_appraise |
+			audit | dont_failsafe
 		condition:= base | lsm  [option]
 			base:	[[func=] [mask=] [fsmagic=] [fsuuid=] [uid=]
 				[euid=] [fowner=]]

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..c5f34f7c5b0f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -224,6 +224,7 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos);
 void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos);
 void ima_policy_stop(struct seq_file *m, void *v);
 int ima_policy_show(struct seq_file *m, void *v);
+void set_failsafe(bool flag);
 
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 63777d1210b1..59e271a20600 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -38,6 +38,11 @@ int ima_appraise;
 int ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
 
+static bool ima_failsafe = 1;
+void set_failsafe(bool flag) {
+	ima_failsafe = flag;
+}
+
 static int __init hash_setup(char *str)
 {
 	struct ima_template_desc *template_desc = ima_template_desc_current();

@@ -263,8 +268,12 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		__putname(pathbuf);
 out:
 	inode_unlock(inode);
-	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
+	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+		if (!ima_failsafe && rc == -EBADF)
+			return 0;
+
 		return -EACCES;
+	}
 	return 0;
 }
 

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f4436626ccb7..cb92c9c04e80 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -46,6 +46,7 @@
 
 int ima_policy_flag;
 static int temp_ima_appraise;
+static bool temp_failsafe = 1;
 
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,

@@ -513,6 +514,12 @@ void ima_update_policy(void)
 	if (ima_rules != policy) {
 		ima_policy_flag = 0;
 		ima_rules = policy;
+
+		/*
+		 * Only permit initial policy replacement, not append, to
+		 * set dont_failsafe.
+		 */
+		set_failsafe(temp_failsafe);
 	}
 	ima_update_policy_flag();
 }

@@ -529,7 +536,7 @@ enum {
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr
+	Opt_pcr, Opt_dont_failsafe
 };
 
 static match_table_t policy_tokens = {

@@ -560,6 +567,7 @@ static match_table_t policy_tokens = {
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
+	{Opt_dont_failsafe, "dont_failsafe"},
 	{Opt_err, NULL}
 };
 

@@ -671,6 +679,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 
 			entry->action = AUDIT;
 			break;
+		case Opt_dont_failsafe:
+			ima_log_string(ab, "action", "dont_failsafe");
+			temp_failsafe = 0;
+			break;
 		case Opt_func:
 			ima_log_string(ab, "func", args[0].from);
 

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Permit normally denied access/execute permission for files in policy
on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
builtin policy.

Mimi Zohar [off-list ref]
---
 Documentation/admin-guide/kernel-parameters.txt | 8 +++++++-
 security/integrity/ima/ima_policy.c             | 2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e438a1fca554..0aed01aabc16 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt

@@ -1478,7 +1478,7 @@
 
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
-			Format: "tcb | appraise_tcb | secure_boot"
+			Format: "tcb | appraise_tcb | secure_boot | fs_unsafe"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read

@@ -1493,6 +1493,12 @@
 			of files (eg. kexec kernel image, kernel modules,
 			firmware, policy, etc) based on file signatures.
 
+			The "fs_unsafe" policy permits normally denied
+			access/execute permission for files in policy on IMA
+			unsupported filesystems.  Note this option, as the
+			name implies, is not safe and not recommended for
+			any environments other than testing.
+
 	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
 			Load a policy which meets the needs of the Trusted
 			Computing Base.  This means IMA will measure all

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index cb92c9c04e80..4ff5640522c7 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -200,6 +200,8 @@ static int __init policy_setup(char *str)
 			ima_use_appraise_tcb = 1;
 		else if (strcmp(p, "secure_boot") == 0)
 			ima_use_secure_boot = 1;
+		else if (strcmp(p, "fs_unsafe") == 0)
+			set_failsafe(0);
 	}
 
 	return 1;

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Define ->integrity_read file operation methods to read data for
integrity hash collection.

Signed-off-by: Mimi Zohar <redacted>
---
 fs/ext2/file.c        | 1 +
 fs/f2fs/file.c        | 1 +
 fs/gfs2/file.c        | 2 ++
 fs/jfs/file.c         | 1 +
 fs/ramfs/file-mmu.c   | 1 +
 fs/ramfs/file-nommu.c | 1 +
 6 files changed, 7 insertions(+)

diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index b21891a6bfca..d57c4259945d 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c

@@ -219,6 +219,7 @@ const struct file_operations ext2_file_operations = {
 	.get_unmapped_area = thp_get_unmapped_area,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct inode_operations ext2_file_inode_operations = {

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 61af721329fa..e93fdeb3eba4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c

@@ -2406,4 +2406,5 @@ const struct file_operations f2fs_file_operations = {
 #endif
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
+	.integrity_read	= generic_file_read_iter,
 };

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index c2062a108d19..9b49d09ba180 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c

@@ -1124,6 +1124,7 @@ const struct file_operations gfs2_file_fops = {
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= simple_nosetlease,
 	.fallocate	= gfs2_fallocate,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct file_operations gfs2_dir_fops = {

@@ -1152,6 +1153,7 @@ const struct file_operations gfs2_file_fops_nolock = {
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= generic_setlease,
 	.fallocate	= gfs2_fallocate,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct file_operations gfs2_dir_fops_nolock = {

diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index 739492c7a3fd..423512a810e4 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c

@@ -162,4 +162,5 @@ const struct file_operations jfs_file_operations = {
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= jfs_compat_ioctl,
 #endif
+	.integrity_read	= generic_file_read_iter,
 };

diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c
index 12af0490322f..4f24d1b589b1 100644
--- a/fs/ramfs/file-mmu.c
+++ b/fs/ramfs/file-mmu.c

@@ -47,6 +47,7 @@ const struct file_operations ramfs_file_operations = {
 	.splice_write	= iter_file_splice_write,
 	.llseek		= generic_file_llseek,
 	.get_unmapped_area	= ramfs_mmu_get_unmapped_area,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct inode_operations ramfs_file_inode_operations = {

diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index 2ef7ce75c062..5ee704fa84e0 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c

@@ -50,6 +50,7 @@ const struct file_operations ramfs_file_operations = {
 	.splice_read		= generic_file_splice_read,
 	.splice_write		= iter_file_splice_write,
 	.llseek			= generic_file_llseek,
+	.integrity_read		= generic_file_read_iter,
 };
 
 const struct inode_operations ramfs_file_inode_operations = {

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Define an ->integrity_read file operation method to read data for
integrity hash collection.

Signed-off-by: Mimi Zohar <redacted>
---
 fs/jffs2/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index c12476e309c6..5a63034cccf5 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c

@@ -57,6 +57,7 @@ const struct file_operations jffs2_file_operations =
 	.mmap =		generic_file_readonly_mmap,
 	.fsync =	jffs2_fsync,
 	.splice_read =	generic_file_splice_read,
+	.integrity_read = generic_file_read_iter,
 };
 
 /* jffs2_file_inode_operations */

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Define an ->integrity_read file operation method to read data for
integrity hash collection.

Signed-off-by: Mimi Zohar <redacted>
---
 fs/ocfs2/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index bfeb647459d9..2832a7c92acd 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c

@@ -2536,6 +2536,7 @@ const struct file_operations ocfs2_fops = {
 	.fallocate	= ocfs2_fallocate,
 	.clone_file_range = ocfs2_file_clone_range,
 	.dedupe_file_range = ocfs2_file_dedupe_range,
+	.integrity_read	= ocfs2_file_read_iter,
 };
 
 const struct file_operations ocfs2_dops = {

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

The builtin "ima_tcb" policy measures all files read by root.  This
policy includes, for example, files on efivars.  Since some files on
these filesystems were previously measured (eg. OsIndicationsSupported),
not measuring them would change the PCR hash value(s), potentially
breaking userspace.

The few filesystems that currently define the ->read file operation
method, either call seq_read() or have a filesystem specific ->read
method.  None of them, at least in the fs directory, take the i_rwsem.

For filesystems that do not define the ->integrity_read file operation
method and have a ->read method, this patch calls the ->read method
to calculate the file hash.

Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/iint.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index df04f35a1d40..75c3cef5fd01 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c

@@ -189,20 +189,29 @@ int integrity_kernel_read(struct file *file, loff_t offset,
 	struct kvec iov = { .iov_base = addr, .iov_len = count };
 	struct kiocb kiocb;
 	struct iov_iter iter;
-	ssize_t ret;
+	ssize_t ret = -EBADF;
 
 	lockdep_assert_held(&inode->i_rwsem);
 
 	if (!(file->f_mode & FMODE_READ))
 		return -EBADF;
-	if (!file->f_op->integrity_read)
-		return -EBADF;
 
 	init_sync_kiocb(&kiocb, file);
 	kiocb.ki_pos = offset;
 	iov_iter_kvec(&iter, READ | ITER_KVEC, &iov, 1, count);
 
-	ret = file->f_op->integrity_read(&kiocb, &iter);
+	if (file->f_op->integrity_read) {
+		ret = file->f_op->integrity_read(&kiocb, &iter);
+	} else if (file->f_op->read) {
+		mm_segment_t old_fs;
+		char __user *buf = (char __user *)addr;
+
+		old_fs = get_fs();
+		set_fs(get_ds());
+		ret = file->f_op->read(file, buf, count, &offset);
+		set_fs(old_fs);
+	}
+
 	BUG_ON(ret == -EIOCBQUEUED);
 	return ret;
 }

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Define an ->integrity_read file operation method to read data for
integrity hash collection.

Signed-off-by: Mimi Zohar <redacted>
---
 fs/ubifs/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 2cda3d67e2d0..eeb4e872e47a 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c

@@ -1754,4 +1754,5 @@ const struct file_operations ubifs_file_operations = {
 #ifdef CONFIG_COMPAT
 	.compat_ioctl   = ubifs_compat_ioctl,
 #endif
+	.integrity_read = generic_file_read_iter,
 };

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Jun 21, 2017 at 02:18:22PM -0400, Mimi Zohar wrote:

From: Christoph Hellwig <hch@lst.de>

Add a new ->integrity_read file operation to read data for integrity
hash collection.  This is defined to be equivalent to ->read_iter,
except that it will be called with the i_rwsem held exclusively.

Changelog v2:
- change iovec to kvec

Changelog v1:
- update the patch description, removing the concept that the presence of
->integrity_read indicates that the file system can support IMA. (Mimi)

Changelog goes below the ---
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Jun 21, 2017 at 02:18:25PM -0400, Mimi Zohar wrote:

Define an ->integrity_read file operation method to read data for
integrity hash collection.

should be folded into patch 2.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Jun 21, 2017 at 02:18:26PM -0400, Mimi Zohar wrote:

Define ->integrity_read file operation methods to read data for
integrity hash collection.

should be folded into patch 2.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

should be folded into patch 2.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

should be folded into patch 2.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Jun 21, 2017 at 02:18:29PM -0400, Mimi Zohar wrote:

Define an ->integrity_read file operation method to read data for
integrity hash collection.

should be folded into patch 2.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

NAK - we'll need an explicit method for the integrity code.

And just curious - what filesystem that you care about actually
implements ->read instead of ->read_iter?  We shouldn't be doing that
for real file systems anymore.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Cc'ing linux-ima-users]

On Wed, 2017-06-28 at 16:41 +0200, Christoph Hellwig wrote:

NAK - we'll need an explicit method for the integrity code.

And just curious - what filesystem that you care about actually
implements ->read instead of ->read_iter?  We shouldn't be doing that
for real file systems anymore.

Right, pseudo filesystems are using ->read. The existing builtin
measurement policies exclude a number of pseudo filesystems, but not
efivarfs. ?Unfortunately, we do not know what type of custom policies
are currently being used.

The contents of the IMA measurement list are verified against a
reference manifest, provided at registration, or against a white list.
Not measuring files that were previously measured could break
userspace applications.

Let's wait to hear back from the larger IMA community as to whether
there is a need to measure files on pseudo filesystems, before
implementing an explicit method.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Jul 05, 2017 at 06:02:15PM +0100, Matthew Garrett wrote:

On Wed, Jul 05, 2017 at 10:50:09AM -0400, Mimi Zohar wrote:

quoted

[Cc'ing linux-ima-users]

On Wed, 2017-06-28 at 16:41 +0200, Christoph Hellwig wrote:

quoted

NAK - we'll need an explicit method for the integrity code.

And just curious - what filesystem that you care about actually
implements ->read instead of ->read_iter?  We shouldn't be doing that
for real file systems anymore.

Right, pseudo filesystems are using ->read. The existing builtin
measurement policies exclude a number of pseudo filesystems, but not
efivarfs. ?Unfortunately, we do not know what type of custom policies
are currently being used.

efi variables contain information that may influence userspace behaviour 
and can also be modified out of band, so I think there's a reasonable 
argument that they should be measured.

Then efivars should grow a ->integrity_read method.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Jul 05, 2017 at 10:50:09AM -0400, Mimi Zohar wrote:

[Cc'ing linux-ima-users]

On Wed, 2017-06-28 at 16:41 +0200, Christoph Hellwig wrote:

quoted

NAK - we'll need an explicit method for the integrity code.

And just curious - what filesystem that you care about actually
implements ->read instead of ->read_iter?  We shouldn't be doing that
for real file systems anymore.

Right, pseudo filesystems are using ->read. The existing builtin
measurement policies exclude a number of pseudo filesystems, but not
efivarfs. ?Unfortunately, we do not know what type of custom policies
are currently being used.

efi variables contain information that may influence userspace behaviour 
and can also be modified out of band, so I think there's a reasonable 
argument that they should be measured.

-- 
Matthew Garrett | mjg59 at srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, 2017-06-28 at 16:38 +0200, Christoph Hellwig wrote:

On Wed, Jun 21, 2017 at 02:18:25PM -0400, Mimi Zohar wrote:

quoted

Define an ->integrity_read file operation method to read data for
integrity hash collection.

should be folded into patch 2.

I was hoping to get some Acks/sign-off's from the individual
filesystem maintainers before squashing them. ?The next version will
be squashed.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html