public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ima: prevent dead lock when a file is opened for direct io (take 3)
@ 2014-05-13 15:59 Dmitry Kasatkin
  2014-05-13 15:59 ` [PATCH 1/2] ima: re-introduce own integrity cache lock Dmitry Kasatkin
  2014-05-13 15:59 ` [PATCH 2/2] ima: allocate user-space like memory for direct-io Dmitry Kasatkin
  0 siblings, 2 replies; 3+ messages in thread
From: Dmitry Kasatkin @ 2014-05-13 15:59 UTC (permalink / raw)
  To: viro, linux-security-module, eparis, zohar, linux-ima-devel
  Cc: linux-kernel, hooanon05g, jmorris, dmitry.kasatkin,
	Dmitry Kasatkin

Hi,

Files are measured or appraised based on the IMA policy.  When a file,
in policy, is opened with the O_DIRECT flag, a deadlock occurs.
Kernel oops can be seen bellow.

ima_process_measurement() takes the i_mutex, before calculating the
hash, appraising the hash against a 'good' known value stored as an
extended attribute, and caching the result, before releasing the
i_mutex.  Holding the i_mutex results in a lockdep with
do_blockdev_direct_IO(), which takes the i_mutex and releases it
for each read.

The first attempt at resolving this lockdep temporarily removed the
O_DIRECT flag and restored it, after calculating the hash.
This resolved the lockdep, but the benefits of using O_DIRECT,
in terms of page cache, were lost.  Al Viro objected to this patch,
because it made the "existing locking rule more convoluted".
(The existing (undocumented) IMA locking rule prohibits ->read() of
regular files from taking the i_mutex. The change would have prohibited
->read() of regular files from taking the i_mutex, unless they were
opened with O_DIRECT.)

The second attempt was to introduce the O_DIRECT_HAVELOCK flag for
file->f_flags, which would tell to do_blockdev_direct_IO() to skip
taking of the i_mutex. This patch did not get any feedback.

This patchset solves O_DIRECT problem with fixing IMA by providing
2 patches.

First patch re-introduce IMA iint->mutex and removes prohibition
of ->read of regular files from taking the i_mutex.

As a problem of i_mutex locking is solved, there is now possibility
for IMA to use direct-io functionality without temporarily removing
O_DIRECT. But that has uncovered that it is impossible to pass
kmalloc or vmalloc allocated buffer to the direct-io code, because
it writes directly to the user-space pages. In order to overcome
this problem, second patch introduces usage of vm_mmap() to allocate
user-space like memory like many drivers do.

Thanks,

Dmitry


