linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Kasatkin <d.kasatkin@samsung.com>
To: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	akpm@linux-foundation.org, viro@ZenIV.linux.org.uk,
	Mimi Zohar <zohar@linux.vnet.ibm.com>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Subject: IMA: kernel reading files opened with O_DIRECT
Date: Wed, 02 Jul 2014 12:40:58 +0300	[thread overview]
Message-ID: <53B3D3AA.3000408@samsung.com> (raw)

Hi,

We are looking for advice on reading files opened for direct_io.

IMA subsystem (security/integrity/ima) reads file content to kernel
buffer with kernel_read() like function to calculate a file hash.
It does not open another instance of 'struct file' but uses one
allocated via 'open' system call.

It works well when file was opened without O_DIRECT flag. In such case
file content is read to the page cache and VFS simply copying data to
the buffer. When buffer is a kernel buffer, it is successfully manged like:

old_fs = get_fs();
set_fs(get_ds());
result = vfs_read(file, (void __user *)addr, count, &pos);
set_fs(old_fs);

In the case when file was opened with O_DIRECT flag, filesystem code
copying data directly to user-space memory and set_fs(get_ds()) trick
does not work anymore.

To overcome this problem we thought about following possible solutions.

1. Allocate user-space memory with vm_mmap().

It works when when file is opened. current->mm is there...
But IMA has certain case to re-read the file at file close.
If file was not explicitly closed with 'close', it will be closed at
process termination somewhere in do_exit().
But at the time file is closing from there, 'current->mm' has already
gone and vm_mmap() fails.

2. Temporarily clear O_DIRECT in file->f_flags.

This is how it looked like in the proposed patch some time ago...

https://lkml.org/lkml/2013/2/20/601

In such approach, by clearing a flag, VFS would be able to write data to
kernel buffer.
But in such case we force to populate page cache which was not an
intention of the process using O_DIRECT.

There reason for the patch was actually a deadlock, because IMA takes
i_mutex and VFS direct_io code also took it.

Al Viro rejected the patch as from his opinion we contaminated locking
rules.

Now we will introduce internal locking and deadlock would not happen
anyway and temporarily clearing O_DIRECT is not to workaround locking
but to be able to read to kernel buffer.

3. Open another instance of the file with 'dentry_open'

Such case would be also similar to temporarily clearing as we would
populate the page cache...

Yeah. O_DIRECT use is certainly rare.. 'man 2 open' has interesting
statement from Linus... :)

"The thing that has always disturbed me about O_DIRECT is that the whole
interface is just stupid, and was probably designed by a deranged monkey
on some serious mind-controlling substances."?Linus

But anyway, there was few complains about deadlock with IMA+O_DIRECT.
Currently we made a patch that when IMA is enabled it does not
measure/appraise files opened with O_DIRECT. Instead it can be
configured to block and log or allow and log O_DIRECT access...

But we would want to cover O_DIRECT case completely and be able to read
file..

Greg KH advised me to write to linux-mm and Andrew as well and ask about
advices on possibility to handle O_DIRECT files.

Is temporarily clearing O_DIRECT flag really unacceptable or not?

Or may be there is a way to allocate "special" user-space like buffer
for kernel and use it with O_DIRECT?

Thanks,
Dmitry


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

             reply	other threads:[~2014-07-02  9:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-02  9:40 Dmitry Kasatkin [this message]
2014-07-02 15:55 ` Jeff Moyer
2014-07-02 18:12   ` Dmitry Kasatkin
2014-07-02 18:40   ` Christoph Hellwig
2014-07-02 18:45     ` Jeff Moyer
2014-07-02 19:07       ` Dmitry Kasatkin
2014-07-11 20:10     ` Pavel Machek
2014-07-11 22:22       ` Dmitry Kasatkin
2014-07-15 13:03         ` Pavel Machek
2014-07-16 12:48           ` Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53B3D3AA.3000408@samsung.com \
    --to=d.kasatkin@samsung.com \
    --cc=akpm@linux-foundation.org \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox