[PATCH v6 0/6] define new fs integrity_read method

[PATCH v6 0/6] define new fs integrity_read method

[Mimi Zohar]

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 of 0's, instead
of the actual file hash, to indicate that the file hash could not
be calculated.  Like for any other file signature verification
error, file access/execute permission is denied.

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.

Direct Access:
Although the new integrity_read method works for files opened with
the "O_DIRECT" flag on block devices that support DAX, for consistency
fail filesystems mounted with/without DAX.  (Refer to commit f9b2a735bddd
"ima: audit log files opened with O_DIRECT flag" for detals.)

Changelog v6:
- Defined simple_read_iter_from_buffer() as a separate patch.
- Made efivarfs usage of simple_read_iter_from_buffer() a separate patch.

Changelog v5:
- fail files opened O_DIRECT, but include access attempt in measurement
list.
- removed ocf2 and gfs2 integrity_read support.

Changelog v4:
- define ext2/4 specific ->integrity_read functions based Jan Kara's
review.
- properly fail file open with O_DIRECT on filesystems not mounted
with "-o dax".
- remove the "permit_directio" IMA policy option.

Changelog 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):
*** BLURB HERE ***

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

Mimi Zohar (5):
  libfs: define simple_read_iter_from_buffer
  efivarfs: replaces the read file operation with read_iter
  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                                  | 17 +++++++
 fs/ext4/file.c                                  | 20 ++++++++
 fs/f2fs/file.c                                  |  1 +
 fs/jffs2/file.c                                 |  1 +
 fs/jfs/file.c                                   |  1 +
 fs/libfs.c                                      | 32 ++++++++++++
 fs/nilfs2/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                | 67 ++++++++++++++++---------
 security/integrity/ima/ima_crypto.c             | 10 ++++
 security/integrity/ima/ima_main.c               | 19 ++++---
 security/integrity/ima/ima_policy.c             | 41 ++++++++++++++-
 23 files changed, 238 insertions(+), 45 deletions(-)

-- 
2.7.4

[PATCH v6 0/6] define new fs integrity_read method

[Mimi Zohar]

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 of 0's, instead
of the actual file hash, to indicate that the file hash could not
be calculated.  Like for any other file signature verification
error, file access/execute permission is denied.

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.

Direct Access:
Although the new integrity_read method works for files opened with
the "O_DIRECT" flag on block devices that support DAX, for consistency
fail filesystems mounted with/without DAX.  (Refer to commit f9b2a735bddd
"ima: audit log files opened with O_DIRECT flag" for detals.)

Changelog v6:
- Defined simple_read_iter_from_buffer() as a separate patch.
- Made efivarfs usage of simple_read_iter_from_buffer() a separate patch.

Changelog v5:
- fail files opened O_DIRECT, but include access attempt in measurement
list.
- removed ocf2 and gfs2 integrity_read support.

Changelog v4:
- define ext2/4 specific ->integrity_read functions based Jan Kara's
review.
- properly fail file open with O_DIRECT on filesystems not mounted
with "-o dax".
- remove the "permit_directio" IMA policy option.

Changelog 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):
*** BLURB HERE ***

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

Mimi Zohar (5):
  libfs: define simple_read_iter_from_buffer
  efivarfs: replaces the read file operation with read_iter
  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                                  | 17 +++++++
 fs/ext4/file.c                                  | 20 ++++++++
 fs/f2fs/file.c                                  |  1 +
 fs/jffs2/file.c                                 |  1 +
 fs/jfs/file.c                                   |  1 +
 fs/libfs.c                                      | 32 ++++++++++++
 fs/nilfs2/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                | 67 ++++++++++++++++---------
 security/integrity/ima/ima_crypto.c             | 10 ++++
 security/integrity/ima/ima_main.c               | 19 ++++---
 security/integrity/ima/ima_policy.c             | 41 ++++++++++++++-
 23 files changed, 238 insertions(+), 45 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

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Mimi Zohar]

In preparation for defining an integrity_read file operation
method for efivarfs, define a simple_read_iter_from_buffer()
function.

(Based on Al's code as posted in thread.)

Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>

---
Changelog v6:
- defined as a separate patch
 
 fs/libfs.c         | 32 ++++++++++++++++++++++++++++++++
 include/linux/fs.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/fs/libfs.c b/fs/libfs.c
index 3aabe553fc45..b6e304c6828b 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/include/linux/fs.h b/include/linux/fs.h
index 6e1fd5d21248..fdec9b763b54 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3097,6 +3097,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);
 
-- 
2.7.4

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Mimi Zohar]

In preparation for defining an integrity_read file operation
method for efivarfs, define a simple_read_iter_from_buffer()
function.

(Based on Al's code as posted in thread.)

Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>

---
Changelog v6:
- defined as a separate patch
 
 fs/libfs.c         | 32 ++++++++++++++++++++++++++++++++
 include/linux/fs.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/fs/libfs.c b/fs/libfs.c
index 3aabe553fc45..b6e304c6828b 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/include/linux/fs.h b/include/linux/fs.h
index 6e1fd5d21248..fdec9b763b54 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3097,6 +3097,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);
 
-- 
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

Re: [PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
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

Re: [PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Mimi Zohar]

On Wed, 2017-08-16 at 08:35 +0200, Christoph Hellwig wrote:
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thank you for the reviewed-by's.

Up to now I haven't been removing the Changelog before sending James a
pull request.  Adding the dashes in the commit itself, only changes
how the patches are applied by others to their local branch, not to
what would be upstreamed.  Am I suppose to be removing the changelog
before sending a pull request?

thanks,

Mimi

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Mimi Zohar]

On Wed, 2017-08-16 at 08:35 +0200, Christoph Hellwig wrote:
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thank you for the reviewed-by's.

Up to now I haven't been removing the Changelog before sending James a
pull request.??Adding the dashes in the commit itself, only changes
how the patches are applied by others to their local branch, not to
what would be upstreamed.??Am I suppose to be removing the changelog
before sending a pull request?

thanks,

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

Re: [PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[James Morris]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

On Wed, 16 Aug 2017, Mimi Zohar wrote:

> On Wed, 2017-08-16 at 08:35 +0200, Christoph Hellwig wrote:
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Thank you for the reviewed-by's.
> 
> Up to now I haven't been removing the Changelog before sending James a
> pull request.  Adding the dashes in the commit itself, only changes
> how the patches are applied by others to their local branch, not to
> what would be upstreamed.  Am I suppose to be removing the changelog
> before sending a pull request?

I don't really understand this, but leave the changelog in?


-- 
James Morris
<jmorris@namei.org>

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[James Morris]

On Wed, 16 Aug 2017, Mimi Zohar wrote:

> On Wed, 2017-08-16 at 08:35 +0200, Christoph Hellwig wrote:
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Thank you for the reviewed-by's.
> 
> Up to now I haven't been removing the Changelog before sending James a
> pull request.??Adding the dashes in the commit itself, only changes
> how the patches are applied by others to their local branch, not to
> what would be upstreamed.??Am I suppose to be removing the changelog
> before sending a pull request?

I don't really understand this, but leave the changelog in?


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Mimi Zohar]

On Thu, 2017-08-17 at 12:42 +1000, James Morris wrote:
> On Wed, 16 Aug 2017, Mimi Zohar wrote:
> 
> > On Wed, 2017-08-16 at 08:35 +0200, Christoph Hellwig wrote:
> > > Looks good,
> > > 
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > Thank you for the reviewed-by's.
> > 
> > Up to now I haven't been removing the Changelog before sending James a
> > pull request.  Adding the dashes in the commit itself, only changes
> > how the patches are applied by others to their local branch, not to
> > what would be upstreamed.  Am I suppose to be removing the changelog
> > before sending a pull request?
> 
> I don't really understand this, but leave the changelog in?

Christoph had commented that the "---" should be before the Changelog,
not afterwards, so that git-am skips them.  The changelog is needed by
reviewers, but once upstreamed it isn't really needed any more.  Based
on Christph's response (offline), any information in the changelog
that is still useful, should move into the commit description itself
or code.

Mimi

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Mimi Zohar]

On Thu, 2017-08-17 at 12:42 +1000, James Morris wrote:
> On Wed, 16 Aug 2017, Mimi Zohar wrote:
> 
> > On Wed, 2017-08-16 at 08:35 +0200, Christoph Hellwig wrote:
> > > Looks good,
> > > 
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > Thank you for the reviewed-by's.
> > 
> > Up to now I haven't been removing the Changelog before sending James a
> > pull request.??Adding the dashes in the commit itself, only changes
> > how the patches are applied by others to their local branch, not to
> > what would be upstreamed.??Am I suppose to be removing the changelog
> > before sending a pull request?
> 
> I don't really understand this, but leave the changelog in?

Christoph had commented that the "---" should be before the Changelog,
not afterwards, so that git-am skips them. ?The changelog is needed by
reviewers, but once upstreamed it isn't really needed any more. ?Based
on Christph's response (offline), any information in the changelog
that is still useful, should move into the commit description itself
or code.

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

Re: [PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Dmitry Kasatkin]

Looks good to me.


On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> In preparation for defining an integrity_read file operation
> method for efivarfs, define a simple_read_iter_from_buffer()
> function.
>
> (Based on Al's code as posted in thread.)
>
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
>
> ---
> Changelog v6:
> - defined as a separate patch
>
>  fs/libfs.c         | 32 ++++++++++++++++++++++++++++++++
>  include/linux/fs.h |  2 ++
>  2 files changed, 34 insertions(+)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 3aabe553fc45..b6e304c6828b 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/include/linux/fs.h b/include/linux/fs.h
> index 6e1fd5d21248..fdec9b763b54 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3097,6 +3097,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);
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,
Dmitry