-----------------------------------------------------------------------
[   38.221614] =============================================
[   38.222422] [ INFO: possible recursive locking detected ]
[   38.223338] 3.13.0-rc7-kds+ #2124 Not tainted
[   38.223993] ---------------------------------------------
[   38.224027] openclose-direc/2637 is trying to acquire lock:
[   38.224027]  (&sb->s_type->i_mutex_key#11){+.+.+.}, at: [<ffffffff8116a414>] __blockdev_direct_IO+0x21e/0x2fbd
[   38.224027] 
[   38.224027] but task is already holding lock:
[   38.224027]  (&sb->s_type->i_mutex_key#11){+.+.+.}, at: [<ffffffff81290748>] process_measurement+0xb0/0x206
[   38.224027] 
[   38.224027] other info that might help us debug this:
[   38.224027]  Possible unsafe locking scenario:
[   38.224027] 
[   38.224027]        CPU0
[   38.224027]        ----
[   38.224027]   lock(&sb->s_type->i_mutex_key#11);
[   38.224027]   lock(&sb->s_type->i_mutex_key#11);
[   38.224027] 
[   38.224027]  *** DEADLOCK ***
[   38.224027] 
[   38.224027]  May be due to missing lock nesting notation
[   38.224027] 
[   38.224027] 1 lock held by openclose-direc/2637:
[   38.224027]  #0:  (&sb->s_type->i_mutex_key#11){+.+.+.}, at: [<ffffffff81290748>] process_measurement+0xb0/0x206
[   38.224027] 
[   38.224027] stack backtrace:
[   38.224027] CPU: 1 PID: 2637 Comm: openclose-direc Not tainted 3.13.0-rc7-kds+ #2124
[   38.224027] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   38.224027]  0000000000000000 ffff8800363cd4c0 ffffffff814edf5e ffffffff821d7930
[   38.224027]  ffff8800363cd580 ffffffff8107f31e ffffffff8102eb8d ffff8800363cd4e8
[   38.224027]  0000000081008925 ffff880000000000 ffffffff00000000 0000000000000000
[   38.224027] Call Trace:
[   38.224027]  [<ffffffff814edf5e>] dump_stack+0x45/0x56
[   38.224027]  [<ffffffff8107f31e>] __lock_acquire+0x802/0x1984
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff8106bf44>] ? sched_clock_cpu+0xc1/0xcc
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff81080a14>] lock_acquire+0xac/0x118
[   38.224027]  [<ffffffff8116a414>] ? __blockdev_direct_IO+0x21e/0x2fbd
[   38.224027]  [<ffffffff814f1a3b>] mutex_lock_nested+0x5e/0x354
[   38.224027]  [<ffffffff8116a414>] ? __blockdev_direct_IO+0x21e/0x2fbd
[   38.224027]  [<ffffffff8116a414>] ? __blockdev_direct_IO+0x21e/0x2fbd
[   38.224027]  [<ffffffff81131c0a>] ? kmem_cache_alloc+0x36/0xf8
[   38.224027]  [<ffffffff8116a3bc>] ? __blockdev_direct_IO+0x1c6/0x2fbd
[   38.224027]  [<ffffffff8116a414>] __blockdev_direct_IO+0x21e/0x2fbd
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff81008925>] ? sched_clock+0x9/0xd
[   38.224027]  [<ffffffff8106bdbc>] ? sched_clock_local+0x12/0x72
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff8106bdbc>] ? sched_clock_local+0x12/0x72
[   38.224027]  [<ffffffff8106bf44>] ? sched_clock_cpu+0xc1/0xcc
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff81008925>] ? sched_clock+0x9/0xd
[   38.224027]  [<ffffffff8106bf44>] ? sched_clock_cpu+0xc1/0xcc
[   38.224027]  [<ffffffff811d53b2>] ext4_ind_direct_IO+0x1fb/0x36a
[   38.224027]  [<ffffffff811a525b>] ? _ext4_get_block+0x160/0x160
[   38.224027]  [<ffffffff811a1f1e>] ext4_direct_IO+0x318/0x3c1
[   38.224027]  [<ffffffff811637cd>] ? __find_get_block+0x1a7/0x1e1
[   38.224027]  [<ffffffff810fe46d>] generic_file_aio_read+0xde/0x61a
[   38.224027]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.224027]  [<ffffffff8106bf44>] ? sched_clock_cpu+0xc1/0xcc
[   38.224027]  [<ffffffff81134c56>] do_sync_read+0x59/0x78
[   38.224027]  [<ffffffff81290c75>] ima_kernel_read+0x5f/0x88
[   38.224027]  [<ffffffff81291483>] ima_calc_file_hash+0x1bd/0x229
[   38.224027]  [<ffffffff811573ac>] ? generic_getxattr+0x4f/0x61
[   38.224027]  [<ffffffff81291383>] ? ima_calc_file_hash+0xbd/0x229
[   38.224027]  [<ffffffff812918b2>] ima_collect_measurement+0x8b/0x11e
[   38.224027]  [<ffffffff814f5163>] ? _raw_write_unlock+0x27/0x2a
[   38.224027]  [<ffffffff812907d3>] process_measurement+0x13b/0x206
[   38.224027]  [<ffffffff812909b1>] ima_file_check+0x113/0x11f
[   38.280032]  [<ffffffff8114231b>] do_last+0x937/0xb83
[   38.280032]  [<ffffffff811427ac>] path_openat+0x245/0x633
[   38.280032]  [<ffffffff8102eb8d>] ? kvm_clock_read+0x1f/0x21
[   38.280032]  [<ffffffff81008925>] ? sched_clock+0x9/0xd
[   38.280032]  [<ffffffff81143972>] do_filp_open+0x3a/0x7f
[   38.280032]  [<ffffffff814f4bf4>] ? _raw_spin_unlock+0x27/0x2a
[   38.280032]  [<ffffffff81150a41>] ? __alloc_fd+0x1b7/0x1c9
[   38.280032]  [<ffffffff811348de>] do_sys_open+0x14d/0x1dc
[   38.280032]  [<ffffffff8113498b>] SyS_open+0x1e/0x20
[   38.280032]  [<ffffffff814fcf69>] system_call_fastpath+0x16/0x1b
-----------------------------------------------------------------------

