Menu
▾
▴
linux-ima-devel
|
From: Mimi Z. <zo...@li...> - 2017-06-09 18:03:57
|
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. Originally, the presence of the integrity_read file operation method, as seen in Christoph's patch, was intended to signify that the file system supports IMA. Other than fixing this locking problem, the filesystem should be able to detect when a file changes and re-measure/re-appraise the file afterwards. IMA makes the determination of when a file changes based on the file system being mounted with i_version, but even without i_version, files would still be measured/appraised initially. Detecting and notifying when a file system is mounted without i_version should be considered a separate issue and posted as a separate patch set, independently of this one. (A very preliminary version is available from https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/ next-log-iversion-experimental.) The large majority of filesystems in the fs directory call generic_file_read_iter() directly (eg. reiserfs, 9p, adfs, affs, afs, bfs, btrfs, exofs, f2fs, fat, gf2, hfs, hfsplus, hpfs, jfs, minix, nifs2, omfs, ramfs, romfs, sysv, ubifs, udf, ufs). Only filesystems that define their own ->read_iter method, whether it takes the i_rwsem or not, should be required to define their own ->integrity_read method. This patch set defines the ->integrity_read file operation method for xfs, ext4, and tpmfs. Ceph, cifs, ecryptfs, ext2, fuse, ocfs2 have their own read_iter, but eventually call generic_file_read_iter(), still need to be converted. Coda and hugetlbfs have their own read_iter functions, which do not call generic_file_read_iter(). Although this patch set addresses the locking issue, until the remaining filesystem define their own ->integrity_read, it introduces the situation where files that were previously measured, might now not be measured and files that were previously appraised, might fail to be appraised properly, even when properly signed/hashed. Mimi Christoph Hellwig (1): ima: use fs method to read integrity data Mimi Zohar (5): tmpfs: define integrity_read file operation method ima: use existing read file operation method to calculate file hash ima: use read_iter (generic_file_read_iter) to calculate file hash security: define new LSM sb_post_new_mount hook ima: indicate possibly missing file measurements or verification fs/btrfs/file.c | 1 + fs/ext4/file.c | 1 + fs/namespace.c | 2 ++ fs/xfs/xfs_file.c | 21 +++++++++++++++++++++ include/linux/fs.h | 1 + include/linux/ima.h | 7 +++++++ include/linux/lsm_hooks.h | 9 +++++++++ include/linux/security.h | 3 +++ mm/shmem.c | 1 + security/integrity/iint.c | 34 +++++++++++++++++++++++++++------- security/integrity/ima/ima_main.c | 31 +++++++++++++++++++++++++++++++ security/security.c | 7 +++++++ 12 files changed, 111 insertions(+), 7 deletions(-) -- 2.7.4 ==== *** BLURB HERE *** Christoph Hellwig (1): ima: use fs method to read integrity data Mimi Zohar (3): tmpfs: define integrity_read file operation method ima: use existing read file operation method to calculate file hash ima: use read_iter (generic_file_read_iter) to calculate file hash fs/btrfs/file.c | 1 + fs/ext4/file.c | 1 + fs/xfs/xfs_file.c | 21 +++++++++++++++++++++ include/linux/fs.h | 1 + mm/shmem.c | 1 + security/integrity/iint.c | 34 +++++++++++++++++++++++++++------- 6 files changed, 52 insertions(+), 7 deletions(-) -- 2.7.4 |
|
From: Mimi Z. <zo...@li...> - 2017-06-09 18:04:01
|
Although temporary filesystems for the most part are not something
that we're interested in measuring or appraising, we do want to at least
measure, and at some point appraise, files on the rootfs.
This patch defines an integrity_read file operation method used for
calculating the file hash.
Signed-off-by: Mimi Zohar <zo...@li...>
---
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
|
|
From: Mimi Z. <zo...@li...> - 2017-06-09 18:04:01
|
From: Christoph Hellwig <hc...@ls...>
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 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 <hc...@ls...>
Signed-off-by: Mimi Zohar <zo...@li...>
---
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 c710d22042f9..c5489672b5aa 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,
char *addr, unsigned long count)
{
- mm_segment_t old_fs;
- char __user *buf = (char __user *)addr;
+ struct inode *inode = file_inode(file);
+ struct iovec 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_init(&iter, READ | ITER_KVEC, &iov, 1, count);
+ ret = file->f_op->integrity_read(&kiocb, &iter);
+ BUG_ON(ret == -EIOCBQUEUED);
return ret;
}
--
2.7.4
|
|
From: kbuild t. r. <lk...@in...> - 2017-06-15 23:50:12
|
Hi Christoph, [auto build test WARNING on security/next] [also build test WARNING on v4.12-rc5 next-20170615] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-use-fs-method-to-read-integrity-data/20170611-062655 base: https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> security//integrity/iint.c:189:42: sparse: incorrect type in initializer (different address spaces) security//integrity/iint.c:189:42: expected void [noderef] <asn:1>*iov_base security//integrity/iint.c:189:42: got char *addr vim +189 security//integrity/iint.c 173 } 174 security_initcall(integrity_iintcache_init); 175 176 177 /* 178 * integrity_kernel_read - read data from the file 179 * 180 * This is a function for reading file content instead of kernel_read(). 181 * It does not perform locking checks to ensure it cannot be blocked. 182 * It does not perform security checks because it is irrelevant for IMA. 183 * 184 */ 185 int integrity_kernel_read(struct file *file, loff_t offset, 186 char *addr, unsigned long count) 187 { 188 struct inode *inode = file_inode(file); > 189 struct iovec iov = { .iov_base = addr, .iov_len = count }; 190 struct kiocb kiocb; 191 struct iov_iter iter; 192 ssize_t ret; 193 194 lockdep_assert_held(&inode->i_rwsem); 195 196 if (!(file->f_mode & FMODE_READ)) 197 return -EBADF; --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation |
|
From: Christoph H. <hc...@ls...> - 2017-06-16 06:18:50
|
Yeah, the iovec there should be a kvec.. |
|
From: Mimi Z. <zo...@li...> - 2017-06-09 18:04:06
|
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.
Since some files on these filesystems were previously
measured/appraised, not measuring them would change the PCR hash
value(s), possibly breaking userspace. For filesystems that do
not define the integrity_read file operation method and have a
read method, use the read method to calculate the file hash.
Signed-off-by: Mimi Zohar <zo...@li...>
---
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 c5489672b5aa..e3ef3fba16dc 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 iovec 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_init(&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
|
|
From: Mimi Z. <zo...@li...> - 2017-06-09 18:04:08
|
The large marjority of filesystems in the fs directory define
generic_file_read_iter as the read_iter file operation method.
Instead of specifying the integrity_read file operation method
for all of these file systems, continue to calculate the file
hash using the read_iter method, when defined as
generic_file_read_iter.
For all other read_iter methods, define an integrity_read
method.
Signed-off-by: Mimi Zohar <zo...@li...>
---
security/integrity/iint.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index e3ef3fba16dc..8164f57f5cea 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -202,6 +202,9 @@ int integrity_kernel_read(struct file *file, loff_t offset,
if (file->f_op->integrity_read) {
ret = file->f_op->integrity_read(&kiocb, &iter);
+ } else if (file->f_op->read_iter &&
+ file->f_op->read_iter == generic_file_read_iter) {
+ ret = file->f_op->read_iter(&kiocb, &iter);
} else if (file->f_op->read) {
mm_segment_t old_fs;
char __user *buf = (char __user *)addr;
--
2.7.4
|
|
From: Mimi Z. <zo...@li...> - 2017-06-09 19:47:35
|
|
From: Mimi Z. <zo...@li...> - 2017-06-09 19:56:52
|
On systems where IMA-appraisal is configured, the file system properly
labeled and the system booted with the "ima_tcb ima_appraise_tcb" boot
command line options, new files created by root will have a file hash
written out as security.ima.
This xfstests creates a file and compares the security.ima before and
after modifying the file. The results are compared with the "good"
file.
(For filesystems that are configured with IMA-appraisal, but aren't
labeled properly, boot the system with the "ima_appraise=tcb" boot
command line option as well.)
Mimi Zohar <zo...@li...>
---
tests/generic/440 | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/440.out | 13 ++++++++
tests/generic/group | 1 +
3 files changed, 103 insertions(+)
create mode 100755 tests/generic/440
create mode 100644 tests/generic/440.out
diff --git a/tests/generic/440 b/tests/generic/440
new file mode 100755
index 0000000..8616a48
--- /dev/null
+++ b/tests/generic/440
@@ -0,0 +1,89 @@
+#! /bin/bash
+# FS QA Test No. 440
+#
+# Tests IMA-appraisal
+# Derived from 062 tests
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+if [ "$FSTYP" = "btrfs" ]; then
+. ./common/btrfs
+elif [ "$FSTYP" = "xfs" ]; then
+. ./common/xfs
+fi
+
+_cleanup()
+{
+ cd /
+ echo; echo "*** unmount"
+ _scratch_unmount 2>/dev/null
+ rm -f $tmp.*
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+getfattr()
+{
+ $GETFATTR_PROG --absolute-names -dh $@ 2>&1 | _filter_scratch
+}
+
+setfattr()
+{
+ $SETFATTR_PROG $@ 2>&1 | _filter_scratch
+}
+
+_create_test_bed()
+{
+ echo "*** create temporary file"
+ echo "Hello" > $SCRATCH_MNT/hello.txt
+}
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+_require_scratch
+_require_attrs
+_require_command "$(which timeout)" "timeout"
+
+# real QA test starts here
+_scratch_mkfs > /dev/null 2>&1 || _fail "mkfs failed"
+_scratch_mount || _fail "mount failed"
+_create_test_bed
+
+xattr="security.ima"
+testfile="hello.txt"
+
+if [ ! -f $SCRATCH_MNT/$testfile ]; then
+ echo "File $testfile does not exist"
+ msleep 1
+fi
+
+echo "*** Reading $SCRATCH_MNT"
+timeout -s KILL 2 cat $SCRATCH_MNT/$testfile > /dev/null
+if [ $? -ne 0 ]; then
+ echo "Failed to read $SCRATCH_MNT/$testfile"
+fi
+
+echo "*** initial security.ima hash"
+getfattr -e hex -n $xattr $SCRATCH_MNT/$testfile
+
+echo " World!" >> $SCRATCH_MNT/$testfile
+
+echo "*** updated security.ima hash"
+getfattr -e hex -n $xattr $SCRATCH_MNT/$testfile
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/440.out b/tests/generic/440.out
new file mode 100644
index 0000000..a827377
--- /dev/null
+++ b/tests/generic/440.out
@@ -0,0 +1,13 @@
+QA output created by 440
+*** create temporary file
+*** Reading /mnt/scratch
+*** initial security.ima hash
+# file: SCRATCH_MNT/hello.txt
+security.ima=0x040466a045b452102c59d840ec097d59d9467e13a3f34f6494e539ffd32c1bb35f18
+
+*** updated security.ima hash
+# file: SCRATCH_MNT/hello.txt
+security.ima=0x0404cddd9990ad741e165a6a50990afe969c2233fc8794d027cdbf382f698a62a22f
+
+
+*** unmount
diff --git a/tests/generic/group b/tests/generic/group
index 5d3e4dc..c1ecc23 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -442,3 +442,4 @@
437 auto quick
438 auto
439 auto quick punch
+440 attr
--
2.9.3
|
|
From: Christoph H. <hc...@ls...> - 2017-06-13 06:47:35
|
Adding the fstests list..
On Fri, Jun 09, 2017 at 03:55:43PM -0400, Mimi Zohar wrote:
> On systems where IMA-appraisal is configured, the file system properly
> labeled and the system booted with the "ima_tcb ima_appraise_tcb" boot
> command line options, new files created by root will have a file hash
> written out as security.ima.
>
> This xfstests creates a file and compares the security.ima before and
> after modifying the file. The results are compared with the "good"
> file.
>
> (For filesystems that are configured with IMA-appraisal, but aren't
> labeled properly, boot the system with the "ima_appraise=tcb" boot
> command line option as well.)
>
> Mimi Zohar <zo...@li...>
> ---
> tests/generic/440 | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/440.out | 13 ++++++++
> tests/generic/group | 1 +
> 3 files changed, 103 insertions(+)
> create mode 100755 tests/generic/440
> create mode 100644 tests/generic/440.out
>
> diff --git a/tests/generic/440 b/tests/generic/440
> new file mode 100755
> index 0000000..8616a48
> --- /dev/null
> +++ b/tests/generic/440
> @@ -0,0 +1,89 @@
> +#! /bin/bash
> +# FS QA Test No. 440
> +#
> +# Tests IMA-appraisal
> +# Derived from 062 tests
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +
> +if [ "$FSTYP" = "btrfs" ]; then
> +. ./common/btrfs
> +elif [ "$FSTYP" = "xfs" ]; then
> +. ./common/xfs
> +fi
> +
> +_cleanup()
> +{
> + cd /
> + echo; echo "*** unmount"
> + _scratch_unmount 2>/dev/null
> + rm -f $tmp.*
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +getfattr()
> +{
> + $GETFATTR_PROG --absolute-names -dh $@ 2>&1 | _filter_scratch
> +}
> +
> +setfattr()
> +{
> + $SETFATTR_PROG $@ 2>&1 | _filter_scratch
> +}
> +
> +_create_test_bed()
> +{
> + echo "*** create temporary file"
> + echo "Hello" > $SCRATCH_MNT/hello.txt
> +}
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +
> +_require_scratch
> +_require_attrs
> +_require_command "$(which timeout)" "timeout"
> +
> +# real QA test starts here
> +_scratch_mkfs > /dev/null 2>&1 || _fail "mkfs failed"
> +_scratch_mount || _fail "mount failed"
> +_create_test_bed
> +
> +xattr="security.ima"
> +testfile="hello.txt"
> +
> +if [ ! -f $SCRATCH_MNT/$testfile ]; then
> + echo "File $testfile does not exist"
> + msleep 1
> +fi
> +
> +echo "*** Reading $SCRATCH_MNT"
> +timeout -s KILL 2 cat $SCRATCH_MNT/$testfile > /dev/null
> +if [ $? -ne 0 ]; then
> + echo "Failed to read $SCRATCH_MNT/$testfile"
> +fi
> +
> +echo "*** initial security.ima hash"
> +getfattr -e hex -n $xattr $SCRATCH_MNT/$testfile
> +
> +echo " World!" >> $SCRATCH_MNT/$testfile
> +
> +echo "*** updated security.ima hash"
> +getfattr -e hex -n $xattr $SCRATCH_MNT/$testfile
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/440.out b/tests/generic/440.out
> new file mode 100644
> index 0000000..a827377
> --- /dev/null
> +++ b/tests/generic/440.out
> @@ -0,0 +1,13 @@
> +QA output created by 440
> +*** create temporary file
> +*** Reading /mnt/scratch
> +*** initial security.ima hash
> +# file: SCRATCH_MNT/hello.txt
> +security.ima=0x040466a045b452102c59d840ec097d59d9467e13a3f34f6494e539ffd32c1bb35f18
> +
> +*** updated security.ima hash
> +# file: SCRATCH_MNT/hello.txt
> +security.ima=0x0404cddd9990ad741e165a6a50990afe969c2233fc8794d027cdbf382f698a62a22f
> +
> +
> +*** unmount
> diff --git a/tests/generic/group b/tests/generic/group
> index 5d3e4dc..c1ecc23 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -442,3 +442,4 @@
> 437 auto quick
> 438 auto
> 439 auto quick punch
> +440 attr
> --
> 2.9.3
---end quoted text---
|
|
From: Christoph H. <hc...@ls...> - 2017-06-13 06:47:06
|
A strong and a weak NAK on this. For one thing you should not call ->read for fs code at all - use read_iter where it fits (it does here) or the kernel_read() helper otherwise. But once again I don't think this is correct - it's a potentially unsafe default, so please wire up the file systems actually tested and known to work manually. E.g. this does the wrong thing for at least NFS and OCFS2. |
|
From: Mimi Z. <zo...@li...> - 2017-06-13 14:18:03
|
On Tue, 2017-06-13 at 08:46 +0200, Christoph Hellwig wrote: > A strong and a weak NAK on this. For one thing you should not > call ->read for fs code at all - use read_iter where it fits > (it does here) or the kernel_read() helper otherwise. Calling ->read directly is intentional. Commit C0430e49b6e7c "ima: introduce ima_kernel_read()" replaced the call to kernel_read with ima_kernel_read(), the non-security checking version of kernel_read(). Subsequently, commit e3c4abbfa97e "integrity: define a new function integrity_read_file()" renamed ima_read_file() to integrity_read_file(). > But once again I don't think this is correct - it's a potentially > unsafe default, so please wire up the file systems actually tested > and known to work manually. > > E.g. this does the wrong thing for at least NFS and OCFS2. Both NFS and OCFS define their own specific read_iter(), nfs_file_read() and ocfs2_file_read_iter() respectively. As these file systems have not yet been converted to use ->read_integrity, the xfstests fail. Mimi |
|
From: Christoph H. <hc...@ls...> - 2017-06-13 14:22:20
|
On Tue, Jun 13, 2017 at 10:17:45AM -0400, Mimi Zohar wrote: > Calling ->read directly is intentional. Commit C0430e49b6e7c "ima: > introduce ima_kernel_read()" replaced the call to kernel_read with > ima_kernel_read(), the non-security checking version of kernel_read(). > Subsequently, commit e3c4abbfa97e "integrity: define a new function > integrity_read_file()" renamed ima_read_file() to > integrity_read_file(). Again, the point is you should not call ->read for in-kernel reads. > Both NFS and OCFS define their own specific read_iter(), > nfs_file_read() and ocfs2_file_read_iter() respectively. As these > file systems have not yet been converted to use ->read_integrity, the > xfstests fail. So they will need to be converted. The xfstests will not just fail, it will deadlock the calling process with this code. |
|
From: Mimi Z. <zo...@li...> - 2017-06-13 15:07:46
|
On Tue, 2017-06-13 at 16:22 +0200, Christoph Hellwig wrote:
> On Tue, Jun 13, 2017 at 10:17:45AM -0400, Mimi Zohar wrote:
> > Calling ->read directly is intentional. Commit C0430e49b6e7c "ima:
> > introduce ima_kernel_read()" replaced the call to kernel_read with
> > ima_kernel_read(), the non-security checking version of kernel_read().
> > Subsequently, commit e3c4abbfa97e "integrity: define a new function
> > integrity_read_file()" renamed ima_read_file() to
> > integrity_read_file().
>
> Again, the point is you should not call ->read for in-kernel reads.
Ok, there was a reason for restoring this behavior. Perhaps,
something that was previously being measured wasn't being measured
without it. Looking ...
> > Both NFS and OCFS define their own specific read_iter(),
> > nfs_file_read() and ocfs2_file_read_iter() respectively. As these
> > file systems have not yet been converted to use ->read_integrity, the
> > xfstests fail.
>
> So they will need to be converted. The xfstests will not just fail,
> it will deadlock the calling process with this code.
Right, process_measurement() is fail safe, meaning that any file,
which matches a rule in the IMA policy, that isn't appraised properly
fails.
from ima_main: process_measurement(
out:
inode_unlock(inode);
if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
return -EACCES;
return 0;
With your patch, we now need to take into consideration that the file
system doesn't support IMA-appraisal. We can't just change the
existing behavior, so we will probably need the ability to override
the fail safe behavior for file systems that do not support IMA.
The bigger problem is that files that were previously measured, might
now not be measured, without any indication in the audit logs or the
IMA measurement list.
Mimi
|
|
From: Christoph H. <hc...@ls...> - 2017-06-14 07:03:33
|
On Tue, Jun 13, 2017 at 11:07:29AM -0400, Mimi Zohar wrote: > The bigger problem is that files that were previously measured, might > now not be measured, without any indication in the audit logs or the > IMA measurement list. And that's exactly what I've been preaching for a long time - you need to decide on what your requirements for IMA are and check for them when enabling it, not just have things sort of work or not at runtime. |
|
From: Dmitry K. <dmi...@gm...> - 2017-07-10 14:03:52
|
On Fri, Jun 9, 2017 at 9:02 PM, Mimi Zohar <zo...@li...> wrote:
> 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.
>
> Since some files on these filesystems were previously
> measured/appraised, not measuring them would change the PCR hash
> value(s), possibly breaking userspace. For filesystems that do
> not define the integrity_read file operation method and have a
> read method, use the read method to calculate the file hash.
>
> Signed-off-by: Mimi Zohar <zo...@li...>
> ---
> 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 c5489672b5aa..e3ef3fba16dc 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 iovec 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_init(&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);
> + }
Hi,
Original code was using "__vfs_read()".
Patch 1 replaced use of it by ->integrity_read.
This patch re-added f_op->read() directly...
While __vfs_read() did it differently
if (file->f_op->read)
return file->f_op->read(file, buf, count, pos);
else if (file->f_op->read_iter)
return new_sync_read(file, buf, count, pos);
Is avoiding use of __vfs_read() is intentional?
Dmitry
> +
> BUG_ON(ret == -EIOCBQUEUED);
> return ret;
> }
> --
> 2.7.4
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel
--
Thanks,
Dmitry
|
|
From: Mimi Z. <zo...@li...> - 2017-07-10 15:14:10
|
On Mon, 2017-07-10 at 17:03 +0300, Dmitry Kasatkin wrote:
> On Fri, Jun 9, 2017 at 9:02 PM, Mimi Zohar <zo...@li...> wrote:
> > 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.
> >
> > Since some files on these filesystems were previously
> > measured/appraised, not measuring them would change the PCR hash
> > value(s), possibly breaking userspace. For filesystems that do
> > not define the integrity_read file operation method and have a
> > read method, use the read method to calculate the file hash.
> >
> > Signed-off-by: Mimi Zohar <zo...@li...>
> > ---
> > 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 c5489672b5aa..e3ef3fba16dc 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 iovec 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_init(&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);
> > + }
>
> Hi,
>
> Original code was using "__vfs_read()".
> Patch 1 replaced use of it by ->integrity_read.
> This patch re-added f_op->read() directly...
>
> While __vfs_read() did it differently
>
> if (file->f_op->read)
> return file->f_op->read(file, buf, count, pos);
> else if (file->f_op->read_iter)
> return new_sync_read(file, buf, count, pos);
>
>
> Is avoiding use of __vfs_read() is intentional?
Yes, some filesystems ->read_iter attempt to take the i_rwsem, which
IMA has already taken. This patch set introduces a new file operation
method ->integrity_read, which reads the file without re-taking the
lock. (This method should probably be renamed ->integrity_read_iter.)
The next version of this patch set will remove this patch, unless
someone provides a reason for needing it. At that point, a new method
equivalent to ->read would need to be defined.
Mimi
|
|
Re: [Linux-ima-devel] [PATCH 4/4] ima: use read_iter
(generic_file_read_iter) to calculate file hash
From: Dmitry K. <dmi...@gm...> - 2017-07-10 14:07:54
|
On Fri, Jun 9, 2017 at 9:02 PM, Mimi Zohar <zo...@li...> wrote:
> The large marjority of filesystems in the fs directory define
> generic_file_read_iter as the read_iter file operation method.
>
> Instead of specifying the integrity_read file operation method
> for all of these file systems, continue to calculate the file
> hash using the read_iter method, when defined as
> generic_file_read_iter.
>
> For all other read_iter methods, define an integrity_read
> method.
>
> Signed-off-by: Mimi Zohar <zo...@li...>
> ---
> security/integrity/iint.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index e3ef3fba16dc..8164f57f5cea 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -202,6 +202,9 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>
> if (file->f_op->integrity_read) {
> ret = file->f_op->integrity_read(&kiocb, &iter);
> + } else if (file->f_op->read_iter &&
> + file->f_op->read_iter == generic_file_read_iter) {
> + ret = file->f_op->read_iter(&kiocb, &iter);
> } else if (file->f_op->read) {
> mm_segment_t old_fs;
> char __user *buf = (char __user *)addr;
Why not __vfs_read()?? it uses new_sync_read()
else if (file->f_op->read_iter)
return new_sync_read(file, buf, count, pos);
> --
> 2.7.4
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel
--
Thanks,
Dmitry
|
|
Re: [Linux-ima-devel] [PATCH 4/4] ima: use read_iter
(generic_file_read_iter) to calculate file hash
From: Mimi Z. <zo...@li...> - 2017-07-10 15:22:46
|
On Mon, 2017-07-10 at 17:07 +0300, Dmitry Kasatkin wrote:
> On Fri, Jun 9, 2017 at 9:02 PM, Mimi Zohar <zo...@li...> wrote:
> > The large marjority of filesystems in the fs directory define
> > generic_file_read_iter as the read_iter file operation method.
> >
> > Instead of specifying the integrity_read file operation method
> > for all of these file systems, continue to calculate the file
> > hash using the read_iter method, when defined as
> > generic_file_read_iter.
> >
> > For all other read_iter methods, define an integrity_read
> > method.
> >
> > Signed-off-by: Mimi Zohar <zo...@li...>
> > ---
> > security/integrity/iint.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > index e3ef3fba16dc..8164f57f5cea 100644
> > --- a/security/integrity/iint.c
> > +++ b/security/integrity/iint.c
> > @@ -202,6 +202,9 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> >
> > if (file->f_op->integrity_read) {
> > ret = file->f_op->integrity_read(&kiocb, &iter);
> > + } else if (file->f_op->read_iter &&
> > + file->f_op->read_iter == generic_file_read_iter) {
> > + ret = file->f_op->read_iter(&kiocb, &iter);
> > } else if (file->f_op->read) {
> > mm_segment_t old_fs;
> > char __user *buf = (char __user *)addr;
>
> Why not __vfs_read()?? it uses new_sync_read()
and that calls read_sync_iter(), which calls ->read_iter. Is there a
problem with directly calling ->integrity_read instead?
Mimi
> else if (file->f_op->read_iter)
> return new_sync_read(file, buf, count, pos);
>
>
|
|
From: Mimi Z. <zo...@li...> - 2017-07-13 13:55:28
|
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, introduced 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 defined a new policy "action" named "dont_failsafe" and a new builtin policy named "fs_unsafe", which can be specified on the boot command line. Change log v3: - define simple_read_iter_from_buffer - replace the existing efivarfs ->read method with ->read_iter method. - squashed other fs definitions of ->integrity_read with this patch. - include dont_failsafe rule when displaying policy. - fail attempt to add dont_failsafe rule when appending to the policy. - moved '---' divider before change log, as requested in review. Mimi Christoph Hellwig (1): ima: use fs method to read integrity data Mimi Zohar (3): ima: always measure and audit files in policy ima: define "dont_failsafe" policy action rule ima: define "fs_unsafe" builtin policy Documentation/ABI/testing/ima_policy | 3 +- Documentation/admin-guide/kernel-parameters.txt | 8 ++++- fs/btrfs/file.c | 1 + fs/efivarfs/file.c | 12 +++++--- 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/libfs.c | 32 +++++++++++++++++++ fs/nilfs2/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 | 3 ++ mm/shmem.c | 1 + security/integrity/iint.c | 20 ++++++++---- 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 | 41 ++++++++++++++++++++++++- 24 files changed, 158 insertions(+), 19 deletions(-) -- 2.7.4 |
|
From: Mimi Z. <zo...@li...> - 2017-07-13 13:55:34
|
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 can not 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 <zo...@li...>
---
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
|
|
From: Mimi Z. <zo...@li...> - 2017-07-13 13:55:46
|
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 <zo...@li...>
---
Changelog v3:
- include dont_failsafe rule when displaying policy
- fail attempt to add dont_failsafe rule when appending to the policy
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 | 29 ++++++++++++++++++++++++++++-
4 files changed, 41 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..3c29aa4e7980 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -40,12 +40,14 @@
#define APPRAISE 0x0004 /* same as IMA_APPRAISE */
#define DONT_APPRAISE 0x0008
#define AUDIT 0x0040
+#define DONT_FAILSAFE 0x0400
#define INVALID_PCR(a) (((a) < 0) || \
(a) >= (FIELD_SIZEOF(struct integrity_iint_cache, measured_pcrs) * 8))
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 +515,9 @@ void ima_update_policy(void)
if (ima_rules != policy) {
ima_policy_flag = 0;
ima_rules = policy;
+
+ /* Only update on initial policy replacement, not append */
+ set_failsafe(temp_failsafe);
}
ima_update_policy_flag();
}
@@ -529,7 +534,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 +565,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}
};
@@ -630,6 +636,11 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
if ((*p == '\0') || (*p == ' ') || (*p == '\t'))
continue;
token = match_token(p, policy_tokens, args);
+ if (entry->action == DONT_FAILSAFE) {
+ /* no args permitted, force invalid rule */
+ token = Opt_dont_failsafe;
+ }
+
switch (token) {
case Opt_measure:
ima_log_string(ab, "action", "measure");
@@ -671,6 +682,19 @@ 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");
+
+ if (entry->action != UNKNOWN)
+ result = -EINVAL;
+
+ /* Permit on initial policy replacement only */
+ if (ima_rules != &ima_policy_rules)
+ temp_failsafe = 0;
+ else
+ result = -EINVAL;
+ entry->action = DONT_FAILSAFE;
+ break;
case Opt_func:
ima_log_string(ab, "func", args[0].from);
@@ -951,6 +975,7 @@ void ima_delete_rules(void)
int i;
temp_ima_appraise = 0;
+ temp_failsafe = 1;
list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) {
for (i = 0; i < MAX_LSM_RULES; i++)
kfree(entry->lsm[i].args_p);
@@ -1042,6 +1067,8 @@ int ima_policy_show(struct seq_file *m, void *v)
seq_puts(m, pt(Opt_dont_appraise));
if (entry->action & AUDIT)
seq_puts(m, pt(Opt_audit));
+ if (entry->action & DONT_FAILSAFE)
+ seq_puts(m, pt(Opt_dont_failsafe));
seq_puts(m, " ");
--
2.7.4
|
|
From: Mimi Z. <zo...@li...> - 2017-07-13 13:55:47
|
Permit normally denied access/execute permission for files in policy
on IMA unsupported filesystems. This patch defines "fs_unsafe", a
builtin policy.
Mimi Zohar <zo...@li...>
Changelog v3:
- include dont_failsafe rule when displaying policy
---
Documentation/admin-guide/kernel-parameters.txt | 8 +++++++-
security/integrity/ima/ima_policy.c | 12 ++++++++++++
2 files changed, 19 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 3c29aa4e7980..6b77b890ff96 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -169,6 +169,10 @@ static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
.flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
};
+static struct ima_rule_entry dont_failsafe_rules[] __ro_after_init = {
+ {.action = DONT_FAILSAFE}
+};
+
static LIST_HEAD(ima_default_rules);
static LIST_HEAD(ima_policy_rules);
static LIST_HEAD(ima_temp_rules);
@@ -188,6 +192,7 @@ __setup("ima_tcb", default_measure_policy_setup);
static bool ima_use_appraise_tcb __initdata;
static bool ima_use_secure_boot __initdata;
+static bool ima_use_dont_failsafe __initdata;
static int __init policy_setup(char *str)
{
char *p;
@@ -201,6 +206,10 @@ 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) {
+ ima_use_dont_failsafe = 1;
+ set_failsafe(0);
+ }
}
return 1;
@@ -470,6 +479,9 @@ void __init ima_init_policy(void)
temp_ima_appraise |= IMA_APPRAISE_POLICY;
}
+ if (ima_use_dont_failsafe)
+ list_add_tail(&dont_failsafe_rules[0].list, &ima_default_rules);
+
ima_rules = &ima_default_rules;
ima_update_policy_flag();
}
--
2.7.4
|
|
From: Mimi Z. <zo...@li...> - 2017-07-13 13:55:48
|
From: Christoph Hellwig <hc...@ls...>
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.
Signed-off-by: Christoph Hellwig <hc...@ls...>
Cc: Matthew Garrett <mat...@ne...>
Cc: Jan Kara <ja...@su...>
Cc: "Theodore Ts'o" <ty...@mi...>
Cc: Andreas Dilger <adi...@di...>
Cc: Jaegeuk Kim <ja...@ke...>
Cc: Chao Yu <yu...@hu...>
Cc: Steven Whitehouse <swh...@re...>
Cc: Bob Peterson <rpe...@re...>
Cc: David Woodhouse <dw...@in...>
Cc: Dave Kleikamp <sh...@ke...>
Cc: Ryusuke Konishi <kon...@la...>
Cc: Mark Fasheh <mf...@ve...>
Cc: Joel Becker <jl...@ev...>
Cc: Richard Weinberger <ri...@no...>
Cc: "Darrick J. Wong" <dar...@or...>
Cc: Hugh Dickins <hu...@go...>
Cc: Chris Mason <cl...@fb...>
Signed-off-by: Mimi Zohar <zo...@li...>
---
Changelog v3:
- define simple_read_iter_from_buffer as suggested by Al Viro
- replace the existing efivarfs ->read method with ->read_iter method.
- squashed other fs definitions of ->integrity_read with this patch.
- Cc'ing maintainers
- moved '---' divider before change log, as requested in patch review
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)
fs/btrfs/file.c | 1 +
fs/efivarfs/file.c | 12 +++++++-----
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/libfs.c | 32 ++++++++++++++++++++++++++++++++
fs/nilfs2/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 | 3 +++
mm/shmem.c | 1 +
security/integrity/iint.c | 20 ++++++++++++++------
18 files changed, 91 insertions(+), 11 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/efivarfs/file.c b/fs/efivarfs/file.c
index 5f22e74bbade..17955a92a5b3 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -64,9 +64,10 @@ static ssize_t efivarfs_file_write(struct file *file,
return bytes;
}
-static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
- size_t count, loff_t *ppos)
+static ssize_t efivarfs_file_read_iter(struct kiocb *iocb,
+ struct iov_iter *iter)
{
+ struct file *file = iocb->ki_filp;
struct efivar_entry *var = file->private_data;
unsigned long datasize = 0;
u32 attributes;
@@ -96,8 +97,8 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
goto out_free;
memcpy(data, &attributes, sizeof(attributes));
- size = simple_read_from_buffer(userbuf, count, ppos,
- data, datasize + sizeof(attributes));
+ size = simple_read_iter_from_buffer(iocb, iter, data,
+ datasize + sizeof(attributes));
out_free:
kfree(data);
@@ -174,8 +175,9 @@ efivarfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long p)
const struct file_operations efivarfs_file_operations = {
.open = simple_open,
- .read = efivarfs_file_read,
+ .read_iter = efivarfs_file_read_iter,
.write = efivarfs_file_write,
.llseek = no_llseek,
.unlocked_ioctl = efivarfs_file_ioctl,
+ .integrity_read = efivarfs_file_read_iter,
};
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/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/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/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 */
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/libfs.c b/fs/libfs.c
index a04395334bb1..e1b4f8695013 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -16,6 +16,7 @@
#include <linux/exportfs.h>
#include <linux/writeback.h>
#include <linux/buffer_head.h> /* sync_mapping_buffers */
+#include <linux/uio.h>
#include <linux/uaccess.h>
@@ -676,6 +677,37 @@ ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
EXPORT_SYMBOL(simple_write_to_buffer);
/**
+ * simple_read_iter_from_buffer - copy data from the buffer to user space
+ * @iocb: struct containing the file, the current position and other info
+ * @to: the user space buffer to read to
+ * @from: the buffer to read from
+ * @available: the size of the buffer
+ *
+ * The simple_read_iter_from_buffer() function reads up to @available bytes
+ * from the current buffer into the user space buffer.
+ *
+ * On success, the current buffer offset is advanced by the number of bytes
+ * read, or a negative value is returned on error.
+ **/
+ssize_t simple_read_iter_from_buffer(struct kiocb *iocb, struct iov_iter *to,
+ const void *from, size_t available)
+{
+ loff_t pos = iocb->ki_pos;
+ size_t ret;
+
+ if (pos < 0)
+ return -EINVAL;
+ if (pos >= available)
+ return 0;
+ ret = copy_to_iter(from + pos, available - pos, to);
+ if (!ret && iov_iter_count(to))
+ return -EFAULT;
+ iocb->ki_pos = pos + ret;
+ return ret;
+}
+EXPORT_SYMBOL(simple_read_iter_from_buffer);
+
+/**
* memory_read_from_buffer - copy data from the buffer
* @to: the kernel space buffer to read to
* @count: the maximum number of bytes to read
diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
index c5fa3dee72fc..55e058ac487f 100644
--- a/fs/nilfs2/file.c
+++ b/fs/nilfs2/file.c
@@ -150,6 +150,7 @@ const struct file_operations nilfs_file_operations = {
/* .release = nilfs_release_file, */
.fsync = nilfs_sync_file,
.splice_read = generic_file_splice_read,
+ .integrity_read = generic_file_read_iter,
};
const struct inode_operations nilfs_file_inode_operations = {
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 = {
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 = {
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,
};
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..d85d2c43afd9 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 {
@@ -3011,6 +3012,8 @@ extern void simple_release_fs(struct vfsmount **mount, int *count);
extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
loff_t *ppos, const void *from, size_t available);
+extern ssize_t simple_read_iter_from_buffer(struct kiocb *iocb,
+ struct iov_iter *to, const void *from, size_t available);
extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
const void __user *from, size_t count);
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
};
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
|
|
From: Jan K. <ja...@su...> - 2017-07-13 16:03:20
|
On Thu 13-07-17 09:54:48, Mimi Zohar wrote:
> 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/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,
> };
I think both ext2 and ext4 need a bit more special handling (similarly to
XFS) due to DAX. E.g. ext4_dax_read_iter() will try to get i_rwsem which is
wrong for integrity_read handler as far as I understand.
> 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/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 = {
For cluster filesystems like gfs2 or ocfs2 I actually wonder whether IMA
works as it should - without special cluster locking another node may be
modifying the file while you read it even when you hold i_rwsem. So don't
these filesystems need some special treatment?
Honza
--
Jan Kara <ja...@su...>
SUSE Labs, CR
|
Thanks for helping keep SourceForge clean.
X