[PATCH v6 1/6] libfs: define simple_read_iter_from_buffer

[Dmitry Kasatkin]

Looks good to me.


On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> In preparation for defining an integrity_read file operation
> method for efivarfs, define a simple_read_iter_from_buffer()
> function.
>
> (Based on Al's code as posted in thread.)
>
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
>
> ---
> Changelog v6:
> - defined as a separate patch
>
>  fs/libfs.c         | 32 ++++++++++++++++++++++++++++++++
>  include/linux/fs.h |  2 ++
>  2 files changed, 34 insertions(+)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 3aabe553fc45..b6e304c6828b 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/include/linux/fs.h b/include/linux/fs.h
> index 6e1fd5d21248..fdec9b763b54 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3097,6 +3097,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);
>
> --
> 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



-- 
Thanks,
Dmitry
--
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

[PATCH v6 2/6] efivarfs: replaces the read file operation with read_iter

[Mimi Zohar]

In preparation for defining the integrity_read file operation
method, replace the read file operation method with read_iter.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>

---
Changelog v6:
- Defined as a separate patch.
 
 fs/efivarfs/file.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index 5f22e74bbade..863f1b100165 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,7 +175,7 @@ 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,
-- 
2.7.4

[PATCH v6 2/6] efivarfs: replaces the read file operation with read_iter

[Mimi Zohar]

In preparation for defining the integrity_read file operation
method, replace the read file operation method with read_iter.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>

---
Changelog v6:
- Defined as a separate patch.
 
 fs/efivarfs/file.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index 5f22e74bbade..863f1b100165 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,7 +175,7 @@ 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,
-- 
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

Re: [PATCH v6 2/6] efivarfs: replaces the read file operation with read_iter

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v6 2/6] efivarfs: replaces the read file operation with read_iter

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
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

[PATCH v6 3/6] ima: always measure and audit files in policy

[Mimi Zohar]

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.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
Changelog v6:
- replace "?:" with if/then
- annotate i_version usage
- reword O_DIRECT comment

Changelog v5:
- Fail files opened O_DIRECT, but include attempt in measurement list.

Changelog v4:
- Based on both -EBADF and -EINVAL
- clean up ima_collect_measurement()

 security/integrity/ima/ima_api.c    | 67 +++++++++++++++++++++++--------------
 security/integrity/ima/ima_crypto.c | 10 ++++++
 security/integrity/ima/ima_main.c   |  7 ++--
 3 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c2edba8de35e..1dee695642a4 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -199,42 +199,59 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	struct inode *inode = file_inode(file);
 	const char *filename = file->f_path.dentry->d_name.name;
 	int result = 0;
+	int length;
+	void *tmpbuf;
+	u64 i_version;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash;
 
-	if (!(iint->flags & IMA_COLLECTED)) {
-		u64 i_version = file_inode(file)->i_version;
+	if (iint->flags & IMA_COLLECTED)
+		goto out;
 
-		if (file->f_flags & O_DIRECT) {
-			audit_cause = "failed(directio)";
-			result = -EACCES;
-			goto out;
-		}
+	/*
+	 * Dectecting file change is based on i_version. On filesystems
+	 * which do not support i_version, support is limited to an initial
+	 * measurement/appraisal/audit.
+	 */
+	i_version = file_inode(file)->i_version;
+	hash.hdr.algo = algo;
 
-		hash.hdr.algo = algo;
-
-		result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
-			ima_calc_buffer_hash(buf, size, &hash.hdr);
-		if (!result) {
-			int length = sizeof(hash.hdr) + hash.hdr.length;
-			void *tmpbuf = krealloc(iint->ima_hash, length,
-						GFP_NOFS);
-			if (tmpbuf) {
-				iint->ima_hash = tmpbuf;
-				memcpy(iint->ima_hash, &hash, length);
-				iint->version = i_version;
-				iint->flags |= IMA_COLLECTED;
-			} else
-				result = -ENOMEM;
-		}
+	/* Initialize hash digest to 0's in case of failure */
+	memset(&hash.digest, 0, sizeof(hash.digest));
+
+	if (buf)
+		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
+	else
+		result = ima_calc_file_hash(file, &hash.hdr);
+
+	if (result && result != -EBADF && result != -EINVAL)
+		goto out;
+
+	length = sizeof(hash.hdr) + hash.hdr.length;
+	tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
+	if (!tmpbuf) {
+		result = -ENOMEM;
+		goto out;
 	}
+
+	iint->ima_hash = tmpbuf;
+	memcpy(iint->ima_hash, &hash, length);
+	iint->version = i_version;
+
+	/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
+	if (result != -EBADF && result != -EINVAL)
+		iint->flags |= IMA_COLLECTED;
 out:
-	if (result)
+	if (result) {
+		if (file->f_flags & O_DIRECT)
+			audit_cause = "failed(directio)";
+
 		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
 				    filename, "collect_data", audit_cause,
 				    result, 0);
+	}
 	return result;
 }
 
@@ -278,7 +295,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 	}
 
 	result = ima_store_template(entry, violation, inode, filename, pcr);
-	if (!result || result == -EEXIST) {
+	if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) {
 		iint->flags |= IMA_MEASURED;
 		iint->measured_pcrs |= (0x1 << pcr);
 	}
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 802d5d20f36f..a856d8c9c9f3 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -441,6 +441,16 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 	loff_t i_size;
 	int rc;
 
+	/*
+	 * For consistency, fail file's opened with the O_DIRECT flag on
+	 * filesystems mounted with/without DAX option.
+	 */
+	if (file->f_flags & O_DIRECT) {
+		hash->length = hash_digest_size[ima_hash_algo];
+		hash->algo = ima_hash_algo;
+		return -EINVAL;
+	}
+
 	i_size = i_size_read(file_inode(file));
 
 	if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..d23dfe6ede18 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,11 +235,8 @@ 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 (file->f_flags & O_DIRECT)
-			rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;
+	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_digsig;
-	}
 
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
@@ -247,7 +244,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 == 0 && (action & IMA_APPRAISE_SUBMASK))
 		rc = ima_appraise_measurement(func, iint, file, pathname,
 					      xattr_value, xattr_len, opened);
 	if (action & IMA_AUDIT)
-- 
2.7.4

[PATCH v6 3/6] ima: always measure and audit files in policy

[Mimi Zohar]

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.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
Changelog v6:
- replace "?:" with if/then
- annotate i_version usage
- reword O_DIRECT comment

Changelog v5:
- Fail files opened O_DIRECT, but include attempt in measurement list.

Changelog v4:
- Based on both -EBADF and -EINVAL
- clean up ima_collect_measurement()

 security/integrity/ima/ima_api.c    | 67 +++++++++++++++++++++++--------------
 security/integrity/ima/ima_crypto.c | 10 ++++++
 security/integrity/ima/ima_main.c   |  7 ++--
 3 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c2edba8de35e..1dee695642a4 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -199,42 +199,59 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	struct inode *inode = file_inode(file);
 	const char *filename = file->f_path.dentry->d_name.name;
 	int result = 0;
+	int length;
+	void *tmpbuf;
+	u64 i_version;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash;
 
-	if (!(iint->flags & IMA_COLLECTED)) {
-		u64 i_version = file_inode(file)->i_version;
+	if (iint->flags & IMA_COLLECTED)
+		goto out;
 
-		if (file->f_flags & O_DIRECT) {
-			audit_cause = "failed(directio)";
-			result = -EACCES;
-			goto out;
-		}
+	/*
+	 * Dectecting file change is based on i_version. On filesystems
+	 * which do not support i_version, support is limited to an initial
+	 * measurement/appraisal/audit.
+	 */
+	i_version = file_inode(file)->i_version;
+	hash.hdr.algo = algo;
 
-		hash.hdr.algo = algo;
-
-		result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
-			ima_calc_buffer_hash(buf, size, &hash.hdr);
-		if (!result) {
-			int length = sizeof(hash.hdr) + hash.hdr.length;
-			void *tmpbuf = krealloc(iint->ima_hash, length,
-						GFP_NOFS);
-			if (tmpbuf) {
-				iint->ima_hash = tmpbuf;
-				memcpy(iint->ima_hash, &hash, length);
-				iint->version = i_version;
-				iint->flags |= IMA_COLLECTED;
-			} else
-				result = -ENOMEM;
-		}
+	/* Initialize hash digest to 0's in case of failure */
+	memset(&hash.digest, 0, sizeof(hash.digest));
+
+	if (buf)
+		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
+	else
+		result = ima_calc_file_hash(file, &hash.hdr);
+
+	if (result && result != -EBADF && result != -EINVAL)
+		goto out;
+
+	length = sizeof(hash.hdr) + hash.hdr.length;
+	tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
+	if (!tmpbuf) {
+		result = -ENOMEM;
+		goto out;
 	}
+
+	iint->ima_hash = tmpbuf;
+	memcpy(iint->ima_hash, &hash, length);
+	iint->version = i_version;
+
+	/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
+	if (result != -EBADF && result != -EINVAL)
+		iint->flags |= IMA_COLLECTED;
 out:
-	if (result)
+	if (result) {
+		if (file->f_flags & O_DIRECT)
+			audit_cause = "failed(directio)";
+
 		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
 				    filename, "collect_data", audit_cause,
 				    result, 0);
+	}
 	return result;
 }
 
@@ -278,7 +295,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 	}
 
 	result = ima_store_template(entry, violation, inode, filename, pcr);