Dmitry Kasatkin (2):
  ima: re-introduce own integrity cache lock
  ima: allocate user-space like memory for direct-io

 security/integrity/iint.c             | 10 ++++++--
 security/integrity/ima/ima_appraise.c | 22 +++++++-----------
 security/integrity/ima/ima_crypto.c   | 44 ++++++++++++++++++++++++++++-------
 security/integrity/ima/ima_main.c     | 27 ++++++++++++---------
 security/integrity/integrity.h        |  5 ++++
 5 files changed, 73 insertions(+), 35 deletions(-)

-- 
1.9.1


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

* [PATCH 1/2] ima: re-introduce own integrity cache lock
  2014-05-13 15:59 [PATCH 0/2] ima: prevent dead lock when a file is opened for direct io (take 3) Dmitry Kasatkin
@ 2014-05-13 15:59 ` Dmitry Kasatkin
  2014-05-13 15:59 ` [PATCH 2/2] ima: allocate user-space like memory for direct-io Dmitry Kasatkin
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Kasatkin @ 2014-05-13 15:59 UTC (permalink / raw)
  To: viro, linux-security-module, eparis, zohar, linux-ima-devel
  Cc: linux-kernel, hooanon05g, jmorris, dmitry.kasatkin,
	Dmitry Kasatkin

Before IMA appraisal was introduced, IMA was using own integrity cache
lock along with i_mutex. process_measurement and ima_file_free took
the iint->mutex first and then the i_mutex, while setxattr, chmod and
chown took the locks in reverse order. To resolve the potential deadlock,
i_mutex was moved to protect entire IMA functionality and the redundant
iint->mutex was eliminated.

Solution was based on the assumption that filesystem code does not take
i_mutex further. But when file is opened with O_DIRECT flag, direct-io
implementation takes i_mutex and produces deadlock. Furthermore, certain
other filesystem operations, such as llseek, also take i_mutex.

To resolve O_DIRECT related deadlock problem, this patch re-introduces
iint->mutex. But to eliminate the original chmod() related deadlock
problem, this patch eliminates the requirement for chmod hooks to take
the iint->mutex by introducing additional atomic iint->attr_flags to
indicate calling of the hooks. The allowed locking order is to take
the iint->mutex first and then the i_mutex.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/iint.c             | 10 ++++++++--
 security/integrity/ima/ima_appraise.c | 22 ++++++++--------------
 security/integrity/ima/ima_main.c     | 27 ++++++++++++++++-----------
 security/integrity/integrity.h        |  5 +++++
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index a521edf..4c202b7 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -95,13 +95,15 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
 	struct rb_node *node, *parent = NULL;
 	struct integrity_iint_cache *iint, *test_iint;
 
