2013-05-23 21:32:48

by Paul Taysom

[permalink] [raw]

Subject: [PATCH] fs: ecryptfs: fixed msync to flush data

---

When msync is called on a memory mapped file, that
data is not flushed to the disk.

In Linux, msync calls fsync for the file. For ecryptfs,
fsync just calls the lower level file system's fsync.
Changed the ecryptfs fsync code to call filemap_write_and_wait
before calling the lower level fsync.

Addresses the problem described in http://crbug.com/239536

Signed-off-by: Paul Taysom <[email protected]>
---
fs/ecryptfs/file.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 201f0a0..16f509d 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -295,6 +295,7 @@ static int ecryptfs_release(struct inode *inode, struct file *file)
static int
ecryptfs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
{
+ filemap_write_and_wait(file->f_mapping);
return vfs_fsync(ecryptfs_file_to_lower(file), datasync);
}

--
1.8.2.1

2013-05-24 23:31:47

by Tyler Hicks

[permalink] [raw]

Subject: Re: [PATCH] fs: ecryptfs: fixed msync to flush data

---

On 2013-05-23 14:31:43, Paul Taysom wrote:
> When msync is called on a memory mapped file, that
> data is not flushed to the disk.
>
> In Linux, msync calls fsync for the file. For ecryptfs,
> fsync just calls the lower level file system's fsync.
> Changed the ecryptfs fsync code to call filemap_write_and_wait
> before calling the lower level fsync.
>
> Addresses the problem described in http://crbug.com/239536
>
> Signed-off-by: Paul Taysom <[email protected]>
> ---

Thanks, Paul! I've pushed this to the eCryptfs next branch:

http://git.kernel.org/cgit/linux/kernel/git/tyhicks/ecryptfs.git/log/?h=next

I also marked it for the stable kernel as affecting 3.6 and newer.

As a side note, we need to make ecryptfs_fsync() more selective about
what gets synced by using filemap_write_and_wait_range() and
vfs_fsync_range(). Since we're already calling vfs_fsync() for the
entire lower file, this patch is good enough for now.

Tyler

> fs/ecryptfs/file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
> index 201f0a0..16f509d 100644
> --- a/fs/ecryptfs/file.c
> +++ b/fs/ecryptfs/file.c
> @@ -295,6 +295,7 @@ static int ecryptfs_release(struct inode *inode, struct file *file)
> static int
> ecryptfs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
> {
> + filemap_write_and_wait(file->f_mapping);
> return vfs_fsync(ecryptfs_file_to_lower(file), datasync);
> }
>
> --
> 1.8.2.1
>

---

Attachments:

(No filename) (1.44 kB)
signature.asc (836.00 B)
Digital signature Download all attachments

2013-06-05 06:53:12

by Tyler Hicks

[permalink] [raw]

Subject: [PATCH] eCryptfs: Check return of filemap_write_and_wait during fsync

---

Error out of ecryptfs_fsync() if filemap_write_and_wait() fails.

Signed-off-by: Tyler Hicks <[email protected]>
Cc: Paul Taysom <[email protected]>
Cc: Olof Johansson <[email protected]>
---

After giving Paul's patch one more look, I noticed that we were ignoring
filemap_write_and_wait()'s return value. I plan to push this patch along with
Paul's original patch.

fs/ecryptfs/file.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 16f509d..a7abbea 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -295,7 +295,12 @@ static int ecryptfs_release(struct inode *inode, struct file *file)
static int
ecryptfs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
{
- filemap_write_and_wait(file->f_mapping);
+ int rc;
+
+ rc = filemap_write_and_wait(file->f_mapping);
+ if (rc)
+ return rc;
+
return vfs_fsync(ecryptfs_file_to_lower(file), datasync);
}

--
1.8.1.2