-	if (!result || result == -EEXIST) {
+	if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) {
 		iint->flags |= IMA_MEASURED;
 		iint->measured_pcrs |= (0x1 << pcr);
 	}
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 802d5d20f36f..a856d8c9c9f3 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -441,6 +441,16 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 	loff_t i_size;
 	int rc;
 
+	/*
+	 * For consistency, fail file's opened with the O_DIRECT flag on
+	 * filesystems mounted with/without DAX option.
+	 */
+	if (file->f_flags & O_DIRECT) {
+		hash->length = hash_digest_size[ima_hash_algo];
+		hash->algo = ima_hash_algo;
+		return -EINVAL;
+	}
+
 	i_size = i_size_read(file_inode(file));
 
 	if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..d23dfe6ede18 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,11 +235,8 @@ 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 (file->f_flags & O_DIRECT)
-			rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;
+	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_digsig;
-	}
 
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
@@ -247,7 +244,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 == 0 && (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

Re: [PATCH v6 3/6] ima: always measure and audit files in policy

[Dmitry Kasatkin]

On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 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.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> Changelog v6:
> - replace "?:" with if/then
> - annotate i_version usage
> - reword O_DIRECT comment
>
> Changelog v5:
> - Fail files opened O_DIRECT, but include attempt in measurement list.
>
> Changelog v4:
> - Based on both -EBADF and -EINVAL
> - clean up ima_collect_measurement()
>
>  security/integrity/ima/ima_api.c    | 67 +++++++++++++++++++++++--------------
>  security/integrity/ima/ima_crypto.c | 10 ++++++
>  security/integrity/ima/ima_main.c   |  7 ++--
>  3 files changed, 54 insertions(+), 30 deletions(-)
>
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c2edba8de35e..1dee695642a4 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -199,42 +199,59 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>         struct inode *inode = file_inode(file);
>         const char *filename = file->f_path.dentry->d_name.name;
>         int result = 0;
> +       int length;
> +       void *tmpbuf;
> +       u64 i_version;
>         struct {
>                 struct ima_digest_data hdr;
>                 char digest[IMA_MAX_DIGEST_SIZE];
>         } hash;
>
> -       if (!(iint->flags & IMA_COLLECTED)) {
> -               u64 i_version = file_inode(file)->i_version;
> +       if (iint->flags & IMA_COLLECTED)
> +               goto out;
>
> -               if (file->f_flags & O_DIRECT) {
> -                       audit_cause = "failed(directio)";
> -                       result = -EACCES;
> -                       goto out;
> -               }
> +       /*
> +        * Dectecting file change is based on i_version. On filesystems
> +        * which do not support i_version, support is limited to an initial
> +        * measurement/appraisal/audit.
> +        */
> +       i_version = file_inode(file)->i_version;
> +       hash.hdr.algo = algo;
>
> -               hash.hdr.algo = algo;
> -
> -               result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
> -                       ima_calc_buffer_hash(buf, size, &hash.hdr);
> -               if (!result) {
> -                       int length = sizeof(hash.hdr) + hash.hdr.length;
> -                       void *tmpbuf = krealloc(iint->ima_hash, length,
> -                                               GFP_NOFS);
> -                       if (tmpbuf) {
> -                               iint->ima_hash = tmpbuf;
> -                               memcpy(iint->ima_hash, &hash, length);
> -                               iint->version = i_version;
> -                               iint->flags |= IMA_COLLECTED;
> -                       } else
> -                               result = -ENOMEM;
> -               }
> +       /* Initialize hash digest to 0's in case of failure */
> +       memset(&hash.digest, 0, sizeof(hash.digest));
> +
> +       if (buf)
> +               result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> +       else
> +               result = ima_calc_file_hash(file, &hash.hdr);
> +
> +       if (result && result != -EBADF && result != -EINVAL)
> +               goto out;
> +
> +       length = sizeof(hash.hdr) + hash.hdr.length;
> +       tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
> +       if (!tmpbuf) {
> +               result = -ENOMEM;
> +               goto out;
>         }
> +
> +       iint->ima_hash = tmpbuf;
> +       memcpy(iint->ima_hash, &hash, length);
> +       iint->version = i_version;
> +
> +       /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> +       if (result != -EBADF && result != -EINVAL)
> +               iint->flags |= IMA_COLLECTED;


Result can be other than 0, EBADF and EINVAL here?
It is confusing.. simpler than can be just

if (!result)
 iint->flags |= IMA_COLLECTED;

Isn't it?


>  out:
> -       if (result)
> +       if (result) {
> +               if (file->f_flags & O_DIRECT)
> +                       audit_cause = "failed(directio)";
> +
>                 integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
>                                     filename, "collect_data", audit_cause,
>                                     result, 0);
> +       }
>         return result;
>  }
>
> @@ -278,7 +295,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
>         }
>
>         result = ima_store_template(entry, violation, inode, filename, pcr);
> -       if (!result || result == -EEXIST) {
> +       if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) {
>                 iint->flags |= IMA_MEASURED;
>                 iint->measured_pcrs |= (0x1 << pcr);
>         }
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 802d5d20f36f..a856d8c9c9f3 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -441,6 +441,16 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
>         loff_t i_size;
>         int rc;
>
> +       /*
> +        * For consistency, fail file's opened with the O_DIRECT flag on
> +        * filesystems mounted with/without DAX option.
> +        */
> +       if (file->f_flags & O_DIRECT) {
> +               hash->length = hash_digest_size[ima_hash_algo];
> +               hash->algo = ima_hash_algo;
> +               return -EINVAL;
> +       }
> +
>         i_size = i_size_read(file_inode(file));
>
>         if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 2aebb7984437..d23dfe6ede18 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -235,11 +235,8 @@ 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 (file->f_flags & O_DIRECT)
> -                       rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;
> +       if (rc != 0 && rc != -EBADF && rc != -EINVAL)
>                 goto out_digsig;
> -       }
>
>         if (!pathbuf)   /* ima_rdwr_violation possibly pre-fetched */
>                 pathname = ima_d_path(&file->f_path, &pathbuf, filename);
> @@ -247,7 +244,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 == 0 && (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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,
Dmitry

[PATCH v6 3/6] ima: always measure and audit files in policy

[Dmitry Kasatkin]

On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 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.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> Changelog v6:
> - replace "?:" with if/then
> - annotate i_version usage
> - reword O_DIRECT comment
>
> Changelog v5:
> - Fail files opened O_DIRECT, but include attempt in measurement list.
>
> Changelog v4:
> - Based on both -EBADF and -EINVAL
> - clean up ima_collect_measurement()
>
>  security/integrity/ima/ima_api.c    | 67 +++++++++++++++++++++++--------------
>  security/integrity/ima/ima_crypto.c | 10 ++++++
>  security/integrity/ima/ima_main.c   |  7 ++--
>  3 files changed, 54 insertions(+), 30 deletions(-)
>
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c2edba8de35e..1dee695642a4 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -199,42 +199,59 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>         struct inode *inode = file_inode(file);
>         const char *filename = file->f_path.dentry->d_name.name;
>         int result = 0;
> +       int length;
> +       void *tmpbuf;
> +       u64 i_version;
>         struct {
>                 struct ima_digest_data hdr;
>                 char digest[IMA_MAX_DIGEST_SIZE];
>         } hash;
>
> -       if (!(iint->flags & IMA_COLLECTED)) {
> -               u64 i_version = file_inode(file)->i_version;
> +       if (iint->flags & IMA_COLLECTED)
> +               goto out;
>
> -               if (file->f_flags & O_DIRECT) {
> -                       audit_cause = "failed(directio)";
> -                       result = -EACCES;
> -                       goto out;
> -               }
> +       /*
> +        * Dectecting file change is based on i_version. On filesystems
> +        * which do not support i_version, support is limited to an initial
> +        * measurement/appraisal/audit.
> +        */
> +       i_version = file_inode(file)->i_version;
> +       hash.hdr.algo = algo;
>
> -               hash.hdr.algo = algo;
> -
> -               result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
> -                       ima_calc_buffer_hash(buf, size, &hash.hdr);
> -               if (!result) {
> -                       int length = sizeof(hash.hdr) + hash.hdr.length;
> -                       void *tmpbuf = krealloc(iint->ima_hash, length,
> -                                               GFP_NOFS);
> -                       if (tmpbuf) {
> -                               iint->ima_hash = tmpbuf;
> -                               memcpy(iint->ima_hash, &hash, length);
> -                               iint->version = i_version;
> -                               iint->flags |= IMA_COLLECTED;
> -                       } else
> -                               result = -ENOMEM;
> -               }
> +       /* Initialize hash digest to 0's in case of failure */
> +       memset(&hash.digest, 0, sizeof(hash.digest));
> +
> +       if (buf)
> +               result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> +       else
> +               result = ima_calc_file_hash(file, &hash.hdr);
> +
> +       if (result && result != -EBADF && result != -EINVAL)
> +               goto out;
> +
> +       length = sizeof(hash.hdr) + hash.hdr.length;
> +       tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
> +       if (!tmpbuf) {
> +               result = -ENOMEM;
> +               goto out;
>         }
> +
> +       iint->ima_hash = tmpbuf;
> +       memcpy(iint->ima_hash, &hash, length);
> +       iint->version = i_version;
> +
> +       /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> +       if (result != -EBADF && result != -EINVAL)
> +               iint->flags |= IMA_COLLECTED;


Result can be other than 0, EBADF and EINVAL here?
It is confusing.. simpler than can be just

if (!result)
 iint->flags |= IMA_COLLECTED;

Isn't it?


>  out:
> -       if (result)
> +       if (result) {
> +               if (file->f_flags & O_DIRECT)
> +                       audit_cause = "failed(directio)";
> +
>                 integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
>                                     filename, "collect_data", audit_cause,
>                                     result, 0);
> +       }
>         return result;
>  }
>
> @@ -278,7 +295,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
>         }
>
>         result = ima_store_template(entry, violation, inode, filename, pcr);
> -       if (!result || result == -EEXIST) {
> +       if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) {
>                 iint->flags |= IMA_MEASURED;
>                 iint->measured_pcrs |= (0x1 << pcr);
>         }
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 802d5d20f36f..a856d8c9c9f3 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -441,6 +441,16 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
>         loff_t i_size;
>         int rc;
>
> +       /*
> +        * For consistency, fail file's opened with the O_DIRECT flag on
> +        * filesystems mounted with/without DAX option.
> +        */
> +       if (file->f_flags & O_DIRECT) {
> +               hash->length = hash_digest_size[ima_hash_algo];
> +               hash->algo = ima_hash_algo;
> +               return -EINVAL;
> +       }
> +
>         i_size = i_size_read(file_inode(file));
>
>         if (ima_ahash_minsize && i_size >= ima_ahash_minsize) {
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 2aebb7984437..d23dfe6ede18 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -235,11 +235,8 @@ 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 (file->f_flags & O_DIRECT)
> -                       rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;
> +       if (rc != 0 && rc != -EBADF && rc != -EINVAL)
>                 goto out_digsig;
> -       }
>
>         if (!pathbuf)   /* ima_rdwr_violation possibly pre-fetched */
>                 pathname = ima_d_path(&file->f_path, &pathbuf, filename);
> @@ -247,7 +244,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 == 0 && (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



-- 
Thanks,
Dmitry
--
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

Re: [PATCH v6 3/6] ima: always measure and audit files in policy

[Mimi Zohar]

On Tue, 2017-08-22 at 13:05 +0300, Dmitry Kasatkin wrote:
> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > 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.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > ---
> > Changelog v6:
> > - replace "?:" with if/then
> > - annotate i_version usage
> > - reword O_DIRECT comment
> >
> > Changelog v5:
> > - Fail files opened O_DIRECT, but include attempt in measurement list.
> >
> > Changelog v4:
> > - Based on both -EBADF and -EINVAL
> > - clean up ima_collect_measurement()
> >
> >  security/integrity/ima/ima_api.c    | 67 +++++++++++++++++++++++--------------
> >  security/integrity/ima/ima_crypto.c | 10 ++++++
> >  security/integrity/ima/ima_main.c   |  7 ++--
> >  3 files changed, 54 insertions(+), 30 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c2edba8de35e..1dee695642a4 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -199,42 +199,59 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
> >         struct inode *inode = file_inode(file);
> >         const char *filename = file->f_path.dentry->d_name.name;
> >         int result = 0;
> > +       int length;
> > +       void *tmpbuf;
> > +       u64 i_version;
> >         struct {
> >                 struct ima_digest_data hdr;
> >                 char digest[IMA_MAX_DIGEST_SIZE];
> >         } hash;
> >
> > -       if (!(iint->flags & IMA_COLLECTED)) {
> > -               u64 i_version = file_inode(file)->i_version;
> > +       if (iint->flags & IMA_COLLECTED)
> > +               goto out;
> >
> > -               if (file->f_flags & O_DIRECT) {
> > -                       audit_cause = "failed(directio)";
> > -                       result = -EACCES;
> > -                       goto out;
> > -               }
> > +       /*
> > +        * Dectecting file change is based on i_version. On filesystems
> > +        * which do not support i_version, support is limited to an initial
> > +        * measurement/appraisal/audit.
> > +        */
> > +       i_version = file_inode(file)->i_version;
> > +       hash.hdr.algo = algo;
> >
> > -               hash.hdr.algo = algo;
> > -
> > -               result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
> > -                       ima_calc_buffer_hash(buf, size, &hash.hdr);
> > -               if (!result) {
> > -                       int length = sizeof(hash.hdr) + hash.hdr.length;
> > -                       void *tmpbuf = krealloc(iint->ima_hash, length,
> > -                                               GFP_NOFS);
> > -                       if (tmpbuf) {
> > -                               iint->ima_hash = tmpbuf;
> > -                               memcpy(iint->ima_hash, &hash, length);
> > -                               iint->version = i_version;
> > -                               iint->flags |= IMA_COLLECTED;
> > -                       } else
> > -                               result = -ENOMEM;
> > -               }
> > +       /* Initialize hash digest to 0's in case of failure */
> > +       memset(&hash.digest, 0, sizeof(hash.digest));
> > +
> > +       if (buf)
> > +               result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> > +       else
> > +               result = ima_calc_file_hash(file, &hash.hdr);
> > +
> > +       if (result && result != -EBADF && result != -EINVAL)
> > +               goto out;
> > +
> > +       length = sizeof(hash.hdr) + hash.hdr.length;
> > +       tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
> > +       if (!tmpbuf) {
> > +               result = -ENOMEM;
> > +               goto out;
> >         }
> > +
> > +       iint->ima_hash = tmpbuf;
> > +       memcpy(iint->ima_hash, &hash, length);
> > +       iint->version = i_version;
> > +
> > +       /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > +       if (result != -EBADF && result != -EINVAL)
> > +               iint->flags |= IMA_COLLECTED;
> 
> 
> Result can be other than 0, EBADF and EINVAL here?
> It is confusing.. simpler than can be just
> 
> if (!result)
>  iint->flags |= IMA_COLLECTED;
> 
> Isn't it?

Yes, that is better.

Mimi

[PATCH v6 3/6] ima: always measure and audit files in policy

[Mimi Zohar]

On Tue, 2017-08-22 at 13:05 +0300, Dmitry Kasatkin wrote:
> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > 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.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > ---
> > Changelog v6:
> > - replace "?:" with if/then
> > - annotate i_version usage
> > - reword O_DIRECT comment
> >
> > Changelog v5:
> > - Fail files opened O_DIRECT, but include attempt in measurement list.
> >
> > Changelog v4:
> > - Based on both -EBADF and -EINVAL
> > - clean up ima_collect_measurement()
> >
> >  security/integrity/ima/ima_api.c    | 67 +++++++++++++++++++++++--------------
> >  security/integrity/ima/ima_crypto.c | 10 ++++++
> >  security/integrity/ima/ima_main.c   |  7 ++--
> >  3 files changed, 54 insertions(+), 30 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c2edba8de35e..1dee695642a4 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -199,42 +199,59 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
> >         struct inode *inode = file_inode(file);
> >         const char *filename = file->f_path.dentry->d_name.name;
> >         int result = 0;
> > +       int length;
> > +       void *tmpbuf;
> > +       u64 i_version;
> >         struct {
> >                 struct ima_digest_data hdr;
> >                 char digest[IMA_MAX_DIGEST_SIZE];
> >         } hash;
> >
> > -       if (!(iint->flags & IMA_COLLECTED)) {
> > -               u64 i_version = file_inode(file)->i_version;
> > +       if (iint->flags & IMA_COLLECTED)
> > +               goto out;
> >
> > -               if (file->f_flags & O_DIRECT) {
> > -                       audit_cause = "failed(directio)";
> > -                       result = -EACCES;
> > -                       goto out;
> > -               }
> > +       /*
> > +        * Dectecting file change is based on i_version. On filesystems
> > +        * which do not support i_version, support is limited to an initial
> > +        * measurement/appraisal/audit.
> > +        */
> > +       i_version = file_inode(file)->i_version;
> > +       hash.hdr.algo = algo;
> >
> > -               hash.hdr.algo = algo;
> > -
> > -               result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
> > -                       ima_calc_buffer_hash(buf, size, &hash.hdr);
> > -               if (!result) {
> > -                       int length = sizeof(hash.hdr) + hash.hdr.length;
> > -                       void *tmpbuf = krealloc(iint->ima_hash, length,
> > -                                               GFP_NOFS);
> > -                       if (tmpbuf) {
> > -                               iint->ima_hash = tmpbuf;
> > -                               memcpy(iint->ima_hash, &hash, length);
> > -                               iint->version = i_version;
> > -                               iint->flags |= IMA_COLLECTED;
> > -                       } else
> > -                               result = -ENOMEM;
> > -               }
> > +       /* Initialize hash digest to 0's in case of failure */
> > +       memset(&hash.digest, 0, sizeof(hash.digest));
> > +
> > +       if (buf)
> > +               result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> > +       else
> > +               result = ima_calc_file_hash(file, &hash.hdr);
> > +
> > +       if (result && result != -EBADF && result != -EINVAL)
> > +               goto out;
> > +
> > +       length = sizeof(hash.hdr) + hash.hdr.length;
> > +       tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
> > +       if (!tmpbuf) {
> > +               result = -ENOMEM;
> > +               goto out;
> >         }
> > +
> > +       iint->ima_hash = tmpbuf;
> > +       memcpy(iint->ima_hash, &hash, length);
> > +       iint->version = i_version;
> > +
> > +       /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > +       if (result != -EBADF && result != -EINVAL)
> > +               iint->flags |= IMA_COLLECTED;
> 
> 
> Result can be other than 0, EBADF and EINVAL here?
> It is confusing.. simpler than can be just
> 
> if (!result)
>  iint->flags |= IMA_COLLECTED;
> 
> Isn't it?

Yes, that is better.

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

[PATCH v6 4/6] ima: use fs method to read integrity data

[Mimi Zohar]

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.

(Based on Christoph's original patch.)

Signed-off-by: Christoph Hellwig <hch@lst.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Jan Kara <jack@suse.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Cc: Bob Peterson <rpeterso@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Dave Kleikamp <shaggy@kernel.org>
Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Cc: Mark Fasheh <mfasheh@versity.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Chris Mason <clm@fb.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
Changelog v6:
- defined separate efivarfs and libfs patches.

Changelog v5:
- removed ocf2 and gfs2 integrity_read support.
- removed ext4 special case to fail O_DIRECT open for buffered read.

Changelog v4:
- define ext2/4 specific ->integrity_read functions.
- properly fail file open with O_DIRECT on filesystem not mounted
with "-o dax".

Changelog 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.

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        |  1 +
 fs/ext2/file.c            | 17 +++++++++++++++++
 fs/ext4/file.c            | 20 ++++++++++++++++++++
 fs/f2fs/file.c            |  1 +
 fs/jffs2/file.c           |  1 +
 fs/jfs/file.c             |  1 +
 fs/nilfs2/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 | 20 ++++++++++++++------
 15 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 9e75d8a39aac..2542dc66c85c 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3125,6 +3125,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 863f1b100165..17955a92a5b3 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -179,4 +179,5 @@ const struct file_operations efivarfs_file_operations = {
 	.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 d34d32bdc944..111069de1973 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -192,6 +192,22 @@ static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return generic_file_read_iter(iocb, to);
 }
 
+static ssize_t ext2_file_integrity_read_iter(struct kiocb *iocb,
+					     struct iov_iter *to)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	lockdep_assert_held(&inode->i_rwsem);
+#ifdef CONFIG_FS_DAX
+	if (!iov_iter_count(to))
+		return 0; /* skip atime */
+
+	if (IS_DAX(iocb->ki_filp->f_mapping->host))
+		return dax_iomap_rw(iocb, to, &ext2_iomap_ops);
+#endif
+	return generic_file_read_iter(iocb, to);
+}
+
 static ssize_t ext2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 #ifdef CONFIG_FS_DAX
@@ -216,6 +232,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	= ext2_file_integrity_read_iter,
 };
 
 const struct inode_operations ext2_file_inode_operations = {
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 58294c9a7e1d..3ab4105c8578 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -74,6 +74,25 @@ static ssize_t ext4_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return generic_file_read_iter(iocb, to);
 }
 
+static ssize_t ext4_file_integrity_read_iter(struct kiocb *iocb,
+					     struct iov_iter *to)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	lockdep_assert_held(&inode->i_rwsem);
+	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
+		return -EIO;
+
+	if (!iov_iter_count(to))
+		return 0; /* skip atime */
+
+#ifdef CONFIG_FS_DAX
+	if (IS_DAX(inode))
+		return dax_iomap_rw(iocb, to, &ext4_iomap_ops);
+#endif
+	return generic_file_read_iter(iocb, to);
+}
+
 /*
  * Called when an inode is released. Note that this is different
  * from ext4_file_open: open gets called at every open, but release
@@ -747,6 +766,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_integrity_read_iter,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 2706130c261b..82ea81da0b2d 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2514,4 +2514,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/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/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/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 8cad0b19b404..5e52a315e18b 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1747,4 +1747,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 c4893e226fd8..0a6704b563d6 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -292,6 +292,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.
  *
@@ -1175,6 +1195,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 fdec9b763b54..8d0d10e1dd93 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1699,6 +1699,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 *);
 } __randomize_layout;
 
 struct inode_operations {
diff --git a/mm/shmem.c b/mm/shmem.c
index b0aa6075d164..805d99011ca4 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3849,6 +3849,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

[PATCH v6 4/6] ima: use fs method to read integrity data

[Mimi Zohar]

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.

(Based on Christoph's original patch.)

Signed-off-by: Christoph Hellwig <hch@lst.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Jan Kara <jack@suse.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Cc: Bob Peterson <rpeterso@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Dave Kleikamp <shaggy@kernel.org>
Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Cc: Mark Fasheh <mfasheh@versity.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Chris Mason <clm@fb.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
Changelog v6:
- defined separate efivarfs and libfs patches.

Changelog v5:
- removed ocf2 and gfs2 integrity_read support.
- removed ext4 special case to fail O_DIRECT open for buffered read.

Changelog v4:
- define ext2/4 specific ->integrity_read functions.
- properly fail file open with O_DIRECT on filesystem not mounted
with "-o dax".

Changelog 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.

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        |  1 +
 fs/ext2/file.c            | 17 +++++++++++++++++
 fs/ext4/file.c            | 20 ++++++++++++++++++++
 fs/f2fs/file.c            |  1 +
 fs/jffs2/file.c           |  1 +
 fs/jfs/file.c             |  1 +
 fs/nilfs2/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 | 20 ++++++++++++++------
 15 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 9e75d8a39aac..2542dc66c85c 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3125,6 +3125,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 863f1b100165..17955a92a5b3 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -179,4 +179,5 @@ const struct file_operations efivarfs_file_operations = {
 	.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 d34d32bdc944..111069de1973 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -192,6 +192,22 @@ static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return generic_file_read_iter(iocb, to);
 }
 
+static ssize_t ext2_file_integrity_read_iter(struct kiocb *iocb,
+					     struct iov_iter *to)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	lockdep_assert_held(&inode->i_rwsem);
+#ifdef CONFIG_FS_DAX
+	if (!iov_iter_count(to))
+		return 0; /* skip atime */
+
+	if (IS_DAX(iocb->ki_filp->f_mapping->host))
+		return dax_iomap_rw(iocb, to, &ext2_iomap_ops);
+#endif
+	return generic_file_read_iter(iocb, to);
+}
+
 static ssize_t ext2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
 #ifdef CONFIG_FS_DAX