+	mutex_lock(&inode->i_mutex);
+
 	iint = integrity_iint_find(inode);
 	if (iint)
-		return iint;
+		goto out;
 
 	iint = kmem_cache_alloc(iint_cache, GFP_NOFS);
 	if (!iint)
-		return NULL;
+		goto out;
 
 	write_lock(&integrity_iint_lock);
 
@@ -123,6 +125,8 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
 	rb_insert_color(node, &integrity_iint_tree);
 
 	write_unlock(&integrity_iint_lock);
+out:
+	mutex_unlock(&inode->i_mutex);
 	return iint;
 }
 
@@ -154,11 +158,13 @@ static void init_once(void *foo)
 	memset(iint, 0, sizeof(*iint));
 	iint->version = 0;
 	iint->flags = 0UL;
+	iint->attr_flags = 0;
 	iint->ima_file_status = INTEGRITY_UNKNOWN;
 	iint->ima_mmap_status = INTEGRITY_UNKNOWN;
 	iint->ima_bprm_status = INTEGRITY_UNKNOWN;
 	iint->ima_module_status = INTEGRITY_UNKNOWN;
 	iint->evm_status = INTEGRITY_UNKNOWN;
+	mutex_init(&iint->mutex);
 }
 
 static int __init integrity_iintcache_init(void)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index d3113d4..0f8579e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -57,10 +57,12 @@ static int ima_fix_xattr(struct dentry *dentry,
 		iint->ima_hash->xattr.ng.type = IMA_XATTR_DIGEST_NG;
 		iint->ima_hash->xattr.ng.algo = algo;
 	}
+	mutex_lock(&dentry->d_inode->i_mutex);
 	rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
 				   &iint->ima_hash->xattr.data[offset],
 				   (sizeof(iint->ima_hash->xattr) - offset) +
 				   iint->ima_hash->length, 0);
+	mutex_unlock(&dentry->d_inode->i_mutex);
 	return rc;
 }
 
@@ -199,7 +201,9 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 		goto out;
 	}
 
+	mutex_lock(&inode->i_mutex);
 	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+	mutex_unlock(&inode->i_mutex);
 	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
 		if ((status == INTEGRITY_NOLABEL)
 		    || (status == INTEGRITY_NOXATTRS))
@@ -313,13 +317,8 @@ void ima_inode_post_setattr(struct dentry *dentry)
 
 	must_appraise = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR);
 	iint = integrity_iint_find(inode);
-	if (iint) {
-		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
-				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
-				 IMA_ACTION_FLAGS);
-		if (must_appraise)
-			iint->flags |= IMA_APPRAISE;
-	}
+	if (iint)
+		set_bit(IMA_CHANGE_ATTR, &iint->attr_flags);
 	if (!must_appraise)
 		rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA);
 	return;
@@ -349,13 +348,8 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
 		return;
 
 	iint = integrity_iint_find(inode);
-	if (!iint)
-		return;
-
-	iint->flags &= ~IMA_DONE_MASK;
-	if (digsig)
-		iint->flags |= IMA_DIGSIG;
-	return;
+	if (iint)
+		set_bit(IMA_CHANGE_XATTR, &iint->attr_flags);
 }
 
 int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index bd7b4cb..1e63c91 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -88,8 +88,6 @@ static void ima_rdwr_violation_check(struct file *file)
 	if (!S_ISREG(inode->i_mode) || !ima_initialized)
 		return;
 
-	mutex_lock(&inode->i_mutex);	/* file metadata: permissions, xattr */
-
 	if (mode & FMODE_WRITE) {
 		if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) {
 			struct integrity_iint_cache *iint;
@@ -104,8 +102,6 @@ static void ima_rdwr_violation_check(struct file *file)
 			send_writers = true;
 	}
 
-	mutex_unlock(&inode->i_mutex);
-
 	if (!send_tomtou && !send_writers)
 		return;
 