@@ -216,6 +232,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	= ext2_file_integrity_read_iter,
 };
 
 const struct inode_operations ext2_file_inode_operations = {
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 58294c9a7e1d..3ab4105c8578 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -74,6 +74,25 @@ static ssize_t ext4_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return generic_file_read_iter(iocb, to);
 }
 
+static ssize_t ext4_file_integrity_read_iter(struct kiocb *iocb,
+					     struct iov_iter *to)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	lockdep_assert_held(&inode->i_rwsem);
+	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
+		return -EIO;
+
+	if (!iov_iter_count(to))
+		return 0; /* skip atime */
+
+#ifdef CONFIG_FS_DAX
+	if (IS_DAX(inode))
+		return dax_iomap_rw(iocb, to, &ext4_iomap_ops);
+#endif
+	return generic_file_read_iter(iocb, to);
+}
+
 /*
  * Called when an inode is released. Note that this is different
  * from ext4_file_open: open gets called at every open, but release
@@ -747,6 +766,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_integrity_read_iter,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 2706130c261b..82ea81da0b2d 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2514,4 +2514,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/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/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/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 8cad0b19b404..5e52a315e18b 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1747,4 +1747,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 c4893e226fd8..0a6704b563d6 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -292,6 +292,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.
  *
@@ -1175,6 +1195,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 fdec9b763b54..8d0d10e1dd93 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1699,6 +1699,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 *);
 } __randomize_layout;
 
 struct inode_operations {
diff --git a/mm/shmem.c b/mm/shmem.c
index b0aa6075d164..805d99011ca4 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3849,6 +3849,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

--
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

Re: [PATCH v6 4/6] ima: use fs method to read integrity data

[Jan Kara]

On Tue 15-08-17 10:43:55, 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.
> 
> (Based on Christoph's original patch.)
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Jan Kara <jack@suse.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Steven Whitehouse <swhiteho@redhat.com>
> Cc: Bob Peterson <rpeterso@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dave Kleikamp <shaggy@kernel.org>
> Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> Cc: Mark Fasheh <mfasheh@versity.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Chris Mason <clm@fb.com>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v6 4/6] ima: use fs method to read integrity data

[Jan Kara]

On Tue 15-08-17 10:43:55, 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.
> 
> (Based on Christoph's original patch.)
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Jan Kara <jack@suse.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Steven Whitehouse <swhiteho@redhat.com>
> Cc: Bob Peterson <rpeterso@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dave Kleikamp <shaggy@kernel.org>
> Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> Cc: Mark Fasheh <mfasheh@versity.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Chris Mason <clm@fb.com>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
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

Re: [PATCH v6 4/6] ima: use fs method to read integrity data

[Mimi Zohar]

On Wed, 2017-08-16 at 15:17 +0200, Jan Kara wrote:
> On Tue 15-08-17 10:43:55, 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.
> > 
> > (Based on Christoph's original patch.)
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> > Cc: Jan Kara <jack@suse.com>
> > Cc: "Theodore Ts'o" <tytso@mit.edu>
> > Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> > Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> > Cc: Chao Yu <yuchao0@huawei.com>
> > Cc: Steven Whitehouse <swhiteho@redhat.com>
> > Cc: Bob Peterson <rpeterso@redhat.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Dave Kleikamp <shaggy@kernel.org>
> > Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> > Cc: Mark Fasheh <mfasheh@versity.com>
> > Cc: Joel Becker <jlbec@evilplan.org>
> > Cc: Richard Weinberger <richard@nod.at>
> > Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Chris Mason <clm@fb.com>
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> 
> Looks good to me. You can add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks!

[PATCH v6 4/6] ima: use fs method to read integrity data

[Mimi Zohar]

On Wed, 2017-08-16 at 15:17 +0200, Jan Kara wrote:
> On Tue 15-08-17 10:43:55, 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.
> > 
> > (Based on Christoph's original patch.)
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> > Cc: Jan Kara <jack@suse.com>
> > Cc: "Theodore Ts'o" <tytso@mit.edu>
> > Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> > Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> > Cc: Chao Yu <yuchao0@huawei.com>
> > Cc: Steven Whitehouse <swhiteho@redhat.com>
> > Cc: Bob Peterson <rpeterso@redhat.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Dave Kleikamp <shaggy@kernel.org>
> > Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> > Cc: Mark Fasheh <mfasheh@versity.com>
> > Cc: Joel Becker <jlbec@evilplan.org>
> > Cc: Richard Weinberger <richard@nod.at>
> > Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Chris Mason <clm@fb.com>
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> 
> Looks good to me. You can add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks!

--
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

Re: [PATCH v6 4/6] ima: use fs method to read integrity data

[Dmitry Kasatkin]

Looks good to me.


On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> 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.
>
> (Based on Christoph's original patch.)
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Jan Kara <jack@suse.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Steven Whitehouse <swhiteho@redhat.com>
> Cc: Bob Peterson <rpeterso@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dave Kleikamp <shaggy@kernel.org>
> Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> Cc: Mark Fasheh <mfasheh@versity.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Chris Mason <clm@fb.com>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> Changelog v6:
> - defined separate efivarfs and libfs patches.
>
> Changelog v5:
> - removed ocf2 and gfs2 integrity_read support.
> - removed ext4 special case to fail O_DIRECT open for buffered read.
>
> Changelog v4:
> - define ext2/4 specific ->integrity_read functions.
> - properly fail file open with O_DIRECT on filesystem not mounted
> with "-o dax".
>
> Changelog 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.
>
> 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        |  1 +
>  fs/ext2/file.c            | 17 +++++++++++++++++
>  fs/ext4/file.c            | 20 ++++++++++++++++++++
>  fs/f2fs/file.c            |  1 +
>  fs/jffs2/file.c           |  1 +
>  fs/jfs/file.c             |  1 +
>  fs/nilfs2/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 | 20 ++++++++++++++------
>  15 files changed, 83 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 9e75d8a39aac..2542dc66c85c 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -3125,6 +3125,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 863f1b100165..17955a92a5b3 100644
> --- a/fs/efivarfs/file.c
> +++ b/fs/efivarfs/file.c
> @@ -179,4 +179,5 @@ const struct file_operations efivarfs_file_operations = {
>         .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 d34d32bdc944..111069de1973 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -192,6 +192,22 @@ static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>         return generic_file_read_iter(iocb, to);
>  }
>
> +static ssize_t ext2_file_integrity_read_iter(struct kiocb *iocb,
> +                                            struct iov_iter *to)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       lockdep_assert_held(&inode->i_rwsem);
> +#ifdef CONFIG_FS_DAX
> +       if (!iov_iter_count(to))
> +               return 0; /* skip atime */
> +
> +       if (IS_DAX(iocb->ki_filp->f_mapping->host))
> +               return dax_iomap_rw(iocb, to, &ext2_iomap_ops);
> +#endif
> +       return generic_file_read_iter(iocb, to);
> +}
> +
>  static ssize_t ext2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  {
>  #ifdef CONFIG_FS_DAX
> @@ -216,6 +232,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 = ext2_file_integrity_read_iter,
>  };
>
>  const struct inode_operations ext2_file_inode_operations = {
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 58294c9a7e1d..3ab4105c8578 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -74,6 +74,25 @@ static ssize_t ext4_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>         return generic_file_read_iter(iocb, to);
>  }
>
> +static ssize_t ext4_file_integrity_read_iter(struct kiocb *iocb,
> +                                            struct iov_iter *to)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       lockdep_assert_held(&inode->i_rwsem);
> +       if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +               return -EIO;
> +
> +       if (!iov_iter_count(to))
> +               return 0; /* skip atime */
> +
> +#ifdef CONFIG_FS_DAX
> +       if (IS_DAX(inode))
> +               return dax_iomap_rw(iocb, to, &ext4_iomap_ops);
> +#endif
> +       return generic_file_read_iter(iocb, to);
> +}
> +
>  /*
>   * Called when an inode is released. Note that this is different
>   * from ext4_file_open: open gets called at every open, but release
> @@ -747,6 +766,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_integrity_read_iter,
>  };
>
>  const struct inode_operations ext4_file_inode_operations = {
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 2706130c261b..82ea81da0b2d 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2514,4 +2514,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/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/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/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 8cad0b19b404..5e52a315e18b 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1747,4 +1747,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 c4893e226fd8..0a6704b563d6 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -292,6 +292,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.
>   *
> @@ -1175,6 +1195,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 fdec9b763b54..8d0d10e1dd93 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1699,6 +1699,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 *);
>  } __randomize_layout;
>
>  struct inode_operations {
> diff --git a/mm/shmem.c b/mm/shmem.c
> index b0aa6075d164..805d99011ca4 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3849,6 +3849,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,
Dmitry

[PATCH v6 4/6] ima: use fs method to read integrity data

[Dmitry Kasatkin]

Looks good to me.


On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> 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.
>
> (Based on Christoph's original patch.)
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: Jan Kara <jack@suse.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Steven Whitehouse <swhiteho@redhat.com>
> Cc: Bob Peterson <rpeterso@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Dave Kleikamp <shaggy@kernel.org>
> Cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> Cc: Mark Fasheh <mfasheh@versity.com>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Chris Mason <clm@fb.com>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> Changelog v6:
> - defined separate efivarfs and libfs patches.
>
> Changelog v5:
> - removed ocf2 and gfs2 integrity_read support.
> - removed ext4 special case to fail O_DIRECT open for buffered read.
>
> Changelog v4:
> - define ext2/4 specific ->integrity_read functions.
> - properly fail file open with O_DIRECT on filesystem not mounted
> with "-o dax".
>
> Changelog 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.
>
> 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        |  1 +
>  fs/ext2/file.c            | 17 +++++++++++++++++
>  fs/ext4/file.c            | 20 ++++++++++++++++++++
>  fs/f2fs/file.c            |  1 +
>  fs/jffs2/file.c           |  1 +
>  fs/jfs/file.c             |  1 +
>  fs/nilfs2/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 | 20 ++++++++++++++------
>  15 files changed, 83 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 9e75d8a39aac..2542dc66c85c 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -3125,6 +3125,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 863f1b100165..17955a92a5b3 100644
> --- a/fs/efivarfs/file.c
> +++ b/fs/efivarfs/file.c
> @@ -179,4 +179,5 @@ const struct file_operations efivarfs_file_operations = {
>         .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 d34d32bdc944..111069de1973 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -192,6 +192,22 @@ static ssize_t ext2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>         return generic_file_read_iter(iocb, to);
>  }
>
> +static ssize_t ext2_file_integrity_read_iter(struct kiocb *iocb,
> +                                            struct iov_iter *to)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       lockdep_assert_held(&inode->i_rwsem);
> +#ifdef CONFIG_FS_DAX
> +       if (!iov_iter_count(to))
> +               return 0; /* skip atime */
> +
> +       if (IS_DAX(iocb->ki_filp->f_mapping->host))
> +               return dax_iomap_rw(iocb, to, &ext2_iomap_ops);
> +#endif
> +       return generic_file_read_iter(iocb, to);
> +}
> +
>  static ssize_t ext2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  {
>  #ifdef CONFIG_FS_DAX
> @@ -216,6 +232,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 = ext2_file_integrity_read_iter,
>  };
>
>  const struct inode_operations ext2_file_inode_operations = {
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 58294c9a7e1d..3ab4105c8578 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -74,6 +74,25 @@ static ssize_t ext4_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>         return generic_file_read_iter(iocb, to);
>  }
>
> +static ssize_t ext4_file_integrity_read_iter(struct kiocb *iocb,
> +                                            struct iov_iter *to)
> +{
> +       struct inode *inode = file_inode(iocb->ki_filp);
> +
> +       lockdep_assert_held(&inode->i_rwsem);
> +       if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +               return -EIO;
> +
> +       if (!iov_iter_count(to))
> +               return 0; /* skip atime */
> +
> +#ifdef CONFIG_FS_DAX
> +       if (IS_DAX(inode))
> +               return dax_iomap_rw(iocb, to, &ext4_iomap_ops);
> +#endif
> +       return generic_file_read_iter(iocb, to);
> +}
> +
>  /*
>   * Called when an inode is released. Note that this is different
>   * from ext4_file_open: open gets called at every open, but release
> @@ -747,6 +766,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_integrity_read_iter,
>  };
>
>  const struct inode_operations ext4_file_inode_operations = {
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 2706130c261b..82ea81da0b2d 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2514,4 +2514,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/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/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/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 8cad0b19b404..5e52a315e18b 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1747,4 +1747,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 c4893e226fd8..0a6704b563d6 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -292,6 +292,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.
>   *
> @@ -1175,6 +1195,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 fdec9b763b54..8d0d10e1dd93 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1699,6 +1699,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 *);
>  } __randomize_layout;
>
>  struct inode_operations {
> diff --git a/mm/shmem.c b/mm/shmem.c
> index b0aa6075d164..805d99011ca4 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3849,6 +3849,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
>
> --
> 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



-- 
Thanks,
Dmitry
--
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

Re: [PATCH v6 4/6] ima: use fs method to read integrity data

[Al Viro]

On Tue, Aug 15, 2017 at 10:43:55AM -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.

Hmm...  I'm really tempted to add default_integrity_read() that would
just call ->read_iter(), with boilerplate part becoming
	.integrity_read = default_integrity_read

Note that all stuff accessed in it would be fresh in caches, so
it's not as if we had serious overhead there.  And we are going
to be reading from file, anyway...

I agree that it should be an opt-in from filesystem; default is still
"don't know how to read, sod off".  It's just that telling at the
glance whether it's supposed to be a simple case or something tricky
is needed would be simpler that way and it might turn out to be
more robust that way...

[PATCH v6 4/6] ima: use fs method to read integrity data

[Al Viro]

On Tue, Aug 15, 2017 at 10:43:55AM -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.

Hmm...  I'm really tempted to add default_integrity_read() that would
just call ->read_iter(), with boilerplate part becoming
	.integrity_read = default_integrity_read

Note that all stuff accessed in it would be fresh in caches, so
it's not as if we had serious overhead there.  And we are going
to be reading from file, anyway...

I agree that it should be an opt-in from filesystem; default is still
"don't know how to read, sod off".  It's just that telling at the
glance whether it's supposed to be a simple case or something tricky
is needed would be simpler that way and it might turn out to be
more robust that way...
--
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

Re: [PATCH v6 4/6] ima: use fs method to read integrity data

[Mimi Zohar]

On Mon, 2017-08-28 at 05:13 +0100, Al Viro wrote:
> On Tue, Aug 15, 2017 at 10:43:55AM -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.
> 
> Hmm...  I'm really tempted to add default_integrity_read() that would
> just call ->read_iter(), with boilerplate part becoming
> 	.integrity_read = default_integrity_read

How can it automatically call the fs read_iter() without knowing if
the fs read_iter() takes the i_rwsem?  Or are you suggesting that the
default_integrity_read is defined as generic_file_read_iter()?

Mimi

> Note that all stuff accessed in it would be fresh in caches, so
> it's not as if we had serious overhead there.  And we are going
> to be reading from file, anyway...
> 
> I agree that it should be an opt-in from filesystem; default is still
> "don't know how to read, sod off".  It's just that telling at the
> glance whether it's supposed to be a simple case or something tricky
> is needed would be simpler that way and it might turn out to be
> more robust that way...
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v6 4/6] ima: use fs method to read integrity data

[Mimi Zohar]

On Mon, 2017-08-28 at 05:13 +0100, Al Viro wrote:
> On Tue, Aug 15, 2017 at 10:43:55AM -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.
> 
> Hmm...  I'm really tempted to add default_integrity_read() that would
> just call ->read_iter(), with boilerplate part becoming
> 	.integrity_read = default_integrity_read

How can it automatically call the fs read_iter() without knowing if
the fs read_iter() takes the i_rwsem? ?Or are you suggesting that the
default_integrity_read is defined as generic_file_read_iter()?

Mimi

> Note that all stuff accessed in it would be fresh in caches, so
> it's not as if we had serious overhead there.  And we are going
> to be reading from file, anyway...
> 
> I agree that it should be an opt-in from filesystem; default is still
> "don't know how to read, sod off".  It's just that telling at the
> glance whether it's supposed to be a simple case or something tricky
> is needed would be simpler that way and it might turn out to be
> more robust that way...
> --
> 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
> 

--
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

[PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Mimi Zohar]

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

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
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    | 12 +++++++++++-
 security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
 4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -38,6 +38,12 @@ 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();
@@ -260,8 +266,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 95209a5f8595..43b85a4fb8e8 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);
 
@@ -949,6 +973,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);
@@ -1040,6 +1065,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

[PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Mimi Zohar]

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

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
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    | 12 +++++++++++-
 security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
 4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -38,6 +38,12 @@ 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();
@@ -260,8 +266,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 95209a5f8595..43b85a4fb8e8 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);
 
@@ -949,6 +973,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);
@@ -1040,6 +1065,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

--
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

Re: [PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Dmitry Kasatkin]

On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Permit normally denied access/execute permission for files in policy
> on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
> policy action rule.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> 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    | 12 +++++++++++-
>  security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
>  4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -38,6 +38,12 @@ 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();
> @@ -260,8 +266,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;
> +

By default IMA is failsafe. ima_failsafe is true.
Return 0 is needed in failsafe mode. right?
But in this logic it will happen if ima_failsafe is false. meaning it
is not failsafe.

Is it a typo?

>                 return -EACCES;
> +       }
>         return 0;
>  }
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 95209a5f8595..43b85a4fb8e8 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);
>
> @@ -949,6 +973,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);
> @@ -1040,6 +1065,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,
Dmitry

[PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Dmitry Kasatkin]

On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Permit normally denied access/execute permission for files in policy
> on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
> policy action rule.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> 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    | 12 +++++++++++-
>  security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
>  4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -38,6 +38,12 @@ 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();
> @@ -260,8 +266,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;
> +

By default IMA is failsafe. ima_failsafe is true.
Return 0 is needed in failsafe mode. right?
But in this logic it will happen if ima_failsafe is false. meaning it
is not failsafe.

Is it a typo?

>                 return -EACCES;
> +       }
>         return 0;
>  }
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 95209a5f8595..43b85a4fb8e8 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);
>
> @@ -949,6 +973,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);
> @@ -1040,6 +1065,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
>
> --
> 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



-- 
Thanks,
Dmitry
--
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

Re: [PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Mimi Zohar]

On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Permit normally denied access/execute permission for files in policy
> > on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
> > policy action rule.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > ---
> > 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    | 12 +++++++++++-
> >  security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
> >  4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -38,6 +38,12 @@ 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();
> > @@ -260,8 +266,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;
> > +
> 
> By default IMA is failsafe. ima_failsafe is true.
> Return 0 is needed in failsafe mode. right?
> But in this logic it will happen if ima_failsafe is false. meaning it
> is not failsafe.
> 
> Is it a typo?

No, the default, as you pointed out above, is failsafe mode.  Only when we are not in failsafe mode, do we allow the file access/execute for file's that we could not appraise.

Mimi

[PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Mimi Zohar]

On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Permit normally denied access/execute permission for files in policy
> > on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
> > policy action rule.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > ---
> > 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    | 12 +++++++++++-
> >  security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
> >  4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -38,6 +38,12 @@ 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();
> > @@ -260,8 +266,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;
> > +
> 
> By default IMA is failsafe. ima_failsafe is true.
> Return 0 is needed in failsafe mode. right?
> But in this logic it will happen if ima_failsafe is false. meaning it
> is not failsafe.
> 
> Is it a typo?

No, the default, as you pointed out above, is failsafe mode. ?Only when we are not in failsafe mode, do we allow the file access/execute for file's that we could not appraise.

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

Re: [PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Dmitry Kasatkin]

On Tue, Aug 22, 2017 at 3:54 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
>> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Permit normally denied access/execute permission for files in policy
>> > on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
>> > policy action rule.
>> >
>> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> >
>> > ---
>> > 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    | 12 +++++++++++-
>> >  security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
>> >  4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
>> > --- a/security/integrity/ima/ima_main.c
>> > +++ b/security/integrity/ima/ima_main.c
>> > @@ -38,6 +38,12 @@ 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();
>> > @@ -260,8 +266,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;
>> > +
>>
>> By default IMA is failsafe. ima_failsafe is true.
>> Return 0 is needed in failsafe mode. right?
>> But in this logic it will happen if ima_failsafe is false. meaning it
>> is not failsafe.
>>
>> Is it a typo?
>
> No, the default, as you pointed out above, is failsafe mode.  Only when we are not in failsafe mode, do we allow the file access/execute for file's that we could not appraise.
>
> Mimi
>

So in your language "failsafe" means IMA must fail/return with error
on failure..

Ok. then logic is correct and OK with me.


-- 
Thanks,
Dmitry

[PATCH v6 5/6] ima: define "dont_failsafe" policy action rule

[Dmitry Kasatkin]

On Tue, Aug 22, 2017 at 3:54 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
>> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Permit normally denied access/execute permission for files in policy
>> > on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
>> > policy action rule.
>> >
>> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> >
>> > ---
>> > 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    | 12 +++++++++++-
>> >  security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
>> >  4 files changed, 42 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 d23dfe6ede18..b00186914df8 100644
>> > --- a/security/integrity/ima/ima_main.c
>> > +++ b/security/integrity/ima/ima_main.c
>> > @@ -38,6 +38,12 @@ 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();
>> > @@ -260,8 +266,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;
>> > +
>>
>> By default IMA is failsafe. ima_failsafe is true.
>> Return 0 is needed in failsafe mode. right?
>> But in this logic it will happen if ima_failsafe is false. meaning it
>> is not failsafe.
>>
>> Is it a typo?
>
> No, the default, as you pointed out above, is failsafe mode.  Only when we are not in failsafe mode, do we allow the file access/execute for file's that we could not appraise.
>
> Mimi
>

So in your language "failsafe" means IMA must fail/return with error
on failure..

Ok. then logic is correct and OK with me.


-- 
Thanks,
Dmitry
--
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

[PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Mimi Zohar]

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

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
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 d9c171ce4190..4e303be83df6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1502,7 +1502,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
@@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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

[PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Mimi Zohar]

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

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

---
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 d9c171ce4190..4e303be83df6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1502,7 +1502,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
@@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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

--
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

Re: [PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Dmitry Kasatkin]

Looks good to me.


On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Permit normally denied access/execute permission for files in policy
> on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
> builtin policy.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> 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 d9c171ce4190..4e303be83df6 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1502,7 +1502,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
> @@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,
Dmitry

[PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Dmitry Kasatkin]

Looks good to me.


On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Permit normally denied access/execute permission for files in policy
> on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
> builtin policy.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> ---
> 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 d9c171ce4190..4e303be83df6 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1502,7 +1502,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
> @@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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
>
> --
> 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



-- 
Thanks,
Dmitry
--
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

Re: [PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Mimi Zohar]

On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
> Looks good to me.

Thank you for reviewing the code!  Can I add your Reviewed-by/Acked-by?

Mimi

> 
> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Permit normally denied access/execute permission for files in policy
> > on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
> > builtin policy.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > ---
> > 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 d9c171ce4190..4e303be83df6 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -1502,7 +1502,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
> > @@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

[PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Mimi Zohar]

On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
> Looks good to me.

Thank you for reviewing the code!  Can I add your Reviewed-by/Acked-by?

Mimi

> 
> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Permit normally denied access/execute permission for files in policy
> > on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
> > builtin policy.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > ---
> > 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 d9c171ce4190..4e303be83df6 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -1502,7 +1502,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
> > @@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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
> >
> > --
> > 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
> 
> 
> 

--
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

Re: [PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Dmitry Kasatkin]

Hi,

Yes, please add one of appropriate.

Thanks,
Dmitry

On Tue, Aug 22, 2017 at 4:13 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
>> Looks good to me.
>
> Thank you for reviewing the code!  Can I add your Reviewed-by/Acked-by?
>
> Mimi
>
>>
>> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Permit normally denied access/execute permission for files in policy
>> > on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
>> > builtin policy.
>> >
>> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> >
>> > ---
>> > 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 d9c171ce4190..4e303be83df6 100644
>> > --- a/Documentation/admin-guide/kernel-parameters.txt
>> > +++ b/Documentation/admin-guide/kernel-parameters.txt
>> > @@ -1502,7 +1502,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
>> > @@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>



-- 
Thanks,
Dmitry

[PATCH v6 6/6] ima: define "fs_unsafe" builtin policy

[Dmitry Kasatkin]

Hi,

Yes, please add one of appropriate.

Thanks,
Dmitry

On Tue, Aug 22, 2017 at 4:13 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2017-08-22 at 13:07 +0300, Dmitry Kasatkin wrote:
>> Looks good to me.
>
> Thank you for reviewing the code!  Can I add your Reviewed-by/Acked-by?
>
> Mimi
>
>>
>> On Tue, Aug 15, 2017 at 5:43 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Permit normally denied access/execute permission for files in policy
>> > on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
>> > builtin policy.
>> >
>> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> >
>> > ---
>> > 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 d9c171ce4190..4e303be83df6 100644
>> > --- a/Documentation/admin-guide/kernel-parameters.txt
>> > +++ b/Documentation/admin-guide/kernel-parameters.txt
>> > @@ -1502,7 +1502,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
>> > @@ -1517,6 +1517,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 43b85a4fb8e8..cddd9dfb01e1 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
>> >
>> > --
>> > 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
>>
>>
>>
>



-- 
Thanks,
Dmitry
--
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

Re: [PATCH v6 0/6] define new fs integrity_read method

[James Morris]

On Tue, 15 Aug 2017, Mimi Zohar wrote:

> 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.

Are there any such filesystems in mainline which are not getting an 
integrity_read method in this patchset?

-- 
James Morris
<jmorris@namei.org>

[PATCH v6 0/6] define new fs integrity_read method

[James Morris]

On Tue, 15 Aug 2017, Mimi Zohar wrote:

> 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.

Are there any such filesystems in mainline which are not getting an 
integrity_read method in this patchset?

-- 
James Morris
<jmorris@namei.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

Re: [PATCH v6 0/6] define new fs integrity_read method

[Christoph Hellwig]

On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> On Tue, 15 Aug 2017, Mimi Zohar wrote:
> 
> > 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.
> 
> Are there any such filesystems in mainline which are not getting an 
> integrity_read method in this patchset?

There are a few, mostly because we're pretty sure the previous integrity
code did the wrong thing for them - e.g. ocfs2 and gfs2 where locking
vs operations on other cluster nodes was missing, or NFS where in addition
to the above deadlocks were 100% reprodicible with current code.

[PATCH v6 0/6] define new fs integrity_read method

[Christoph Hellwig]

On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> On Tue, 15 Aug 2017, Mimi Zohar wrote:
> 
> > 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.
> 
> Are there any such filesystems in mainline which are not getting an 
> integrity_read method in this patchset?

There are a few, mostly because we're pretty sure the previous integrity
code did the wrong thing for them - e.g. ocfs2 and gfs2 where locking
vs operations on other cluster nodes was missing, or NFS where in addition
to the above deadlocks were 100% reprodicible with current code.
--
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

Re: [PATCH v6 0/6] define new fs integrity_read method

[James Morris]

On Wed, 16 Aug 2017, Christoph Hellwig wrote:

> On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> > On Tue, 15 Aug 2017, Mimi Zohar wrote:
> > 
> > > 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.
> > 
> > Are there any such filesystems in mainline which are not getting an 
> > integrity_read method in this patchset?
> 
> There are a few, mostly because we're pretty sure the previous integrity
> code did the wrong thing for them - e.g. ocfs2 and gfs2 where locking
> vs operations on other cluster nodes was missing, or NFS where in addition
> to the above deadlocks were 100% reprodicible with current code.

Should we do a warn_once for these filesystems when IMA is used?


-- 
James Morris
<jmorris@namei.org>

[PATCH v6 0/6] define new fs integrity_read method

[James Morris]

On Wed, 16 Aug 2017, Christoph Hellwig wrote:

> On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> > On Tue, 15 Aug 2017, Mimi Zohar wrote:
> > 
> > > 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.
> > 
> > Are there any such filesystems in mainline which are not getting an 
> > integrity_read method in this patchset?
> 
> There are a few, mostly because we're pretty sure the previous integrity
> code did the wrong thing for them - e.g. ocfs2 and gfs2 where locking
> vs operations on other cluster nodes was missing, or NFS where in addition
> to the above deadlocks were 100% reprodicible with current code.

Should we do a warn_once for these filesystems when IMA is used?


-- 
James Morris
<jmorris@namei.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

Re: [PATCH v6 0/6] define new fs integrity_read method

[Mimi Zohar]

On Wed, 2017-08-16 at 19:52 +1000, James Morris wrote:
> On Wed, 16 Aug 2017, Christoph Hellwig wrote:
> 
> > On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> > > On Tue, 15 Aug 2017, Mimi Zohar wrote:
> > > 
> > > > 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.
> > > 
> > > Are there any such filesystems in mainline which are not getting an 
> > > integrity_read method in this patchset?
> > 
> > There are a few, mostly because we're pretty sure the previous integrity
> > code did the wrong thing for them - e.g. ocfs2 and gfs2 where locking
> > vs operations on other cluster nodes was missing, or NFS where in addition
> > to the above deadlocks were 100% reprodicible with current code.
> 
> Should we do a warn_once for these filesystems when IMA is used?

I don't think it is necessary.  In terms of IMA-measurement, any file
in policy on an unsupported filesystem will be in the measurement
list, but the file hash will be 0's.  In terms of IMA-appraisal, any
file in policy on an unsupported filesystem will fail appraisal, since
the file hash is 0.

A separate patch set will emit a warning when a file system is mounted
without i_version.

Mimi

[PATCH v6 0/6] define new fs integrity_read method

[Mimi Zohar]

On Wed, 2017-08-16 at 19:52 +1000, James Morris wrote:
> On Wed, 16 Aug 2017, Christoph Hellwig wrote:
> 
> > On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> > > On Tue, 15 Aug 2017, Mimi Zohar wrote:
> > > 
> > > > 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.
> > > 
> > > Are there any such filesystems in mainline which are not getting an 
> > > integrity_read method in this patchset?
> > 
> > There are a few, mostly because we're pretty sure the previous integrity
> > code did the wrong thing for them - e.g. ocfs2 and gfs2 where locking
> > vs operations on other cluster nodes was missing, or NFS where in addition
> > to the above deadlocks were 100% reprodicible with current code.
> 
> Should we do a warn_once for these filesystems when IMA is used?

I don't think it is necessary. ?In terms of IMA-measurement, any file
in policy on an unsupported filesystem will be in the measurement
list, but the file hash will be 0's. ?In terms of IMA-appraisal, any
file in policy on an unsupported filesystem will fail appraisal, since
the file hash is 0.

A separate patch set will emit a warning when a file system is mounted
without i_version.

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

Re: [PATCH v6 0/6] define new fs integrity_read method

[Al Viro]

On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> On Tue, 15 Aug 2017, Mimi Zohar wrote:
> 
> > 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.
> 
> Are there any such filesystems in mainline which are not getting an 
> integrity_read method in this patchset?

A lot - all network ones, minix/sysv/ufs, isofs/udf/efs, fat/hfs/hfs+/hpfs/affs/ntfs,
ecryptfs, reiserfs, romfs, cramfs, bfs, qnx[46], etc.

[PATCH v6 0/6] define new fs integrity_read method

[Al Viro]

On Wed, Aug 16, 2017 at 12:43:58PM +1000, James Morris wrote:
> On Tue, 15 Aug 2017, Mimi Zohar wrote:
> 
> > 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.
> 
> Are there any such filesystems in mainline which are not getting an 
> integrity_read method in this patchset?

A lot - all network ones, minix/sysv/ufs, isofs/udf/efs, fat/hfs/hfs+/hpfs/affs/ntfs,
ecryptfs, reiserfs, romfs, cramfs, bfs, qnx[46], etc.
--
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