@@ -127,14 +123,14 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
 	if (!(mode & FMODE_WRITE))
 		return;
 
-	mutex_lock(&inode->i_mutex);
+	mutex_lock(&iint->mutex);
 	if (atomic_read(&inode->i_writecount) == 1 &&
 	    iint->version != inode->i_version) {
 		iint->flags &= ~IMA_DONE_MASK;
 		if (iint->flags & IMA_APPRAISE)
 			ima_update_xattr(iint, file);
 	}
-	mutex_unlock(&inode->i_mutex);
+	mutex_unlock(&iint->mutex);
 }
 
 /**
@@ -186,11 +182,20 @@ static int process_measurement(struct file *file, const char *filename,
 	/*  Is the appraise rule hook specific?  */
 	_func = (action & IMA_FILE_APPRAISE) ? FILE_CHECK : function;
 
-	mutex_lock(&inode->i_mutex);
-
 	iint = integrity_inode_get(inode);
 	if (!iint)
-		goto out;
+		goto out_unlocked;
+
+	mutex_lock(&iint->mutex);
+
+	if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->attr_flags))
+		/* reset appraisal flags if ima_inode_post_setattr was called */
+		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
+				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK);
+
+	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->attr_flags))
+		/* reset all flags if ima_inode_setxattr was called */
+		iint->flags &= ~IMA_DONE_MASK;
 
 	/* Determine if already appraised/measured based on bitmask
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
@@ -234,9 +239,9 @@ static int process_measurement(struct file *file, const char *filename,
 out_digsig:
 	if ((mask & MAY_WRITE) && (iint->flags & IMA_DIGSIG))
 		rc = -EACCES;
-out:
-	mutex_unlock(&inode->i_mutex);
+	mutex_unlock(&iint->mutex);
 	kfree(xattr_value);
+out_unlocked:
 	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
 		return -EACCES;
 	return 0;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2fb5e53..f73cd06 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -50,6 +50,9 @@
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
 				 IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED)
 
+#define IMA_CHANGE_XATTR	0
+#define IMA_CHANGE_ATTR		1
+
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,
 	EVM_XATTR_HMAC,
@@ -96,9 +99,11 @@ struct signature_v2_hdr {
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
 	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
+	struct mutex mutex;	/* protects: version, flags, digest */
 	struct inode *inode;	/* back pointer to inode in question */
 	u64 version;		/* track inode changes */
 	unsigned long flags;
+	unsigned long attr_flags;
 	enum integrity_status ima_file_status:4;
 	enum integrity_status ima_mmap_status:4;
 	enum integrity_status ima_bprm_status:4;
-- 
1.9.1


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

* [PATCH 2/2] ima: allocate user-space like memory for direct-io
  2014-05-13 15:59 [PATCH 0/2] ima: prevent dead lock when a file is opened for direct io (take 3) Dmitry Kasatkin
  2014-05-13 15:59 ` [PATCH 1/2] ima: re-introduce own integrity cache lock Dmitry Kasatkin
@ 2014-05-13 15:59 ` Dmitry Kasatkin
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Kasatkin @ 2014-05-13 15:59 UTC (permalink / raw)
  To: viro, linux-security-module, eparis, zohar, linux-ima-devel
  Cc: linux-kernel, hooanon05g, jmorris, dmitry.kasatkin,
	Dmitry Kasatkin

For files opened with O_DIRECT flag, file I/O is done directly to/from
user-space pages. kmalloc or vmalloc allocated memory is not suitable
for direct IO and set_fs(get_ds()) trick does not work there.

This patch uses vm_mmap() call to allocated annonymous user-space like
memory from the kernel.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_crypto.c | 44 ++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index de5b974..59efc0a 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -21,6 +21,7 @@
 #include <linux/scatterlist.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/mman.h>
 #include <crypto/hash.h>
 #include <crypto/hash_info.h>
 #include "ima.h"
@@ -36,7 +37,7 @@ static struct crypto_shash *ima_shash_tfm;
  *
  */
 static int ima_kernel_read(struct file *file, loff_t offset,
-			   char *addr, unsigned long count)
+			   char *addr, unsigned long count, bool kmem)
 {
 	mm_segment_t old_fs;
 	char __user *buf = addr;
@@ -47,13 +48,16 @@ static int ima_kernel_read(struct file *file, loff_t offset,
 	if (!file->f_op->read && !file->f_op->aio_read)
 		return -EINVAL;
 
-	old_fs = get_fs();
-	set_fs(get_ds());
+	if (kmem) {
+		old_fs = get_fs();
+		set_fs(get_ds());
+	}
 	if (file->f_op->read)
 		ret = file->f_op->read(file, buf, count, &offset);
 	else
 		ret = do_sync_read(file, buf, count, &offset);
-	set_fs(old_fs);
+	if (kmem)
+		set_fs(old_fs);
 	return ret;
 }
 
@@ -102,6 +106,7 @@ static int ima_calc_file_hash_tfm(struct file *file,
 {
 	loff_t i_size, offset = 0;
 	char *rbuf;
+	unsigned long addr = 0;
 	int rc, read = 0;
 	struct {
 		struct shash_desc shash;
@@ -122,9 +127,22 @@ static int ima_calc_file_hash_tfm(struct file *file,
 	if (i_size == 0)
 		goto out;
 
-	rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!rbuf)
-		return -ENOMEM;
+	if (file->f_flags & O_DIRECT) {
+		/* direct-io code writes to user-space buffers
+		 * set_fs(get_ds()) trick does not work there
+		 */
+		addr = vm_mmap(NULL, 0, PAGE_SIZE,
+				PROT_READ | PROT_WRITE,
+				MAP_PRIVATE | MAP_ANONYMOUS, 0);
+		if (IS_ERR_VALUE(addr))
+			return addr;
+	}
+
+	rbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!rbuf) {
+		rc = -ENOMEM;
+		goto nomem;
+	}
 
 	if (!(file->f_mode & FMODE_READ)) {
 		file->f_mode |= FMODE_READ;
@@ -134,7 +152,9 @@ static int ima_calc_file_hash_tfm(struct file *file,
 	while (offset < i_size) {
 		int rbuf_len;
 
-		rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
+		rbuf_len = ima_kernel_read(file, offset,
+					   addr ? (char *)addr : rbuf,
+					   PAGE_SIZE, !addr);
 		if (rbuf_len < 0) {
 			rc = rbuf_len;
 			break;
@@ -143,6 +163,11 @@ static int ima_calc_file_hash_tfm(struct file *file,
 			break;
 		offset += rbuf_len;
 
+		if (addr && __copy_from_user(rbuf, (void __user *)addr,
+					     rbuf_len)) {
+			rc = -EFAULT;
+			break;
+		}
 		rc = crypto_shash_update(&desc.shash, rbuf, rbuf_len);
 		if (rc)
 			break;
@@ -150,6 +175,9 @@ static int ima_calc_file_hash_tfm(struct file *file,
 	if (read)
 		file->f_mode &= ~FMODE_READ;
 	kfree(rbuf);
+nomem:
+	if (addr)
+		vm_munmap(addr, PAGE_SIZE);
 out:
 	if (!rc)
 		rc = crypto_shash_final(&desc.shash, hash->digest);
-- 
1.9.1


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

end of thread, other threads:[~2014-05-13 16:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-13 15:59 [PATCH 0/2] ima: prevent dead lock when a file is opened for direct io (take 3) Dmitry Kasatkin
2014-05-13 15:59 ` [PATCH 1/2] ima: re-introduce own integrity cache lock Dmitry Kasatkin
2014-05-13 15:59 ` [PATCH 2/2] ima: allocate user-space like memory for direct-io Dmitry Kasatkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox